Skip to content

Commit 3ceae66

Browse files
authored
chore(cli): file sink telemetry client writes valid json (#710)
Pulled out of #631. Now writes valid json to the telemetry file --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent fdec382 commit 3ceae66

File tree

2 files changed

+15
-24
lines changed

2 files changed

+15
-24
lines changed

packages/aws-cdk/lib/cli/telemetry/file-sink.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import * as fs from 'fs';
2-
import * as path from 'path';
31
import { ToolkitError, type IIoHost } from '@aws-cdk/toolkit-lib';
2+
import * as fs from 'fs-extra';
43
import type { TelemetrySchema } from './schema';
54
import type { ITelemetrySink } from './sink-interface';
65
import { IoHelper } from '../../api-private';
@@ -38,23 +37,19 @@ export class FileTelemetrySink implements ITelemetrySink {
3837
throw new ToolkitError(`Telemetry file already exists at ${this.logFilePath}`);
3938
}
4039

41-
// Create the file if necessary
42-
const directory = path.dirname(this.logFilePath);
43-
if (!fs.existsSync(directory)) {
44-
fs.mkdirSync(directory, { recursive: true });
45-
}
40+
// Create the file
41+
fs.ensureFileSync(this.logFilePath);
42+
fs.writeJsonSync(this.logFilePath, []);
4643
}
4744

4845
/**
4946
* Emit an event.
5047
*/
5148
public async emit(event: TelemetrySchema): Promise<void> {
5249
try {
53-
// Format the events as a JSON string with pretty printing
54-
const output = JSON.stringify(event, null, 2) + '\n';
55-
56-
// Write to file
57-
fs.appendFileSync(this.logFilePath, output);
50+
const json = fs.readJsonSync(this.logFilePath);
51+
json.push(event);
52+
fs.writeJSONSync(this.logFilePath, json, { spaces: 2 });
5853
} catch (e: any) {
5954
// Never throw errors, just log them via ioHost
6055
await this.ioHelper.defaults.trace(`Failed to add telemetry event: ${e.message}`);

packages/aws-cdk/test/cli/telemetry/file-sink.test.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import * as fs from 'fs';
21
import * as os from 'os';
32
import * as path from 'path';
3+
import * as fs from 'fs-extra';
44
import { IoHelper } from '../../../lib/api-private';
55
import { CliIoHost } from '../../../lib/cli/io-host';
66
import { FileTelemetrySink } from '../../../lib/cli/telemetry/file-sink';
@@ -69,10 +69,8 @@ describe('FileTelemetrySink', () => {
6969
await client.emit(testEvent);
7070

7171
// THEN
72-
expect(fs.existsSync(logFilePath)).toBe(true);
73-
const fileContent = fs.readFileSync(logFilePath, 'utf8');
74-
const parsedContent = JSON.parse(fileContent);
75-
expect(parsedContent).toEqual(testEvent);
72+
const fileJson = fs.readJSONSync(logFilePath, 'utf8');
73+
expect(fileJson).toEqual([testEvent]);
7674
});
7775

7876
test('appends data to a file', async () => {
@@ -116,11 +114,9 @@ describe('FileTelemetrySink', () => {
116114

117115
// THEN
118116
expect(fs.existsSync(logFilePath)).toBe(true);
119-
const fileContent = fs.readFileSync(logFilePath, 'utf8');
120-
121-
// The file should contain two JSON objects, each pretty-printed with a newline
122-
const expectedSingleEvent = JSON.stringify(testEvent, null, 2) + '\n';
123-
expect(fileContent).toBe(expectedSingleEvent + expectedSingleEvent);
117+
const fileJson = fs.readJSONSync(logFilePath, 'utf8');
118+
const expectedJson = [testEvent, testEvent];
119+
expect(fileJson).toEqual(expectedJson);
124120
});
125121

126122
test('constructor throws if file already exists', async () => {
@@ -178,8 +174,8 @@ describe('FileTelemetrySink', () => {
178174

179175
const client = new FileTelemetrySink({ logFilePath, ioHost });
180176

181-
// Mock fs.appendFileSync to throw an error
182-
jest.spyOn(fs, 'appendFileSync').mockImplementation(() => {
177+
// Mock fs.writeJSONSync to throw an error
178+
jest.spyOn(fs, 'writeJSONSync').mockImplementation(() => {
183179
throw new Error('File write error');
184180
});
185181

0 commit comments

Comments
 (0)