Skip to content

Rely on AppInsights API for exceptions #13878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MikhailArkhipov merged 28 commits into from
Sep 22, 2020
Merged

Rely on AppInsights API for exceptions #13878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
04e0b34
Fix path
May 6, 2020
9297d96
Merge branch 'master' of https://github.com/Microsoft/vscode-python
May 6, 2020
9f3d5ac
Merge branch 'master' of https://github.com/Microsoft/vscode-python
May 11, 2020
607bf98
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jun 1, 2020
e566214
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jun 11, 2020
4477900
Actually fix settings
Jun 18, 2020
d0d50de
Add news
Jun 18, 2020
03aa5f9
Add test
Jun 18, 2020
e4a032f
Format
Jun 18, 2020
27eeec7
Suppress 'jediEnabled' removal
Jun 18, 2020
89f432e
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jun 23, 2020
3ed655b
Drop survey first launch threshold
Jun 26, 2020
64ac38a
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jun 29, 2020
52a4325
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jun 30, 2020
67d7cb8
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jul 8, 2020
0ce6d91
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jul 22, 2020
571115e
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Jul 30, 2020
41fbcbb
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Aug 4, 2020
8fee970
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Aug 10, 2020
67ea545
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Aug 17, 2020
25d634f
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Aug 18, 2020
de9b63d
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Aug 18, 2020
355a381
Merge branch 'main' of https://github.com/Microsoft/vscode-python
Sep 10, 2020
20c7c53
Telemetry for exceptions
Sep 10, 2020
692a6b0
Merge branch 'main' of https://github.com/Microsoft/vscode-python int…
Sep 10, 2020
1c15da5
Remove custom exception telemetry
Sep 10, 2020
ce56804
Remove unused
Sep 10, 2020
97804c7
Merge branch 'main' of https://github.com/Microsoft/vscode-python int…
Sep 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/client/activation/node/languageServerProxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,12 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy {
// Replace all slashes in the method name so it doesn't get scrubbed by vscode-extension-telemetry.
method: telemetryEvent.Properties.method?.replace(/\//g, '.')
};
sendTelemetryEvent(eventName, telemetryEvent.Measurements, formattedProperties);
sendTelemetryEvent(
eventName,
telemetryEvent.Measurements,
formattedProperties,
telemetryEvent.Exception
);
});
}
await this.registerTestServices();
Expand Down
42 changes: 3 additions & 39 deletions src/client/telemetry/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the MIT License.

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

Expand Down Expand Up @@ -118,17 +117,16 @@ 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> = {};
let eventNameSent = eventName as 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.
eventNameSent = 'ERROR';
customProperties = { originalEventName: eventName as string, stackTrace: serializeStackTrace(ex) };
reporter.sendTelemetryErrorEvent(eventNameSent, customProperties, measures, []);
customProperties = { originalEventName: eventName as string };
reporter.sendTelemetryException(ex, customProperties, measures);
} else {
if (properties) {
const data = properties as any;
Expand Down Expand Up @@ -290,40 +288,6 @@ export function sendTelemetryWhenDone<P extends IEventNamePropertyMapping, E ext
}
}

function serializeStackTrace(ex: Error): string {
// We aren't showing the error message (ex.message) since it might contain PII.
let trace = '';
for (const frame of stackTrace.parse(ex)) {
const filename = frame.getFileName();
if (filename) {
const lineno = frame.getLineNumber();
const colno = frame.getColumnNumber();
trace += `\n\tat ${getCallsite(frame)} ${filename}:${lineno}:${colno}`;
} else {
trace += '\n\tat <anonymous>';
}
}
// Ensure we always use `/` as path separators.
// This way stack traces (with relative paths) coming from different OS will always look the same.
return trace.trim().replace(/\\/g, '/');
}

function getCallsite(frame: stackTrace.StackFrame) {
const parts: string[] = [];
if (typeof frame.getTypeName() === 'string' && frame.getTypeName().length > 0) {
parts.push(frame.getTypeName());
}
if (typeof frame.getMethodName() === 'string' && frame.getMethodName().length > 0) {
parts.push(frame.getMethodName());
}
if (typeof frame.getFunctionName() === 'string' && frame.getFunctionName().length > 0) {
if (parts.length !== 2 || parts.join('.') !== frame.getFunctionName()) {
parts.push(frame.getFunctionName());
}
}
return parts.join('.');
}

