From e68b4b79bbe5400065b688dc98e2f6c96520244e Mon Sep 17 00:00:00 2001 From: John Chou Date: Fri, 15 Sep 2023 20:15:42 +0800 Subject: [PATCH 1/3] Fix no common properties when logging error by algining 'properties' contract of data --- src/vs/workbench/api/common/extHostTelemetry.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostTelemetry.ts b/src/vs/workbench/api/common/extHostTelemetry.ts index ea9d652f1439e..be917cb6c243e 100644 --- a/src/vs/workbench/api/common/extHostTelemetry.ts +++ b/src/vs/workbench/api/common/extHostTelemetry.ts @@ -237,7 +237,10 @@ export class ExtHostTelemetryLogger { if ('properties' in data) { data.properties = updatedData; } else { - data = updatedData; + data = { + ...data, + properties: updatedData + }; } return data; From dd9753d0f7ea1fd01d57dc6e4ffc1ddd5ec44b65 Mon Sep 17 00:00:00 2001 From: John Chou Date: Fri, 15 Sep 2023 23:34:51 +0800 Subject: [PATCH 2/3] Refactoring and update UT test --- .../workbench/api/common/extHostTelemetry.ts | 20 ++++++++----------- .../api/test/browser/extHostTelemetry.test.ts | 6 +++++- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTelemetry.ts b/src/vs/workbench/api/common/extHostTelemetry.ts index be917cb6c243e..c083f6b50319c 100644 --- a/src/vs/workbench/api/common/extHostTelemetry.ts +++ b/src/vs/workbench/api/common/extHostTelemetry.ts @@ -221,10 +221,11 @@ export class ExtHostTelemetryLogger { mixInCommonPropsAndCleanData(data: Record): Record { // Some telemetry modules prefer to break properties and measurmements up // We mix common properties into the properties tab. - let updatedData = 'properties' in data ? (data.properties ?? {}) : data; + const { properties = {}, measurements, ...restData } = data; // We don't clean measurements since they are just numbers - updatedData = cleanData(updatedData, []); + let updatedData = cleanData(properties, []); + const sanitizedRestData = cleanData(restData, []); if (this._additionalCommonProperties) { updatedData = mixin(updatedData, this._additionalCommonProperties); @@ -234,16 +235,11 @@ export class ExtHostTelemetryLogger { updatedData = mixin(updatedData, this._commonProperties); } - if ('properties' in data) { - data.properties = updatedData; - } else { - data = { - ...data, - properties: updatedData - }; - } - - return data; + return { + ...sanitizedRestData, + properties: updatedData, + measurements + }; } private logEvent(eventName: string, data?: Record): void { diff --git a/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts b/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts index 262ba48a3efee..162e3b22bd225 100644 --- a/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts +++ b/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts @@ -183,7 +183,7 @@ suite('ExtHostTelemetry', function () { assert.strictEqual(functionSpy.dataArr.length, 1); assert.strictEqual(functionSpy.dataArr[0].eventName, `${mockExtensionIdentifier.name}/test-event`); assert.strictEqual(functionSpy.dataArr[0].data['test-data'], 'test-data'); - assert.strictEqual(functionSpy.dataArr[0].data['common.foo'], 'bar'); + assert.strictEqual(functionSpy.dataArr[0].data.properties['common.foo'], 'bar'); logger.logUsage('test-event', { 'test-data': 'test-data' }); assert.strictEqual(functionSpy.dataArr.length, 2); @@ -218,6 +218,9 @@ suite('ExtHostTelemetry', function () { 'fake-token': 'token=123', 'fake-slack-token': 'xoxp-123', 'fake-path': '/Users/username/.vscode/extensions', + properties: { + 'deep-fake-password': 'pwd=123' + } }); assert.strictEqual(functionSpy.dataArr.length, 1); @@ -227,6 +230,7 @@ suite('ExtHostTelemetry', function () { assert.strictEqual(functionSpy.dataArr[0].data['fake-token'], ''); assert.strictEqual(functionSpy.dataArr[0].data['fake-slack-token'], ''); assert.strictEqual(functionSpy.dataArr[0].data['fake-path'], ''); + assert.strictEqual(functionSpy.dataArr[0].data.properties['deep-fake-password'], ''); }); test('Ensure output channel is logged to', function () { From 046025f1dc712c7d1149a831530a935f881ec9f3 Mon Sep 17 00:00:00 2001 From: John Chou Date: Fri, 22 Sep 2023 00:47:22 +0800 Subject: [PATCH 3/3] Remove measurements handling by comment --- src/vs/workbench/api/common/extHostTelemetry.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTelemetry.ts b/src/vs/workbench/api/common/extHostTelemetry.ts index c083f6b50319c..61aa1f2e24f71 100644 --- a/src/vs/workbench/api/common/extHostTelemetry.ts +++ b/src/vs/workbench/api/common/extHostTelemetry.ts @@ -219,11 +219,11 @@ export class ExtHostTelemetryLogger { } mixInCommonPropsAndCleanData(data: Record): Record { - // Some telemetry modules prefer to break properties and measurmements up + // Some telemetry modules prefer to break properties and measurements up // We mix common properties into the properties tab. - const { properties = {}, measurements, ...restData } = data; + const { properties = {}, ...restData } = data; - // We don't clean measurements since they are just numbers + // cleanData() only cleans string now let updatedData = cleanData(properties, []); const sanitizedRestData = cleanData(restData, []); @@ -237,8 +237,7 @@ export class ExtHostTelemetryLogger { return { ...sanitizedRestData, - properties: updatedData, - measurements + properties: updatedData }; }