From 41cb61cc270f48bc984e7229ecaa74b5ca5e0480 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Date: Tue, 2 Jun 2020 10:01:59 -0700 Subject: [PATCH] Add support for VS Code's experiment service (#11979) * Add npm package * News entry * experiments.ts -> experiments/manager.ts * Wrong issue number * eexperimentGroups -> experiments/experimentGroups * Move experiments.unit.tests.ts -> experiments/ * Commit new files * Merge gone sideways * Add types * Add opt in/out handling * Activation (service registry) * Don't pin tas-client to one version * Unit tests + fixes * Lol forgot to remove a comment + add headers * Forgot 'use strict' in service.ts * Use IApplicationEnvironment instead of IExtensions * Apply suggestions from code review Co-authored-by: Don Jayamanne * Remove unnecessary formatted props * n e v e r m i n d * Aight fixed it * flight -> experiment * Check stub calls instead of ctor impl * removed getExperimentService stub check * Set shared properties for all telemetry events * Add test for shared properties Co-authored-by: Don Jayamanne --- news/1 Enhancements/10790.md | 1 + package-lock.json | 66 +++++ package.json | 1 + src/client/common/experiments/service.ts | 91 +++++++ src/client/common/experiments/telemetry.ts | 24 ++ src/client/common/serviceRegistry.ts | 4 +- src/client/common/types.ts | 8 + src/client/telemetry/index.ts | 76 ++++-- .../common/experiments/service.unit.test.ts | 256 ++++++++++++++++++ .../common/experiments/telemetry.unit.test.ts | 62 +++++ src/test/common/installer.test.ts | 3 + src/test/common/moduleInstaller.test.ts | 3 + src/test/telemetry/index.unit.test.ts | 47 +++- 13 files changed, 611 insertions(+), 31 deletions(-) create mode 100644 news/1 Enhancements/10790.md create mode 100644 src/client/common/experiments/service.ts create mode 100644 src/client/common/experiments/telemetry.ts create mode 100644 src/test/common/experiments/service.unit.test.ts create mode 100644 src/test/common/experiments/telemetry.unit.test.ts diff --git a/news/1 Enhancements/10790.md b/news/1 Enhancements/10790.md new file mode 100644 index 000000000000..1da59c36dc77 --- /dev/null +++ b/news/1 Enhancements/10790.md @@ -0,0 +1 @@ +Integrate VS Code experiment framework in the extension. diff --git a/package-lock.json b/package-lock.json index aae95237a16e..1f33fd54cc91 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5318,6 +5318,14 @@ "resolved": "https://registry.npmjs.org/aws4/-/aws4-1.8.0.tgz", "integrity": "sha512-ReZxvNHIOv88FlT7rxcXIIC0fPt4KZqZbOlivyWtXLt8ESx84zd3kMC6iK5jVeS2qt+g7ftS7ye4fi06X5rtRQ==" }, + "axios": { + "version": "0.19.2", + "resolved": "https://registry.npmjs.org/axios/-/axios-0.19.2.tgz", + "integrity": "sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA==", + "requires": { + "follow-redirects": "1.5.10" + } + }, "azure-devops-node-api": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/azure-devops-node-api/-/azure-devops-node-api-7.2.0.tgz", @@ -10320,6 +10328,29 @@ } } }, + "follow-redirects": { + "version": "1.5.10", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.5.10.tgz", + "integrity": "sha512-0V5l4Cizzvqt5D44aTXbFZz+FtyXV1vrDN6qrelxtfYQKW0KO0W2T/hkE8xvGa/540LkZlkaUjO4ailYTFtHVQ==", + "requires": { + "debug": "=3.1.0" + }, + "dependencies": { + "debug": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", + "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "requires": { + "ms": "2.0.0" + } + }, + "ms": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", + "integrity": "sha1-VgiurfwAvmwpAd9fmGF4jeDVl8g=" + } + } + }, "font-awesome": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/font-awesome/-/font-awesome-4.7.0.tgz", @@ -22033,6 +22064,33 @@ } } }, + "tas-client": { + "version": "0.0.762", + "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.0.762.tgz", + "integrity": "sha512-i4dcYHkk2rnmBHr8RC1IoZVHU9wQT+OmDDnUeRW/vbpjDSCqgL5Qh8KeHs3DizJgqP9MWjLK/Kmqfm8VbD6g3g==", + "requires": { + "axios": "^0.19.0" + } + }, + "tcp-port-used": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/tcp-port-used/-/tcp-port-used-1.0.1.tgz", + "integrity": "sha512-rwi5xJeU6utXoEIiMvVBMc9eJ2/ofzB+7nLOdnZuFTmNCLqRiQh2sMG9MqCxHU/69VC/Fwp5dV9306Qd54ll1Q==", + "requires": { + "debug": "4.1.0", + "is2": "2.0.1" + }, + "dependencies": { + "debug": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.0.tgz", + "integrity": "sha512-heNPJUJIqC+xB6ayLAMHaIrmN9HKa7aQO8MGqKpvCA+uJYVcvR6l5kgdrhRuwPFHU7P5/A1w0BjByPHwpfTDKg==", + "requires": { + "ms": "^2.1.1" + } + } + } + }, "teeny-request": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/teeny-request/-/teeny-request-6.0.1.tgz", @@ -24620,6 +24678,14 @@ "resolved": "https://registry.npmjs.org/vscode-languageserver-types/-/vscode-languageserver-types-3.16.0-next.1.tgz", "integrity": "sha512-tZFUSbyjUcrh+qQf13ALX4QDdOfDX0cVaBFgy7ktJ0VwS7AW/yRKgGPSxVqqP9OCMNPdqP57O5q47w2pEwfaUg==" }, + "vscode-tas-client": { + "version": "0.0.757", + "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.0.757.tgz", + "integrity": "sha512-IXP+vFTIE1HpvYxqm7SuFe/K5N3cPoF4TTH6uYGAeuuYxV586tjYFtK9UmF00ajzcqvLGvsPgxmfgLeX6vUUtA==", + "requires": { + "tas-client": "0.0.762" + } + }, "vscode-test": { "version": "1.2.3", "resolved": "https://registry.npmjs.org/vscode-test/-/vscode-test-1.2.3.tgz", diff --git a/package.json b/package.json index 4b8dc7e22bb9..fa70475a33d0 100644 --- a/package.json +++ b/package.json @@ -2991,6 +2991,7 @@ "vscode-languageclient": "^6.2.0-next.2", "vscode-languageserver": "^6.2.0-next.2", "vscode-languageserver-protocol": "^3.16.0-next.2", + "vscode-tas-client": "^0.0.757", "vsls": "^0.3.1291", "winreg": "^1.2.4", "winston": "^3.2.1", diff --git a/src/client/common/experiments/service.ts b/src/client/common/experiments/service.ts new file mode 100644 index 000000000000..68e01e5b2ea3 --- /dev/null +++ b/src/client/common/experiments/service.ts @@ -0,0 +1,91 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, named } from 'inversify'; +import { Memento } from 'vscode'; +import { getExperimentationService, IExperimentationService, TargetPopulation } from 'vscode-tas-client'; +import { sendTelemetryEvent } from '../../telemetry'; +import { EventName } from '../../telemetry/constants'; +import { IApplicationEnvironment } from '../application/types'; +import { GLOBAL_MEMENTO, IConfigurationService, IExperimentService, IMemento, IPythonSettings } from '../types'; +import { ExperimentationTelemetry } from './telemetry'; + +export class ExperimentService implements IExperimentService { + /** + * Experiments the user requested to opt into manually. + */ + public _optInto: string[] = []; + /** + * Experiments the user requested to opt out from manually. + */ + public _optOutFrom: string[] = []; + + private readonly experimentationService?: IExperimentationService; + private readonly settings: IPythonSettings; + + constructor( + @inject(IConfigurationService) readonly configurationService: IConfigurationService, + @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, + @inject(IMemento) @named(GLOBAL_MEMENTO) readonly globalState: Memento + ) { + this.settings = configurationService.getSettings(undefined); + + // Users can only opt in or out of experiment groups, not control groups. + const optInto = this.settings.experiments.optInto; + const optOutFrom = this.settings.experiments.optOutFrom; + this._optInto = optInto.filter((exp) => !exp.endsWith('control')); + this._optOutFrom = optOutFrom.filter((exp) => !exp.endsWith('control')); + + // Don't initialize the experiment service if the extension's experiments setting is disabled. + const enabled = this.settings.experiments.enabled; + if (!enabled) { + return; + } + + let targetPopulation: TargetPopulation; + + if (this.appEnvironment.channel === 'insiders') { + targetPopulation = TargetPopulation.Insiders; + } else { + targetPopulation = TargetPopulation.Public; + } + + const telemetryReporter = new ExperimentationTelemetry(); + + this.experimentationService = getExperimentationService( + this.appEnvironment.extensionName, + this.appEnvironment.packageJson.version!, + targetPopulation, + telemetryReporter, + globalState + ); + } + + public async inExperiment(experiment: string): Promise { + if (!this.experimentationService) { + return false; + } + + // Currently the service doesn't support opting in and out of experiments, + // so we need to perform these checks and send the corresponding telemetry manually. + if (this._optOutFrom.includes('All') || this._optOutFrom.includes(experiment)) { + sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_OPT_IN_OUT, undefined, { + expNameOptedOutOf: experiment + }); + + return false; + } + + if (this._optInto.includes('All') || this._optInto.includes(experiment)) { + sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_OPT_IN_OUT, undefined, { + expNameOptedInto: experiment + }); + + return true; + } + + return this.experimentationService.isCachedFlightEnabled(experiment); + } +} diff --git a/src/client/common/experiments/telemetry.ts b/src/client/common/experiments/telemetry.ts new file mode 100644 index 000000000000..f6e13c9b8947 --- /dev/null +++ b/src/client/common/experiments/telemetry.ts @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { IExperimentationTelemetry } from 'vscode-tas-client'; +import { sendTelemetryEvent, setSharedProperty } from '../../telemetry'; + +export class ExperimentationTelemetry implements IExperimentationTelemetry { + public setSharedProperty(name: string, value: string): void { + // Add the shared property to all telemetry being sent, not just events being sent by the experimentation package. + setSharedProperty(name, value); + } + + public postEvent(eventName: string, properties: Map): void { + const formattedProperties: { [key: string]: string } = {}; + properties.forEach((value, key) => { + formattedProperties[key] = value; + }); + + // tslint:disable-next-line: no-any + sendTelemetryEvent(eventName as any, undefined, formattedProperties); + } +} diff --git a/src/client/common/serviceRegistry.ts b/src/client/common/serviceRegistry.ts index 18f52c0ff53a..0fd06c872ec5 100644 --- a/src/client/common/serviceRegistry.ts +++ b/src/client/common/serviceRegistry.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { IExtensionSingleActivationService } from '../activation/types'; -import { IFileDownloader, IHttpClient, IInterpreterPathService } from '../common/types'; +import { IExperimentService, IFileDownloader, IHttpClient, IInterpreterPathService } from '../common/types'; import { LiveShareApi } from '../datascience/liveshare/liveshare'; import { INotebookExecutionLogger } from '../datascience/types'; import { IServiceManager } from '../ioc/types'; @@ -40,6 +40,7 @@ import { ConfigurationService } from './configuration/service'; import { CryptoUtils } from './crypto'; import { EditorUtils } from './editor'; import { ExperimentsManager } from './experiments/manager'; +import { ExperimentService } from './experiments/service'; import { FeatureDeprecationManager } from './featureDeprecationManager'; import { ExtensionInsidersDailyChannelRule, @@ -146,6 +147,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ILiveShareApi, LiveShareApi); serviceManager.addSingleton(ICryptoUtils, CryptoUtils); serviceManager.addSingleton(IExperimentsManager, ExperimentsManager); + serviceManager.addSingleton(IExperimentService, ExperimentService); serviceManager.addSingleton(ITerminalHelper, TerminalHelper); serviceManager.addSingleton( diff --git a/src/client/common/types.ts b/src/client/common/types.ts index ff33f32e19f0..044f433562bd 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -618,6 +618,14 @@ export interface IExperimentsManager { sendTelemetryIfInExperiment(experimentName: string): void; } +/** + * Experiment service leveraging VS Code's experiment framework. + */ +export const IExperimentService = Symbol('IExperimentService'); +export interface IExperimentService { + inExperiment(experimentName: string): Promise; +} + export type InterpreterConfigurationScope = { uri: Resource; configTarget: ConfigurationTarget }; export type InspectInterpreterSettingType = { globalValue?: string; diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 89a918976ca9..ff261c18c032 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -55,6 +55,24 @@ export function isTelemetryDisabled(workspaceService: IWorkspaceService): boolea return settings.globalValue === false ? true : false; } +// Shared properties set by the IExperimentationTelemetry implementation. +const sharedProperties: Record = {}; +/** + * Set shared properties for all telemetry events. + */ +export function setSharedProperty(name: string, value: string): void { + sharedProperties[name] = value; +} + +/** + * Reset shared properties for testing purposes. + */ +export function _resetSharedProperties(): void { + for (const key of Object.keys(sharedProperties)) { + delete sharedProperties[key]; + } +} + let telemetryReporter: TelemetryReporter | undefined; function getTelemetryReporter() { if (!isTestExecution() && telemetryReporter) { @@ -95,35 +113,37 @@ export function sendTelemetryEvent

