-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add support for VS Code's experiment service #11979
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.
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
Changes from all commits
6f19e0b
bf7bd56
b5bef69
9502146
b0683b9
3271dc7
e500572
89e3935
3def3dd
821bada
168c028
cd37cba
7a37473
79f5a24
622bc96
d9ca2ca
e73f68e
c303506
9e75931
fbe5ff3
c0a2337
a9e1267
cc1f165
060c062
fc29418
fd28d4b
ccb8f67
4b2d931
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Integrate VS Code experiment framework in the extension. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| 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); | ||
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // 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; | ||
| } | ||
|
|
||
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const telemetryReporter = new ExperimentationTelemetry(); | ||
|
|
||
| this.experimentationService = getExperimentationService( | ||
| this.appEnvironment.extensionName, | ||
| this.appEnvironment.packageJson.version!, | ||
| targetPopulation, | ||
| telemetryReporter, | ||
| globalState | ||
| ); | ||
| } | ||
|
|
||
| public async inExperiment(experiment: string): Promise<boolean> { | ||
karrtikr marked this conversation as resolved.
Show resolved
Hide resolved
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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); | ||
karrtikr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| 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 { | ||
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public setSharedProperty(name: string, value: string): void { | ||
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Add the shared property to all telemetry being sent, not just events being sent by the experimentation package. | ||
| setSharedProperty(name, value); | ||
| } | ||
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 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); | ||
kimadeline marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,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) { | ||
|
|
@@ -123,6 +141,9 @@ export function sendTelemetryEvent<P extends IEventNamePropertyMapping, E extend | |
| }); | ||
| } | ||
|
|
||
| // Add shared properties to telemetry props (we may overwrite existing ones). | ||
| Object.assign(customProperties, sharedProperties); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please clarify the purpose of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's required by the telemetry library:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand, but what I meant was if you have any idea why is it required, what kind of properties are those, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The properties that get attached to your telemetry events just specify what flights the user is a part of - this allows you to compute A/B metrics based off of any of your telemetry events. Without this, you'd have to do some complicated joining or only compute metrics based off of events that have the flight info attached to them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, makes sense. Mind adding this to the property description @kimadeline ?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this would be more suited in the integration guide that I am currently writing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just that it's not clear when reading the code on what a "shared property" is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would've expected this to be addressed before merging. |
||
|
|
||
| reporter.sendTelemetryEvent(eventNameSent, customProperties, measures); | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.