Skip to content

Commit 367e002

Browse files
author
Mikhail Arkhipov
authored
Rely on AppInsights API for exceptions (#13878)
* Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Telemetry for exceptions * Remove custom exception telemetry * Remove unused
1 parent 221c0d5 commit 367e002

File tree

3 files changed

+18
-117
lines changed

3 files changed

+18
-117
lines changed

‎src/client/activation/node/languageServerProxy.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,12 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy {
126126
// Replace all slashes in the method name so it doesn't get scrubbed by vscode-extension-telemetry.
127127
method: telemetryEvent.Properties.method?.replace(/\//g, '.')
128128
};
129-
sendTelemetryEvent(eventName, telemetryEvent.Measurements, formattedProperties);
129+
sendTelemetryEvent(
130+
eventName,
131+
telemetryEvent.Measurements,
132+
formattedProperties,
133+
telemetryEvent.Exception
134+
);
130135
});
131136
}
132137
await this.registerTestServices();

‎src/client/telemetry/index.ts

Lines changed: 3 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT License.
33

44
import type { JSONObject } from '@phosphor/coreutils';
5-
import * as stackTrace from 'stack-trace';
65
// tslint:disable-next-line: import-name
76
import TelemetryReporter from 'vscode-extension-telemetry/lib/telemetryReporter';
87

@@ -118,17 +117,16 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend
118117
const reporter = getTelemetryReporter();
119118
const measures = typeof durationMs === 'number' ? { duration: durationMs } : durationMs ? durationMs : undefined;
120119
let customProperties: Record<string, string> = {};
121-
let eventNameSent = eventName as string;
120+
const eventNameSent = eventName as string;
122121

123122
if (ex) {
124123
// When sending telemetry events for exceptions no need to send custom properties.
125124
// Else we have to review all properties every time as part of GDPR.
126125
// Assume we have 10 events all with their own properties.
127126
// As we have errors for each event, those properties are treated as new data items.
128127
// Hence they need to be classified as part of the GDPR process, and thats unnecessary and onerous.
129-
eventNameSent = 'ERROR';
130-
customProperties = { originalEventName: eventName as string, stackTrace: serializeStackTrace(ex) };
131-
reporter.sendTelemetryErrorEvent(eventNameSent, customProperties, measures, []);
128+
customProperties = { originalEventName: eventName as string };
129+
reporter.sendTelemetryException(ex, customProperties, measures);
132130
} else {
133131
if (properties) {
134132
const data = properties as any;
@@ -290,40 +288,6 @@ export function sendTelemetryWhenDone<P extends IEventNamePropertyMapping, E ext
290288
}
291289
}
292290

293-
function serializeStackTrace(ex: Error): string {
294-
// We aren't showing the error message (ex.message) since it might contain PII.
295-
let trace = '';
296-
for (const frame of stackTrace.parse(ex)) {
297-
const filename = frame.getFileName();
298-
if (filename) {
299-
const lineno = frame.getLineNumber();
300-
const colno = frame.getColumnNumber();
301-
trace += `\n\tat ${getCallsite(frame)} ${filename}:${lineno}:${colno}`;
302-
} else {
303-
trace += '\n\tat <anonymous>';
304-
}
305-
}
306-
// Ensure we always use `/` as path separators.
307-
// This way stack traces (with relative paths) coming from different OS will always look the same.
308-
return trace.trim().replace(/\\/g, '/');
309-
}
310-
311-
function getCallsite(frame: stackTrace.StackFrame) {
312-
const parts: string[] = [];
313-
if (typeof frame.getTypeName() === 'string' && frame.getTypeName().length > 0) {
314-
parts.push(frame.getTypeName());
315-
}
316-
if (typeof frame.getMethodName() === 'string' && frame.getMethodName().length > 0) {
317-
parts.push(frame.getMethodName());
318-
}
319-
if (typeof frame.getFunctionName() === 'string' && frame.getFunctionName().length > 0) {
320-
if (parts.length !== 2 || parts.join('.') !== frame.getFunctionName()) {
321-
parts.push(frame.getFunctionName());
322-
}
323-
}
324-
return parts.join('.');
325-
}
326-
327291
/**
328292
* Map all shared properties to their data types.
329293
*/

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

