Skip to content

Commit

Permalink
Add support for VS Code's experiment service (microsoft#11979)
Browse files Browse the repository at this point in the history
* 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 <don.jayamanne@yahoo.com>
* 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 <don.jayamanne@yahoo.com>
  • Loading branch information
2 people authored and karthiknadig committed Jun 5, 2020
1 parent 832bd4c commit 41cb61c
Show file tree
Hide file tree
Showing 13 changed files with 611 additions and 31 deletions.
1 change: 1 addition & 0 deletions news/1 Enhancements/10790.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Integrate VS Code experiment framework in the extension.
66 changes: 66 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
91 changes: 91 additions & 0 deletions src/client/common/experiments/service.ts
Original file line number Diff line number Diff line change
@@ -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<boolean> {
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);
}
}
24 changes: 24 additions & 0 deletions src/client/common/experiments/telemetry.ts
Original file line number Diff line number Diff line change
@@ -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<string, string>): 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);
}
}
4 changes: 3 additions & 1 deletion src/client/common/serviceRegistry.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -146,6 +147,7 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<ILiveShareApi>(ILiveShareApi, LiveShareApi);
serviceManager.addSingleton<ICryptoUtils>(ICryptoUtils, CryptoUtils);
serviceManager.addSingleton<IExperimentsManager>(IExperimentsManager, ExperimentsManager);
serviceManager.addSingleton<IExperimentService>(IExperimentService, ExperimentService);

serviceManager.addSingleton<ITerminalHelper>(ITerminalHelper, TerminalHelper);
serviceManager.addSingleton<ITerminalActivationCommandProvider>(
Expand Down
8 changes: 8 additions & 0 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>;
}

export type InterpreterConfigurationScope = { uri: Resource; configTarget: ConfigurationTarget };
export type InspectInterpreterSettingType = {
globalValue?: string;
Expand Down
76 changes: 48 additions & 28 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {};
/**
* 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) {
Expand Down Expand Up @@ -95,35 +113,37 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend
// 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.
const props: Record<string, string> = {};
props.stackTrace = getStackTrace(ex);
props.originalEventName = (eventName as any) as string;
reporter.sendTelemetryEvent('ERROR', props, measures);
}
const customProperties: Record<string, string> = {};
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(
Expand Down
Loading

0 comments on commit 41cb61c

Please sign in to comment.