Skip to content

Commit bc0550f

Browse files
connor4312connor4312
committed
debug: filter exceptions from DAP telemetry on web
This change will filter DAP keys beginning with `!` when the remote authority is "other", which SteVen recommended as a way to tell if we're on web/codespaces. Isidor mentioned previously that there was filtering for keys beginning with an underscore, but this was only applied to some data in error responses, not general telemetry events. An alternative approach to this is filtering exceptions based on the environment on the extension side, but I figured it would be better to have VS Code do that so that we don't end up with N many possibly deviant sets of environment detection logic.
1 parent f8f0c0e commit bc0550f

10 files changed

Lines changed: 112 additions & 5 deletions

File tree

‎src/vs/editor/standalone/browser/simpleServices.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,7 @@ export class StandaloneTelemetryService implements ITelemetryService {
548548
_serviceBrand: undefined;
549549

550550
public isOptedIn = false;
551+
public sendErrorTelemetry = false;
551552

552553
public setEnabled(value: boolean): void {
553554
}

‎src/vs/platform/telemetry/common/telemetry.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ export interface ITelemetryData {
2323

2424
export interface ITelemetryService {
2525

26+
/**
27+
* Whether error telemetry will get sent. If false, `publicLogError` will no-op.
28+
*/
29+
readonly sendErrorTelemetry: boolean;
30+
2631
_serviceBrand: undefined;
2732

2833
/**

‎src/vs/platform/telemetry/common/telemetryService.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export class TelemetryService implements ITelemetryService {
3535
private _piiPaths: string[];
3636
private _userOptIn: boolean;
3737
private _enabled: boolean;
38-
private _sendErrorTelemetry: boolean;
38+
public readonly sendErrorTelemetry: boolean;
3939

4040
private readonly _disposables = new DisposableStore();
4141
private _cleanupPatterns: RegExp[] = [];
@@ -49,7 +49,7 @@ export class TelemetryService implements ITelemetryService {
4949
this._piiPaths = config.piiPaths || [];
5050
this._userOptIn = true;
5151
this._enabled = true;
52-
this._sendErrorTelemetry = !!config.sendErrorTelemetry;
52+
this.sendErrorTelemetry = !!config.sendErrorTelemetry;
5353

5454
// static cleanup pattern for: `file:///DANGEROUS/PATH/resources/app/Useful/Information`
5555
this._cleanupPatterns = [/file:\/\/\/.*?\/resources\/app\//gi];
@@ -148,7 +148,7 @@ export class TelemetryService implements ITelemetryService {
148148
}
149149

150150
publicLogError(errorEventName: string, data?: ITelemetryData): Promise<any> {
151-
if (!this._sendErrorTelemetry) {
151+
if (!this.sendErrorTelemetry) {
152152
return Promise.resolve(undefined);
153153
}
154154

‎src/vs/platform/telemetry/common/telemetryUtils.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { isObject } from 'vs/base/common/types';
1313

1414
export const NullTelemetryService = new class implements ITelemetryService {
1515
_serviceBrand: undefined;
16+
readonly sendErrorTelemetry = false;
17+
1618
publicLog(eventName: string, data?: ITelemetryData) {
1719
return Promise.resolve(undefined);
1820
}

‎src/vs/workbench/contrib/debug/browser/debugSession.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,17 @@ export class DebugSession implements IDebugSession {
848848
// and the user opted in telemetry
849849
if (this.raw.customTelemetryService && this.telemetryService.isOptedIn) {
850850
// __GDPR__TODO__ We're sending events in the name of the debug extension and we can not ensure that those are declared correctly.
851-
this.raw.customTelemetryService.publicLog(event.body.output, event.body.data);
851+
let data = event.body.data;
852+
if (!this.raw.customTelemetryService.sendErrorTelemetry && event.body.data) {
853+
data = {};
854+
for (const key of Object.keys(event.body.data)) {
855+
if (!key.startsWith('!')) {
856+
data[key] = event.body.data[key];
857+
}
858+
}
859+
}
860+
861+
this.raw.customTelemetryService.publicLog(event.body.output, data);
852862
}
853863

854864
return;

‎src/vs/workbench/contrib/debug/node/debugHelperService.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,17 @@ import { getPathFromAmdModule } from 'vs/base/common/amd';
1010
import { TelemetryService } from 'vs/platform/telemetry/common/telemetryService';
1111
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1212
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
13+
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
14+
import { cleanRemoteAuthority } from 'vs/platform/telemetry/common/telemetryUtils';
1315

1416
export class NodeDebugHelperService implements IDebugHelperService {
1517
_serviceBrand: undefined;
1618

19+
constructor(
20+
@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
21+
) { }
22+
23+
1724
createTelemetryService(configurationService: IConfigurationService, args: string[]): TelemetryService | undefined {
1825

1926
const client = new TelemetryClient(
@@ -33,7 +40,10 @@ export class NodeDebugHelperService implements IDebugHelperService {
3340
const channel = client.getChannel('telemetryAppender');
3441
const appender = new TelemetryAppenderClient(channel);
3542

36-
return new TelemetryService({ appender, sendErrorTelemetry: true }, configurationService);
43+
return new TelemetryService({
44+
appender,
45+
sendErrorTelemetry: cleanRemoteAuthority(this.environmentService.configuration.remoteAuthority) !== 'other'
46+
}, configurationService);
3747
}
3848
}
3949

‎src/vs/workbench/contrib/debug/test/browser/telemetry.test.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as assert from 'assert';
7+
import { MockDebugAdapter, createMockDebugModel } from 'vs/workbench/contrib/debug/test/common/mockDebug';
8+
import { DebugModel } from 'vs/workbench/contrib/debug/common/debugModel';
9+
import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession';
10+
import { generateUuid } from 'vs/base/common/uuid';
11+
import { NullOpenerService } from 'vs/platform/opener/common/opener';
12+
import { RawDebugSession } from 'vs/workbench/contrib/debug/browser/rawDebugSession';
13+
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
14+
import { stub, SinonStub } from 'sinon';
15+
import { timeout } from 'vs/base/common/async';
16+
17+
suite('Debug - DebugSession telemetry', () => {
18+
let model: DebugModel;
19+
let session: DebugSession;
20+
let adapter: MockDebugAdapter;
21+
let telemetry: { isOptedIn: boolean; sendErrorTelemetry: boolean; publicLog: SinonStub };
22+
23+
setup(() => {
24+
telemetry = { isOptedIn: true, sendErrorTelemetry: true, publicLog: stub() };
25+
adapter = new MockDebugAdapter();
26+
model = createMockDebugModel();
27+
28+
const telemetryService = telemetry as Partial<ITelemetryService> as ITelemetryService;
29+
session = new DebugSession(generateUuid(), undefined!, undefined!, model, undefined, undefined!, telemetryService, undefined!, undefined!, undefined!, undefined!, undefined!, undefined!, NullOpenerService, undefined!, undefined!);
30+
session.initializeForTest(new RawDebugSession(adapter, undefined!, undefined!, telemetryService, undefined!, undefined!, undefined!));
31+
});
32+
33+
test('does not send telemetry when opted out', async () => {
34+
telemetry.isOptedIn = false;
35+
adapter.sendEventBody('output', {
36+
category: 'telemetry',
37+
output: 'someEvent',
38+
data: { foo: 'bar', '!err': 'oh no!' }
39+
});
40+
41+
await timeout(0);
42+
assert.strictEqual(telemetry.publicLog.callCount, 0);
43+
});
44+
45+
test('logs telemetry and exceptions when enabled', async () => {
46+
adapter.sendEventBody('output', {
47+
category: 'telemetry',
48+
output: 'someEvent',
49+
data: { foo: 'bar', '!err': 'oh no!' }
50+
});
51+
52+
await timeout(0);
53+
assert.deepStrictEqual(telemetry.publicLog.args[0], [
54+
'someEvent',
55+
{ foo: 'bar', '!err': 'oh no!' }
56+
]);
57+
});
58+
59+
test('filters exceptions when error reporting disabled', async () => {
60+
telemetry.sendErrorTelemetry = false;
61+
62+
adapter.sendEventBody('output', {
63+
category: 'telemetry',
64+
output: 'someEvent',
65+
data: { foo: 'bar', '!err': 'oh no!' }
66+
});
67+
68+
await timeout(0);
69+
assert.deepStrictEqual(telemetry.publicLog.args[0], [
70+
'someEvent',
71+
{ foo: 'bar' }
72+
]);
73+
});
74+
});

‎src/vs/workbench/services/telemetry/browser/telemetryService.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export class TelemetryService extends Disposable implements ITelemetryService {
3636
_serviceBrand: undefined;
3737

3838
private impl: ITelemetryService;
39+
public readonly sendErrorTelemetry = false;
3940

4041
constructor(
4142
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,

‎src/vs/workbench/services/telemetry/electron-browser/telemetryService.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export class TelemetryService extends Disposable implements ITelemetryService {
2424
_serviceBrand: undefined;
2525

2626
private impl: ITelemetryService;
27+
public readonly sendErrorTelemetry: boolean;
2728

2829
constructor(
2930
@IWorkbenchEnvironmentService environmentService: INativeWorkbenchEnvironmentService,
@@ -48,6 +49,8 @@ export class TelemetryService extends Disposable implements ITelemetryService {
4849
} else {
4950
this.impl = NullTelemetryService;
5051
}
52+
53+
this.sendErrorTelemetry = this.impl.sendErrorTelemetry;
5154
}
5255

5356
setEnabled(value: boolean): void {

‎src/vs/workbench/test/electron-browser/textsearch.perf.integrationTest.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ suite.skip('TextSearch performance (integration)', () => {
163163
class TestTelemetryService implements ITelemetryService {
164164
public _serviceBrand: undefined;
165165
public isOptedIn = true;
166+
public sendErrorTelemetry = true;
166167

167168
public events: any[] = [];
168169

0 commit comments

Comments
 (0)