Lines changed: 9 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { instance, mock, verify, when } from 'ts-mockito';
1111
import { WorkspaceConfiguration } from 'vscode';
1212
import { IWorkspaceService } from '../../client/common/application/types';
1313
import { WorkspaceService } from '../../client/common/application/workspace';
14-
import { EXTENSION_ROOT_DIR } from '../../client/constants';
1514
import {
1615
_resetSharedProperties,
1716
clearTelemetryReporter,
@@ -30,6 +29,8 @@ suite('Telemetry', () => {
3029
public static properties: Record<string, string>[] = [];
3130
public static measures: {}[] = [];
3231
public static errorProps: string[] | undefined;
32+
public static exception: Error | undefined;
33+
3334
public static clear() {
3435
Reporter.eventName = [];
3536
Reporter.properties = [];
@@ -45,6 +46,10 @@ suite('Telemetry', () => {
4546
this.sendTelemetryEvent(eventName, properties, measures);
4647
Reporter.errorProps = errorProps;
4748
}
49+
public sendTelemetryException(error: Error, properties?: {}, measures?: {}): void {
50+
this.sendTelemetryEvent('Exception', properties, measures);
51+
Reporter.exception = error;
52+
}
4853
}
4954

5055
setup(() => {
@@ -155,95 +160,22 @@ suite('Telemetry', () => {
155160
expect(Reporter.measures).to.deep.equal([measures]);
156161
expect(Reporter.properties).to.deep.equal([expectedProperties]);
157162
});
158-
test('Send Error Telemetry', () => {
159-
rewiremock.enable();
160-
const error = new Error('Boo');
161-
rewiremock('vscode-extension-telemetry').with({ default: Reporter });
162-
163-
const eventName = 'Testing';
164-
const properties = { hello: 'world', foo: 'bar' };
165-
const measures = { start: 123, end: 987 };
166-
167-
// tslint:disable-next-line:no-any
168-
sendTelemetryEvent(eventName as any, measures, properties as any, error);
169-
170-
const expectedErrorProperties = {
171-
originalEventName: eventName
172-
};
173-
174-
expect(Reporter.eventName).to.deep.equal(['ERROR']);
175-
expect(Reporter.measures).to.deep.equal([measures]);
176-
expect(Reporter.properties[0].stackTrace).to.be.length.greaterThan(1);
177-
delete Reporter.properties[0].stackTrace;
178-
expect(Reporter.properties).to.deep.equal([expectedErrorProperties]);
179-
expect(Reporter.errorProps).to.deep.equal([]);
180-
});
181-
test('Send Error Telemetry with stack trace', () => {
163+
test('Send Exception Telemetry', () => {
182164
rewiremock.enable();
183165
const error = new Error('Boo');
184-
const root = EXTENSION_ROOT_DIR.replace(/\\/g, '/');
185-
error.stack = [
186-
'Error: Boo',
187-
`at Context.test (${root}/src/test/telemetry/index.unit.test.ts:50:23)`,
188-
`at callFn (${root}/node_modules/mocha/lib/runnable.js:372:21)`,
189-
`at Test.Runnable.run (${root}/node_modules/mocha/lib/runnable.js:364:7)`,
190-
`at Runner.runTest (${root}/node_modules/mocha/lib/runner.js:455:10)`,
191-
`at ${root}/node_modules/mocha/lib/runner.js:573:12`,
192-
`at next (${root}/node_modules/mocha/lib/runner.js:369:14)`,
193-
`at ${root}/node_modules/mocha/lib/runner.js:379:7`,
194-
`at next (${root}/node_modules/mocha/lib/runner.js:303:14)`,
195-
`at ${root}/node_modules/mocha/lib/runner.js:342:7`,
196-
`at done (${root}/node_modules/mocha/lib/runnable.js:319:5)`,
197-
`at callFn (${root}/node_modules/mocha/lib/runnable.js:395:7)`,
198-
`at Hook.Runnable.run (${root}/node_modules/mocha/lib/runnable.js:364:7)`,
199-
`at next (${root}/node_modules/mocha/lib/runner.js:317:10)`,
200-
`at Immediate.<anonymous> (${root}/node_modules/mocha/lib/runner.js:347:5)`,
201-
'at runCallback (timers.js:789:20)',
202-
'at tryOnImmediate (timers.js:751:5)',
203-
'at processImmediate [as _immediateCallback] (timers.js:722:5)'
204-
].join('\n\t');
205166
rewiremock('vscode-extension-telemetry').with({ default: Reporter });
206167

207168
const eventName = 'Testing';
208169
const properties = { hello: 'world', foo: 'bar' };
209-
const measures = { start: 123, end: 987 };
210170

211171
// tslint:disable-next-line:no-any
212-
sendTelemetryEvent(eventName as any, measures, properties as any, error);
172+
sendTelemetryEvent(eventName as any, {}, properties as any, error);
213173

214174
const expectedErrorProperties = {
215175
originalEventName: eventName
216176
};
217177

218-
const stackTrace = Reporter.properties[0].stackTrace;
219-
delete Reporter.properties[0].stackTrace;
220-
221-
expect(Reporter.eventName).to.deep.equal(['ERROR']);
222-
expect(Reporter.measures).to.deep.equal([measures]);
223178
expect(Reporter.properties).to.deep.equal([expectedErrorProperties]);
224-
expect(stackTrace).to.be.length.greaterThan(1);
225-
expect(Reporter.errorProps).to.deep.equal([]);
226-
227-
const expectedStack = [
228-
`at Context.test ${root}/src/test/telemetry/index.unit.test.ts:50:23`,
229-
`at callFn ${root}/node_modules/mocha/lib/runnable.js:372:21`,
230-
`at Test.Runnable.run ${root}/node_modules/mocha/lib/runnable.js:364:7`,
231-
`at Runner.runTest ${root}/node_modules/mocha/lib/runner.js:455:10`,
232-
`at ${root}/node_modules/mocha/lib/runner.js:573:12`,
233-
`at next ${root}/node_modules/mocha/lib/runner.js:369:14`,
234-
`at ${root}/node_modules/mocha/lib/runner.js:379:7`,
235-
`at next ${root}/node_modules/mocha/lib/runner.js:303:14`,
236-
`at ${root}/node_modules/mocha/lib/runner.js:342:7`,
237-
`at done ${root}/node_modules/mocha/lib/runnable.js:319:5`,
238-
`at callFn ${root}/node_modules/mocha/lib/runnable.js:395:7`,
239-
`at Hook.Runnable.run ${root}/node_modules/mocha/lib/runnable.js:364:7`,
240-
`at next ${root}/node_modules/mocha/lib/runner.js:317:10`,
241-
`at Immediate ${root}/node_modules/mocha/lib/runner.js:347:5`,
242-
'at runCallback timers.js:789:20',
243-
'at tryOnImmediate timers.js:751:5',
244-
'at processImmediate [as _immediateCallback] timers.js:722:5'
245-
].join('\n\t');
246-
247-
expect(stackTrace).to.be.equal(expectedStack);
179+
expect(Reporter.exception).to.deep.equal(error);
248180
});
249181
});

0 commit comments

Comments
 (0)