Use sendTelemetryErrorEvent, preserve properties but redact errors (#14453) (#14525)

Author: jakebailey

src/client/telemetry/index.ts

@@ -116,53 +116,56 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend
     }
     const reporter = getTelemetryReporter();
     const measures = typeof durationMs === 'number' ? { duration: durationMs } : durationMs ? durationMs : undefined;
-    let customProperties: Record<string, string> = {};
+    const customProperties: Record<string, string> = {};
     const eventNameSent = eventName as string;
 
-    if (ex) {
-        // When sending telemetry events for exceptions no need to send custom properties.
-        // Else we have to review all properties every time as part of GDPR.
-        // Assume we have 10 events all with their own properties.
-        // As we have errors for each event, those properties are treated as new data items.
-        // Hence they need to be classified as part of the GDPR process, and thats unnecessary and onerous.
-        customProperties = { originalEventName: eventName as string };
-        reporter.sendTelemetryException(ex, customProperties, measures);
-    } else {
-        if (properties) {
-            const data = properties as any;
-            Object.getOwnPropertyNames(data).forEach((prop) => {
-                if (data[prop] === undefined || data[prop] === null) {
-                    return;
-                }
-                try {
-                    // If there are any errors in serializing one property, ignore that and move on.
-                    // Else nothing will be sent.
-                    customProperties[prop] =
-                        typeof data[prop] === 'string'
-                            ? data[prop]
-                            : typeof data[prop] === 'object'
-                            ? 'object'
-                            : data[prop].toString();
-                } catch (ex) {
-                    traceError(`Failed to serialize ${prop} for ${eventName}`, ex);
-                }
-            });
-        }
-
-        // Add shared properties to telemetry props (we may overwrite existing ones).
-        Object.assign(customProperties, sharedProperties);
-
-        // Remove shared DS properties from core extension telemetry.
-        Object.keys(sharedProperties).forEach((shareProperty) => {
-            if (
-                customProperties[shareProperty] &&
-                shareProperty.startsWith('ds_') &&
-                !(eventNameSent.startsWith('DS_') || eventNameSent.startsWith('DATASCIENCE'))
-            ) {
-                delete customProperties[shareProperty];
+    if (properties) {
+        const data = properties as any;
+        Object.getOwnPropertyNames(data).forEach((prop) => {
+            if (data[prop] === undefined || data[prop] === null) {
+                return;
+            }
+            try {
+                // If there are any errors in serializing one property, ignore that and move on.
+                // Else nothing will be sent.
+                customProperties[prop] =
+                    typeof data[prop] === 'string'
+                        ? data[prop]
+                        : typeof data[prop] === 'object'
+                        ? 'object'
+                        : data[prop].toString();
+            } catch (ex) {
+                traceError(`Failed to serialize ${prop} for ${eventName}`, ex);
             }
         });
+    }
+
+    // Add shared properties to telemetry props (we may overwrite existing ones).
+    Object.assign(customProperties, sharedProperties);
+
+    // Remove shared DS properties from core extension telemetry.
+    Object.keys(sharedProperties).forEach((shareProperty) => {
+        if (
+            customProperties[shareProperty] &&
+            shareProperty.startsWith('ds_') &&
+            !(eventNameSent.startsWith('DS_') || eventNameSent.startsWith('DATASCIENCE'))
+        ) {
+            delete customProperties[shareProperty];
+        }
+    });
 
+    if (ex) {
+        const errorProps = {
+            errorName: ex.name,
+            errorMessage: ex.message,
+            errorStack: ex.stack ?? ''
+        };
+        Object.assign(customProperties, errorProps);
+
+        // To avoid hardcoding the names and forgetting to update later.
+        const errorPropNames = Object.getOwnPropertyNames(errorProps);
+        reporter.sendTelemetryErrorEvent(eventNameSent, customProperties, measures, errorPropNames);
+    } else {
         reporter.sendTelemetryEvent(eventNameSent, customProperties, measures);
     }
 

src/test/telemetry/index.unit.test.ts

@@ -46,9 +46,8 @@ suite('Telemetry', () => {
             this.sendTelemetryEvent(eventName, properties, measures);
             Reporter.errorProps = errorProps;
         }
-        public sendTelemetryException(error: Error, properties?: {}, measures?: {}): void {
-            this.sendTelemetryEvent('Exception', properties, measures);
-            Reporter.exception = error;
+        public sendTelemetryException(_error: Error, _properties?: {}, _measures?: {}): void {
+            throw new Error('sendTelemetryException is unsupported');
         }
     }
 
@@ -166,16 +165,22 @@ suite('Telemetry', () => {
         rewiremock('vscode-extension-telemetry').with({ default: Reporter });
 
         const eventName = 'Testing';
+        const measures = { start: 123, end: 987 };
         const properties = { hello: 'world', foo: 'bar' };
 
         // tslint:disable-next-line:no-any
-        sendTelemetryEvent(eventName as any, {}, properties as any, error);
+        sendTelemetryEvent(eventName as any, measures, properties as any, error);
 
-        const expectedErrorProperties = {
-            originalEventName: eventName
+        const expectedProperties = {
+            ...properties,
+            errorName: error.name,
+            errorMessage: error.message,
+            errorStack: error.stack
         };
 
-        expect(Reporter.properties).to.deep.equal([expectedErrorProperties]);
-        expect(Reporter.exception).to.deep.equal(error);
+        expect(Reporter.eventName).to.deep.equal([eventName]);
+        expect(Reporter.properties).to.deep.equal([expectedProperties]);
+        expect(Reporter.measures).to.deep.equal([measures]);
+        expect(Reporter.errorProps).to.deep.equal(['errorName', 'errorMessage', 'errorStack']);
     });
 });