= {}; - props.stackTrace = getStackTrace(ex); - props.originalEventName = (eventName as any) as string; - reporter.sendTelemetryEvent('ERROR', props, measures); - } - const customProperties: Record = {}; - if (properties) { - // tslint:disable-next-line:prefer-type-cast no-any - 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 nothign will be sent. - // tslint:disable-next-line:prefer-type-cast no-any no-unsafe-any - (customProperties as any)[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); - } - }); + eventNameSent = 'ERROR'; + customProperties = { originalEventName: eventName as string, stackTrace: serializeStackTrace(ex) }; + reporter.sendTelemetryErrorEvent(eventNameSent, 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); + + reporter.sendTelemetryEvent(eventNameSent, customProperties, measures); } - reporter.sendTelemetryEvent((eventName as any) as string, customProperties, measures); + if (process.env && process.env.VSC_PYTHON_LOG_TELEMETRY) { traceInfo( `Telemetry Event : ${eventName} Measures: ${JSON.stringify(measures)} Props: ${JSON.stringify( diff --git a/src/test/common/experiments/service.unit.test.ts b/src/test/common/experiments/service.unit.test.ts new file mode 100644 index 000000000000..7cc26dcd0fc7 --- /dev/null +++ b/src/test/common/experiments/service.unit.test.ts @@ -0,0 +1,256 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { instance, mock, when } from 'ts-mockito'; +import * as tasClient from 'vscode-tas-client'; +import { ApplicationEnvironment } from '../../../client/common/application/applicationEnvironment'; +import { Channel, IApplicationEnvironment } from '../../../client/common/application/types'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { ExperimentService } from '../../../client/common/experiments/service'; +import { IConfigurationService } from '../../../client/common/types'; +import * as Telemetry from '../../../client/telemetry'; +import { EventName } from '../../../client/telemetry/constants'; +import { PVSC_EXTENSION_ID_FOR_TESTS } from '../../constants'; +import { MockMemento } from '../../mocks/mementos'; + +suite('Experimentation service', () => { + const extensionVersion = '1.2.3'; + + let configurationService: IConfigurationService; + let appEnvironment: IApplicationEnvironment; + let globalMemento: MockMemento; + + setup(() => { + configurationService = mock(ConfigurationService); + appEnvironment = mock(ApplicationEnvironment); + globalMemento = new MockMemento(); + }); + + teardown(() => { + sinon.restore(); + }); + + function configureSettings(enabled: boolean, optInto: string[], optOutFrom: string[]) { + when(configurationService.getSettings(undefined)).thenReturn({ + experiments: { + enabled, + optInto, + optOutFrom + } + // tslint:disable-next-line: no-any + } as any); + } + + function configureApplicationEnvironment(channel: Channel, version: string) { + when(appEnvironment.channel).thenReturn(channel); + when(appEnvironment.extensionName).thenReturn(PVSC_EXTENSION_ID_FOR_TESTS); + when(appEnvironment.packageJson).thenReturn({ version }); + } + + suite('Initialization', () => { + test('Users with a release version of the extension should be in the Public target population', () => { + const getExperimentationServiceStub = sinon.stub(tasClient, 'getExperimentationService'); + + configureSettings(true, [], []); + configureApplicationEnvironment('stable', extensionVersion); + + // tslint:disable-next-line: no-unused-expression + new ExperimentService(instance(configurationService), instance(appEnvironment), globalMemento); + + sinon.assert.calledWithExactly( + getExperimentationServiceStub, + PVSC_EXTENSION_ID_FOR_TESTS, + extensionVersion, + tasClient.TargetPopulation.Public, + sinon.match.any, + globalMemento + ); + }); + + test('Users with an Insiders version of the extension should be the Insiders target population', () => { + const getExperimentationServiceStub = sinon.stub(tasClient, 'getExperimentationService'); + + configureSettings(true, [], []); + configureApplicationEnvironment('insiders', extensionVersion); + + // tslint:disable-next-line: no-unused-expression + new ExperimentService(instance(configurationService), instance(appEnvironment), globalMemento); + + sinon.assert.calledWithExactly( + getExperimentationServiceStub, + PVSC_EXTENSION_ID_FOR_TESTS, + extensionVersion, + tasClient.TargetPopulation.Insiders, + sinon.match.any, + globalMemento + ); + }); + + test('Users can only opt into experiment groups', () => { + sinon.stub(tasClient, 'getExperimentationService'); + + configureSettings(true, ['Foo - experiment', 'Bar - control'], []); + configureApplicationEnvironment('stable', extensionVersion); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + + assert.deepEqual(experimentService._optInto, ['Foo - experiment']); + }); + + test('Users can only opt out of experiment groups', () => { + sinon.stub(tasClient, 'getExperimentationService'); + configureSettings(true, [], ['Foo - experiment', 'Bar - control']); + configureApplicationEnvironment('stable', extensionVersion); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + + assert.deepEqual(experimentService._optOutFrom, ['Foo - experiment']); + }); + }); + + suite('In-experiment check', () => { + const experiment = 'Test Experiment - experiment'; + let telemetryEvents: { eventName: string; properties: object }[] = []; + let isCachedFlightEnabledStub: sinon.SinonStub; + let sendTelemetryEventStub: sinon.SinonStub; + + setup(() => { + sendTelemetryEventStub = sinon + .stub(Telemetry, 'sendTelemetryEvent') + .callsFake((eventName: string, _, properties: object) => { + const telemetry = { eventName, properties }; + telemetryEvents.push(telemetry); + }); + + isCachedFlightEnabledStub = sinon.stub().returns(Promise.resolve(true)); + sinon.stub(tasClient, 'getExperimentationService').returns({ + isCachedFlightEnabled: isCachedFlightEnabledStub + // tslint:disable-next-line: no-any + } as any); + + configureApplicationEnvironment('stable', extensionVersion); + }); + + teardown(() => { + telemetryEvents = []; + }); + + test('If the opt-in and opt-out arrays are empty, return the value from the experimentation framework for a given experiment', async () => { + configureSettings(true, [], []); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + const result = await experimentService.inExperiment(experiment); + + assert.isTrue(result); + sinon.assert.notCalled(sendTelemetryEventStub); + sinon.assert.calledOnce(isCachedFlightEnabledStub); + }); + + test('If the experiment setting is disabled, inExperiment should return false', async () => { + configureSettings(false, [], []); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + const result = await experimentService.inExperiment(experiment); + + assert.isFalse(result); + sinon.assert.notCalled(sendTelemetryEventStub); + sinon.assert.notCalled(isCachedFlightEnabledStub); + }); + + test('If the opt-in setting contains "All", inExperiment should return true', async () => { + configureSettings(true, ['All'], []); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + const result = await experimentService.inExperiment(experiment); + + assert.isTrue(result); + assert.equal(telemetryEvents.length, 1); + assert.deepEqual(telemetryEvents[0], { + eventName: EventName.PYTHON_EXPERIMENTS_OPT_IN_OUT, + properties: { expNameOptedInto: experiment } + }); + sinon.assert.notCalled(isCachedFlightEnabledStub); + }); + + test('If the opt-in setting contains the experiment name, inExperiment should return true', async () => { + configureSettings(true, [experiment], []); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + const result = await experimentService.inExperiment(experiment); + + assert.isTrue(result); + assert.equal(telemetryEvents.length, 1); + assert.deepEqual(telemetryEvents[0], { + eventName: EventName.PYTHON_EXPERIMENTS_OPT_IN_OUT, + properties: { expNameOptedInto: experiment } + }); + sinon.assert.notCalled(isCachedFlightEnabledStub); + }); + + test('If the opt-out setting contains "All", inExperiment should return false', async () => { + configureSettings(true, [], ['All']); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + const result = await experimentService.inExperiment(experiment); + + assert.isFalse(result); + assert.equal(telemetryEvents.length, 1); + assert.deepEqual(telemetryEvents[0], { + eventName: EventName.PYTHON_EXPERIMENTS_OPT_IN_OUT, + properties: { expNameOptedOutOf: experiment } + }); + sinon.assert.notCalled(isCachedFlightEnabledStub); + }); + + test('If the opt-out setting contains the experiment name, inExperiment should return false', async () => { + configureSettings(true, [], [experiment]); + + const experimentService = new ExperimentService( + instance(configurationService), + instance(appEnvironment), + globalMemento + ); + const result = await experimentService.inExperiment(experiment); + + assert.isFalse(result); + assert.equal(telemetryEvents.length, 1); + assert.deepEqual(telemetryEvents[0], { + eventName: EventName.PYTHON_EXPERIMENTS_OPT_IN_OUT, + properties: { expNameOptedOutOf: experiment } + }); + sinon.assert.notCalled(isCachedFlightEnabledStub); + }); + }); +}); diff --git a/src/test/common/experiments/telemetry.unit.test.ts b/src/test/common/experiments/telemetry.unit.test.ts new file mode 100644 index 000000000000..d49ba5599c71 --- /dev/null +++ b/src/test/common/experiments/telemetry.unit.test.ts @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { ExperimentationTelemetry } from '../../../client/common/experiments/telemetry'; +import * as Telemetry from '../../../client/telemetry'; + +suite('Experimentation telemetry', () => { + const event = 'SomeEventName'; + + let telemetryEvents: { eventName: string; properties: object }[] = []; + let sendTelemetryEventStub: sinon.SinonStub; + let setSharedPropertyStub: sinon.SinonStub; + let experimentTelemetry: ExperimentationTelemetry; + let eventProperties: Map; + + setup(() => { + sendTelemetryEventStub = sinon + .stub(Telemetry, 'sendTelemetryEvent') + .callsFake((eventName: string, _, properties: object) => { + const telemetry = { eventName, properties }; + telemetryEvents.push(telemetry); + }); + setSharedPropertyStub = sinon.stub(Telemetry, 'setSharedProperty'); + + eventProperties = new Map(); + eventProperties.set('foo', 'one'); + eventProperties.set('bar', 'two'); + + experimentTelemetry = new ExperimentationTelemetry(); + }); + + teardown(() => { + telemetryEvents = []; + sinon.restore(); + }); + + test('Calling postEvent should send a telemetry event', () => { + experimentTelemetry.postEvent(event, eventProperties); + + sinon.assert.calledOnce(sendTelemetryEventStub); + assert.equal(telemetryEvents.length, 1); + assert.deepEqual(telemetryEvents[0], { + eventName: event, + properties: { + foo: 'one', + bar: 'two' + } + }); + }); + + test('Shared properties should be set for all telemetry events', () => { + const shared = { key: 'shared', value: 'three' }; + + experimentTelemetry.setSharedProperty(shared.key, shared.value); + + sinon.assert.calledOnce(setSharedPropertyStub); + }); +}); diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index c1f849e755a2..4fe19d92e2c9 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -30,6 +30,7 @@ import { ConfigurationService } from '../../client/common/configuration/service' import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; import { ExperimentsManager } from '../../client/common/experiments/manager'; +import { ExperimentService } from '../../client/common/experiments/service'; import { FeatureDeprecationManager } from '../../client/common/featureDeprecationManager'; import { ExtensionInsidersDailyChannelRule, @@ -102,6 +103,7 @@ import { ICryptoUtils, ICurrentProcess, IEditorUtils, + IExperimentService, IExperimentsManager, IExtensions, IFeatureDeprecationManager, @@ -253,6 +255,7 @@ suite('Installer', () => { ioc.serviceManager.addSingleton(ILiveShareApi, LiveShareApi); ioc.serviceManager.addSingleton(ICryptoUtils, CryptoUtils); ioc.serviceManager.addSingleton(IExperimentsManager, ExperimentsManager); + ioc.serviceManager.addSingleton(IExperimentService, ExperimentService); ioc.serviceManager.addSingleton(ITerminalHelper, TerminalHelper); ioc.serviceManager.addSingleton( diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index 4a6aa24ded62..cf08e46e0074 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -37,6 +37,7 @@ import { ConfigurationService } from '../../client/common/configuration/service' import { CryptoUtils } from '../../client/common/crypto'; import { EditorUtils } from '../../client/common/editor'; import { ExperimentsManager } from '../../client/common/experiments/manager'; +import { ExperimentService } from '../../client/common/experiments/service'; import '../../client/common/extensions'; import { FeatureDeprecationManager } from '../../client/common/featureDeprecationManager'; import { @@ -101,6 +102,7 @@ import { ICryptoUtils, ICurrentProcess, IEditorUtils, + IExperimentService, IExperimentsManager, IExtensions, IFeatureDeprecationManager, @@ -266,6 +268,7 @@ suite('Module Installer', () => { ioc.serviceManager.addSingleton(ILiveShareApi, LiveShareApi); ioc.serviceManager.addSingleton(ICryptoUtils, CryptoUtils); ioc.serviceManager.addSingleton(IExperimentsManager, ExperimentsManager); + ioc.serviceManager.addSingleton(IExperimentService, ExperimentService); ioc.serviceManager.addSingleton( ITerminalActivationCommandProvider, diff --git a/src/test/telemetry/index.unit.test.ts b/src/test/telemetry/index.unit.test.ts index aad20386af82..779d48fecdf6 100644 --- a/src/test/telemetry/index.unit.test.ts +++ b/src/test/telemetry/index.unit.test.ts @@ -12,7 +12,13 @@ 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 { clearTelemetryReporter, isTelemetryDisabled, sendTelemetryEvent } from '../../client/telemetry'; +import { + _resetSharedProperties, + clearTelemetryReporter, + isTelemetryDisabled, + sendTelemetryEvent, + setSharedProperty +} from '../../client/telemetry'; suite('Telemetry', () => { let workspaceService: IWorkspaceService; @@ -46,6 +52,7 @@ suite('Telemetry', () => { process.env.VSC_PYTHON_UNIT_TEST = oldValueOfVSC_PYTHON_UNIT_TEST; process.env.VSC_PYTHON_CI_TEST = oldValueOfVSC_PYTHON_CI_TEST; rewiremock.disable(); + _resetSharedProperties(); }); const testsForisTelemetryDisabled = [ @@ -94,7 +101,7 @@ suite('Telemetry', () => { expect(Reporter.measures).to.deep.equal([measures]); expect(Reporter.properties).to.deep.equal([properties]); }); - test('Send Telemetry', () => { + test('Send Telemetry with no properties', () => { rewiremock.enable(); rewiremock('vscode-extension-telemetry').with({ default: Reporter }); @@ -106,6 +113,42 @@ suite('Telemetry', () => { expect(Reporter.measures).to.deep.equal([undefined], 'Measures should be empty'); expect(Reporter.properties).to.deep.equal([{}], 'Properties should be empty'); }); + test('Send Telemetry with shared properties', () => { + rewiremock.enable(); + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measures = { start: 123, end: 987 }; + const expectedProperties = { ...properties, one: 'two' }; + + setSharedProperty('one', 'two'); + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName as any, measures, properties as any); + + expect(Reporter.eventName).to.deep.equal([eventName]); + expect(Reporter.measures).to.deep.equal([measures]); + expect(Reporter.properties).to.deep.equal([expectedProperties]); + }); + test('Shared properties will replace existing ones', () => { + rewiremock.enable(); + rewiremock('vscode-extension-telemetry').with({ default: Reporter }); + + const eventName = 'Testing'; + const properties = { hello: 'world', foo: 'bar' }; + const measures = { start: 123, end: 987 }; + const expectedProperties = { ...properties, foo: 'baz' }; + + setSharedProperty('foo', 'baz'); + + // tslint:disable-next-line:no-any + sendTelemetryEvent(eventName as any, measures, properties as any); + + expect(Reporter.eventName).to.deep.equal([eventName]); + 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');