/**
* Map all shared properties to their data types.
*/
Expand Down
86 changes: 9 additions & 77 deletions src/test/telemetry/index.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { instance, mock, verify, when } from 'ts-mockito';
import { WorkspaceConfiguration } from 'vscode';
import { IWorkspaceService } from '../../client/common/application/types';
import { WorkspaceService } from '../../client/common/application/workspace';
import { EXTENSION_ROOT_DIR } from '../../client/constants';
import {
_resetSharedProperties,
clearTelemetryReporter,
Expand All @@ -30,6 +29,8 @@ suite('Telemetry', () => {
public static properties: Record<string, string>[] = [];
public static measures: {}[] = [];
public static errorProps: string[] | undefined;
public static exception: Error | undefined;

public static clear() {
Reporter.eventName = [];
Reporter.properties = [];
Expand All @@ -45,6 +46,10 @@ 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;
}
}

setup(() => {
Expand Down Expand Up @@ -155,95 +160,22 @@ suite('Telemetry', () => {
expect(Reporter.measures).to.deep.equal([measures]);
expect(Reporter.properties).to.deep.equal([expectedProperties]);
});
test('Send Error Telemetry', () => {
rewiremock.enable();
const error = new Error('Boo');
rewiremock('vscode-extension-telemetry').with({ default: Reporter });

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

// tslint:disable-next-line:no-any
sendTelemetryEvent(eventName as any, measures, properties as any, error);

const expectedErrorProperties = {
originalEventName: eventName
};

expect(Reporter.eventName).to.deep.equal(['ERROR']);
expect(Reporter.measures).to.deep.equal([measures]);
expect(Reporter.properties[0].stackTrace).to.be.length.greaterThan(1);
delete Reporter.properties[0].stackTrace;
expect(Reporter.properties).to.deep.equal([expectedErrorProperties]);
expect(Reporter.errorProps).to.deep.equal([]);
});
test('Send Error Telemetry with stack trace', () => {
test('Send Exception Telemetry', () => {
rewiremock.enable();
const error = new Error('Boo');
const root = EXTENSION_ROOT_DIR.replace(/\\/g, '/');
error.stack = [
'Error: Boo',
`at Context.test (${root}/src/test/telemetry/index.unit.test.ts:50:23)`,
`at callFn (${root}/node_modules/mocha/lib/runnable.js:372:21)`,
`at Test.Runnable.run (${root}/node_modules/mocha/lib/runnable.js:364:7)`,
`at Runner.runTest (${root}/node_modules/mocha/lib/runner.js:455:10)`,
`at ${root}/node_modules/mocha/lib/runner.js:573:12`,
`at next (${root}/node_modules/mocha/lib/runner.js:369:14)`,
`at ${root}/node_modules/mocha/lib/runner.js:379:7`,
`at next (${root}/node_modules/mocha/lib/runner.js:303:14)`,
`at ${root}/node_modules/mocha/lib/runner.js:342:7`,
`at done (${root}/node_modules/mocha/lib/runnable.js:319:5)`,
`at callFn (${root}/node_modules/mocha/lib/runnable.js:395:7)`,
`at Hook.Runnable.run (${root}/node_modules/mocha/lib/runnable.js:364:7)`,
`at next (${root}/node_modules/mocha/lib/runner.js:317:10)`,
`at Immediate.<anonymous> (${root}/node_modules/mocha/lib/runner.js:347:5)`,
'at runCallback (timers.js:789:20)',
'at tryOnImmediate (timers.js:751:5)',
'at processImmediate [as _immediateCallback] (timers.js:722:5)'
].join('\n\t');
rewiremock('vscode-extension-telemetry').with({ default: Reporter });

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

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

const expectedErrorProperties = {
originalEventName: eventName
};

const stackTrace = Reporter.properties[0].stackTrace;
delete Reporter.properties[0].stackTrace;

expect(Reporter.eventName).to.deep.equal(['ERROR']);
expect(Reporter.measures).to.deep.equal([measures]);
expect(Reporter.properties).to.deep.equal([expectedErrorProperties]);
expect(stackTrace).to.be.length.greaterThan(1);
expect(Reporter.errorProps).to.deep.equal([]);

const expectedStack = [
`at Context.test ${root}/src/test/telemetry/index.unit.test.ts:50:23`,
`at callFn ${root}/node_modules/mocha/lib/runnable.js:372:21`,
`at Test.Runnable.run ${root}/node_modules/mocha/lib/runnable.js:364:7`,
`at Runner.runTest ${root}/node_modules/mocha/lib/runner.js:455:10`,
`at ${root}/node_modules/mocha/lib/runner.js:573:12`,
`at next ${root}/node_modules/mocha/lib/runner.js:369:14`,
`at ${root}/node_modules/mocha/lib/runner.js:379:7`,
`at next ${root}/node_modules/mocha/lib/runner.js:303:14`,
`at ${root}/node_modules/mocha/lib/runner.js:342:7`,
`at done ${root}/node_modules/mocha/lib/runnable.js:319:5`,
`at callFn ${root}/node_modules/mocha/lib/runnable.js:395:7`,
`at Hook.Runnable.run ${root}/node_modules/mocha/lib/runnable.js:364:7`,
`at next ${root}/node_modules/mocha/lib/runner.js:317:10`,
`at Immediate ${root}/node_modules/mocha/lib/runner.js:347:5`,
'at runCallback timers.js:789:20',
'at tryOnImmediate timers.js:751:5',
'at processImmediate [as _immediateCallback] timers.js:722:5'
].join('\n\t');

expect(stackTrace).to.be.equal(expectedStack);
expect(Reporter.exception).to.deep.equal(error);
});
});