diff --git a/package-lock.json b/package-lock.json index 75c18ebefb7..672a8ad5506 100644 --- a/package-lock.json +++ b/package-lock.json @@ -82,7 +82,7 @@ "vscode-languageclient": "8.0.2-next.5", "vscode-languageserver": "8.0.2-next.5", "vscode-languageserver-protocol": "3.17.2-next.6", - "vscode-tas-client": "^0.1.27", + "vscode-tas-client": "^0.1.63", "ws": "^6.2.2", "zeromq": "^6.0.0-beta.16", "zeromqold": "npm:zeromq@^6.0.0-beta.6" @@ -6194,11 +6194,11 @@ } }, "node_modules/axios": { - "version": "0.25.0", - "resolved": "https://registry.npmjs.org/axios/-/axios-0.25.0.tgz", - "integrity": "sha512-cD8FOb0tRH3uuEe6+evtAbgJtfxr7ly3fQjYcMcuPlgkwVS9xboaVIpcDV+cYQe+yGykgwZCs1pzjntcGa6l5g==", + "version": "0.26.1", + "resolved": "https://registry.npmjs.org/axios/-/axios-0.26.1.tgz", + "integrity": "sha512-fPwcX4EvnSHuInCMItEhAGnaSEXRBjtzh9fOtsE6E1G6p7vl7edEeZe11QHf18+6+9gR5PbKV/sGKNaD8YaMeA==", "dependencies": { - "follow-redirects": "^1.14.7" + "follow-redirects": "^1.14.8" } }, "node_modules/axobject-query": { @@ -11069,9 +11069,9 @@ "dev": true }, "node_modules/follow-redirects": { - "version": "1.14.9", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.14.9.tgz", - "integrity": "sha512-MQDfihBQYMcyy5dhRDJUHcw7lb2Pv/TuE6xP1vyraLukNDHKbDxDNaOE3NbCAdKQApno+GPRyo1YAp89yCjK4w==", + "version": "1.15.2", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz", + "integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==", "funding": [ { "type": "individual", @@ -21442,11 +21442,11 @@ } }, "node_modules/tas-client": { - "version": "0.1.30", - "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.1.30.tgz", - "integrity": "sha512-4JINL2r0m7UWU8nIsfWI97SPxjnqz6nk0JtS5ajL5SCksPK8Yjx9yik4DbwwbsbXLJu9C2jasEKnDY40UxEh1g==", + "version": "0.1.58", + "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.1.58.tgz", + "integrity": "sha512-fOWii4wQXuo9Zl0oXgvjBzZWzKc5MmUR6XQWX93WU2c1SaP1plPo/zvXP8kpbZ9fvegFOHdapszYqMTRq/SRtg==", "dependencies": { - "axios": "^0.25.0" + "axios": "^0.26.1" } }, "node_modules/tcp-port-used": { @@ -23329,11 +23329,11 @@ "integrity": "sha512-TiAkLABgqkVWdAlC3XlOfdhdjIAdVU4YntPUm9kKGbXr+MGwpVnKz2KZMNBcvG0CFx8Hi8qliL0iq+ndPB720w==" }, "node_modules/vscode-tas-client": { - "version": "0.1.31", - "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.1.31.tgz", - "integrity": "sha512-GbvRil0TFdWtG0hvROi/haZkuKNkC/aVjhXWNCKBnS5VwpKtTRnt+o9M5FSETkW7dhfDTK/Jmv53372WtLnVSA==", + "version": "0.1.63", + "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.1.63.tgz", + "integrity": "sha512-TY5TPyibzi6rNmuUB7eRVqpzLzNfQYrrIl/0/F8ukrrbzOrKVvS31hM3urE+tbaVrnT+TMYXL16GhX57vEowhA==", "dependencies": { - "tas-client": "0.1.30" + "tas-client": "0.1.58" }, "engines": { "vscode": "^1.19.1" @@ -28904,11 +28904,11 @@ "dev": true }, "axios": { - "version": "0.25.0", - "resolved": "https://registry.npmjs.org/axios/-/axios-0.25.0.tgz", - "integrity": "sha512-cD8FOb0tRH3uuEe6+evtAbgJtfxr7ly3fQjYcMcuPlgkwVS9xboaVIpcDV+cYQe+yGykgwZCs1pzjntcGa6l5g==", + "version": "0.26.1", + "resolved": "https://registry.npmjs.org/axios/-/axios-0.26.1.tgz", + "integrity": "sha512-fPwcX4EvnSHuInCMItEhAGnaSEXRBjtzh9fOtsE6E1G6p7vl7edEeZe11QHf18+6+9gR5PbKV/sGKNaD8YaMeA==", "requires": { - "follow-redirects": "^1.14.7" + "follow-redirects": "^1.14.8" } }, "axobject-query": { @@ -32812,9 +32812,9 @@ } }, "follow-redirects": { - "version": "1.14.9", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.14.9.tgz", - "integrity": "sha512-MQDfihBQYMcyy5dhRDJUHcw7lb2Pv/TuE6xP1vyraLukNDHKbDxDNaOE3NbCAdKQApno+GPRyo1YAp89yCjK4w==" + "version": "1.15.2", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz", + "integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==" }, "fontkit": { "version": "1.8.1", @@ -40865,11 +40865,11 @@ } }, "tas-client": { - "version": "0.1.30", - "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.1.30.tgz", - "integrity": "sha512-4JINL2r0m7UWU8nIsfWI97SPxjnqz6nk0JtS5ajL5SCksPK8Yjx9yik4DbwwbsbXLJu9C2jasEKnDY40UxEh1g==", + "version": "0.1.58", + "resolved": "https://registry.npmjs.org/tas-client/-/tas-client-0.1.58.tgz", + "integrity": "sha512-fOWii4wQXuo9Zl0oXgvjBzZWzKc5MmUR6XQWX93WU2c1SaP1plPo/zvXP8kpbZ9fvegFOHdapszYqMTRq/SRtg==", "requires": { - "axios": "^0.25.0" + "axios": "^0.26.1" } }, "tcp-port-used": { @@ -42377,11 +42377,11 @@ "integrity": "sha512-TiAkLABgqkVWdAlC3XlOfdhdjIAdVU4YntPUm9kKGbXr+MGwpVnKz2KZMNBcvG0CFx8Hi8qliL0iq+ndPB720w==" }, "vscode-tas-client": { - "version": "0.1.31", - "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.1.31.tgz", - "integrity": "sha512-GbvRil0TFdWtG0hvROi/haZkuKNkC/aVjhXWNCKBnS5VwpKtTRnt+o9M5FSETkW7dhfDTK/Jmv53372WtLnVSA==", + "version": "0.1.63", + "resolved": "https://registry.npmjs.org/vscode-tas-client/-/vscode-tas-client-0.1.63.tgz", + "integrity": "sha512-TY5TPyibzi6rNmuUB7eRVqpzLzNfQYrrIl/0/F8ukrrbzOrKVvS31hM3urE+tbaVrnT+TMYXL16GhX57vEowhA==", "requires": { - "tas-client": "0.1.30" + "tas-client": "0.1.58" } }, "vscode-uri": { diff --git a/package.json b/package.json index 8da571a3b22..83a653c39c3 100644 --- a/package.json +++ b/package.json @@ -2082,7 +2082,7 @@ "vscode-languageclient": "8.0.2-next.5", "vscode-languageserver": "8.0.2-next.5", "vscode-languageserver-protocol": "3.17.2-next.6", - "vscode-tas-client": "^0.1.27", + "vscode-tas-client": "^0.1.63", "ws": "^6.2.2", "zeromq": "^6.0.0-beta.16", "zeromqold": "npm:zeromq@^6.0.0-beta.6" diff --git a/src/extension.node.ts b/src/extension.node.ts index ae5630af947..2cdabc4c5ef 100644 --- a/src/extension.node.ts +++ b/src/extension.node.ts @@ -334,7 +334,6 @@ async function activateLegacy( const experimentService = serviceContainer.get(IExperimentService); // This must be done first, this guarantees all experiment information has loaded & all telemetry will contain experiment info. await experimentService.activate(); - experimentService.logExperiments(); const applicationEnv = serviceManager.get(IApplicationEnvironment); const configuration = serviceManager.get(IConfigurationService); diff --git a/src/extension.web.ts b/src/extension.web.ts index b1343d6a781..ab99be3d1b4 100644 --- a/src/extension.web.ts +++ b/src/extension.web.ts @@ -315,7 +315,6 @@ async function activateLegacy( const experimentService = serviceContainer.get(IExperimentService); // This must be done first, this guarantees all experiment information has loaded & all telemetry will contain experiment info. await experimentService.activate(); - experimentService.logExperiments(); const applicationEnv = serviceManager.get(IApplicationEnvironment); const configuration = serviceManager.get(IConfigurationService); diff --git a/src/kernels/jupyter/finder/remoteKernelFinder.ts b/src/kernels/jupyter/finder/remoteKernelFinder.ts index 4e03e04fbc1..51bef5e5924 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinder.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinder.ts @@ -271,8 +271,6 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable { private async getFromCache(cancelToken?: CancellationToken): Promise { try { - traceVerbose('UniversalRemoteKernelFinder: get from cache'); - let results: RemoteKernelConnectionMetadata[] = this.cache; const key = this.cacheKey; diff --git a/src/kernels/raw/finder/interpreterKernelSpecFinderHelper.node.ts b/src/kernels/raw/finder/interpreterKernelSpecFinderHelper.node.ts index 8d0ab2966b4..e50ffaab326 100644 --- a/src/kernels/raw/finder/interpreterKernelSpecFinderHelper.node.ts +++ b/src/kernels/raw/finder/interpreterKernelSpecFinderHelper.node.ts @@ -95,7 +95,9 @@ export async function findKernelSpecsInInterpreter( } }); results = results.filter((r) => !r.specFile || !originalSpecFiles.has(r.specFile)); - traceVerbose(`Kernel Specs found in interpreter ${interpreter.id} are ${JSON.stringify(results)}`); + if (results.length) { + traceVerbose(`Kernel Specs found in interpreter ${interpreter.id} are ${JSON.stringify(results)}`); + } // There was also an old bug where the same item would be registered more than once. Eliminate these dupes // too. const uniqueKernelSpecs: IJupyterKernelSpec[] = []; @@ -169,7 +171,7 @@ export class InterpreterSpecificKernelSpecsFinder implements IDisposable { private async listKernelSpecsImpl() { const cancelToken = this.cancelToken.token; - traceVerbose(`Listing Python & non-Python kernels for Interpreter ${getDisplayPath(this.interpreter.uri)}`); + traceVerbose(`Search for KernelSpecs in Interpreter ${getDisplayPath(this.interpreter.uri)}`); const [kernelSpecsBelongingToPythonEnvironment, tempDirForKernelSpecs] = await Promise.all([ findKernelSpecsInInterpreter(this.interpreter, cancelToken, this.jupyterPaths, this.kernelSpecFinder), this.jupyterPaths.getKernelSpecTempRegistrationFolder() @@ -215,7 +217,6 @@ export class InterpreterSpecificKernelSpecsFinder implements IDisposable { id: getKernelId(k, this.interpreter) }); - traceVerbose(`Found kernel spec at end of discovery ${kernelSpec?.id}`); // Check if we have already seen this. if (kernelSpec && !distinctKernelMetadata.has(kernelSpec.id)) { distinctKernelMetadata.set(kernelSpec.id, kernelSpec); @@ -233,7 +234,6 @@ export class InterpreterSpecificKernelSpecsFinder implements IDisposable { interpreter: this.interpreter, id: getKernelId(spec, this.interpreter) }); - traceVerbose(`Kernel for interpreter ${this.interpreter.id} is ${result.id}`); if (!distinctKernelMetadata.has(result.id)) { distinctKernelMetadata.set(result.id, result); } diff --git a/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts b/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts index 7784ff0aa90..d55dd435806 100644 --- a/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts +++ b/src/kernels/raw/finder/localKernelSpecFinderBase.node.ts @@ -155,7 +155,6 @@ export class LocalKernelSpecFinder implements IDisposable { const files = await this.fs.searchLocal(`**/kernel.json`, kernelSearchPath.fsPath, true); return files.map((item) => uriPath.joinPath(kernelSearchPath, item)); } else { - traceVerbose(`Not Searching for kernels as path does not exist, ${getDisplayPath(kernelSearchPath)}`); return []; } })(); @@ -320,8 +319,8 @@ export async function loadKernelSpec( let kernelJson: ReadWrite; try { traceVerbose( - `Loading kernelspec from ${getDisplayPath(specPath)} for ${ - interpreter?.uri ? getDisplayPath(interpreter.uri) : '' + `Loading kernelspec from ${getDisplayPath(specPath)} ${ + interpreter?.uri ? `for ${getDisplayPath(interpreter.uri)}` : '' }` ); kernelJson = JSON.parse(await fs.readFile(specPath)); diff --git a/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts b/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts index df5e979fd3e..a3e37173635 100644 --- a/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts +++ b/src/kernels/raw/finder/localPythonAndRelatedNonPythonKernelSpecFinder.node.ts @@ -86,7 +86,6 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS this.disposables.push(this._onDidChangeKernels); interpreterService.onDidChangeInterpreters( () => { - traceVerbose(`refreshData after detecting changes to interpreters`); this.refreshCancellation?.cancel(); this.refreshData().catch(noop); }, diff --git a/src/platform/common/experiments/groups.ts b/src/platform/common/experiments/groups.ts deleted file mode 100644 index 89c2d4f5159..00000000000 --- a/src/platform/common/experiments/groups.ts +++ /dev/null @@ -1,4 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -export enum Experiments {} diff --git a/src/platform/common/experiments/service.ts b/src/platform/common/experiments/service.ts index 371ecd357b3..ed0045dc2c5 100644 --- a/src/platform/common/experiments/service.ts +++ b/src/platform/common/experiments/service.ts @@ -4,24 +4,18 @@ import { inject, injectable, named } from 'inversify'; import { Memento } from 'vscode'; import { getExperimentationService, IExperimentationService, TargetPopulation } from 'vscode-tas-client'; -import { IApplicationEnvironment } from '../application/types'; -import { JVSC_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../constants'; -import { traceVerbose } from '../../logging'; -import { - GLOBAL_MEMENTO, - IConfigurationService, - IExperimentService, - IJupyterSettings, - IMemento, - IOutputChannel -} from '../types'; +import { IApplicationEnvironment, IWorkspaceService } from '../application/types'; +import { JVSC_EXTENSION_ID } from '../constants'; +import { traceInfo, traceVerbose } from '../../logging'; +import { GLOBAL_MEMENTO, IConfigurationService, IExperimentService, IJupyterSettings, IMemento } from '../types'; import { Experiments } from '../utils/localize'; -import { Experiments as ExperimentGroups } from './groups'; +import { Experiments as ExperimentGroups } from '../types'; import { ExperimentationTelemetry } from './telemetry.node'; // This is a hacky way to determine what experiments have been loaded by the Experiments service. // There's no public API yet, hence we access the global storage that is updated by the experiments package. const EXP_MEMENTO_KEY = 'VSCode.ABExp.FeatureData'; +const EXP_CONFIG_ID = 'vscode'; /** * Exposes an api to determine what experiments are in use. Experiments are generally feature flags that can be used to try out different features for a subset of users. @@ -40,16 +34,15 @@ export class ExperimentService implements IExperimentService { private readonly experimentationService?: IExperimentationService; private readonly settings: IJupyterSettings; - private logged?: boolean; private get enabled() { - return this.settings.experiments.enabled; + return this.settings.experiments.enabled && !this.settings.experiments.optOutFrom.includes('All'); } constructor( @inject(IConfigurationService) readonly configurationService: IConfigurationService, + @inject(IWorkspaceService) readonly workspace: IWorkspaceService, @inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment, - @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento, - @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel + @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento ) { this.settings = configurationService.getSettings(undefined); @@ -81,79 +74,119 @@ export class ExperimentService implements IExperimentService { telemetryReporter, this.globalState ); - - traceVerbose(`Experimentation service retrieved: ${this.experimentationService}`); - - this.logExperiments(); } public async activate() { - if (this.experimentationService) { + if (this.experimentationService && this.enabled) { + traceVerbose(`Experimentation service retrieved: ${this.experimentationService}`); await this.experimentationService.initializePromise; + this.logExperiments(); } } public async inExperiment(experiment: ExperimentGroups): Promise { - if (!this.experimentationService) { + return this.inExperimentSync(experiment); + } + public inExperimentSync(experiment: ExperimentGroups): boolean { + if (!this.experimentationService || !this.enabled) { 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. - switch (this.getOptInOptOutStatus(experiment)) { - case 'optOut': { - return false; - } - case 'optIn': { - await this.experimentationService.isCachedFlightEnabled(experiment.toString()); - return true; - } - - default: - return this.experimentationService.isCachedFlightEnabled(experiment.toString()); + if (this._optOutFrom.includes(experiment.toString())) { + return false; } - } - public async getExperimentValue(experiment: string): Promise { - if (!this.experimentationService || this._optOutFrom.includes(experiment)) { - return; + if (this._optInto.includes(experiment.toString()) || this._optInto.includes('All')) { + // Check if the user was already in the experiment server-side. We need to do + // this to ensure the experiment service is ready and internal states are fully + // synced with the experiment server. + this.experimentationService.getTreatmentVariable(EXP_CONFIG_ID, experiment as unknown as string); + return true; } + // If getTreatmentVariable returns undefined, + // it means that the value for this experiment was not found on the server. - return this.experimentationService.getTreatmentVariableAsync('vscode', experiment); + const treatmentVariable = this.experimentationService.getTreatmentVariable( + EXP_CONFIG_ID, + experiment as unknown as string + ); + return treatmentVariable === true; } - public logExperiments() { - if (!this.experimentationService || this.logged) { + + public async getExperimentValue( + experiment: ExperimentGroups + ): Promise { + if ( + !this.experimentationService || + !this.enabled || + this._optOutFrom.includes(experiment as unknown as string) + ) { return; } - this.logged = true; - const experiments = this.globalState.get<{ features: string[] }>(EXP_MEMENTO_KEY, { features: [] }); - experiments.features.forEach((exp) => { - // Filter out experiments groups that are not from the Python extension. - if (exp.toLowerCase().startsWith('jupyter')) { - this.output.appendLine(Experiments.inGroup(exp)); - } - }); - this.getExperimentsUserHasManuallyOptedInto().forEach((exp) => { - this.output.appendLine(Experiments.inGroup(exp.toString())); - }); + + return this.experimentationService.getTreatmentVariable(EXP_CONFIG_ID, experiment as unknown as string); } - private getOptInOptOutStatus(experiment: ExperimentGroups): 'optOut' | 'optIn' | undefined { - if (!this.experimentationService) { + private logExperiments() { + const telemetrySettings = this.workspace.getConfiguration('telemetry'); + let experimentsDisabled = false; + if (telemetrySettings && telemetrySettings.get('enableTelemetry') === false) { + traceInfo('Telemetry is disabled'); + experimentsDisabled = true; + } + + if (telemetrySettings && telemetrySettings.get('telemetryLevel') === 'off') { + traceInfo('Telemetry level is off'); + experimentsDisabled = true; + } + + if (experimentsDisabled) { + traceInfo('Experiments are disabled, only manually opted experiments are active.'); + } + + if (this._optOutFrom.includes('All')) { + // We prioritize opt out first + // Since we are in the Opt Out all case, this means when checking for experiment we + // short circuit and return. So, printing out additional experiment info might cause + // confusion. So skip printing out any specific experiment details to the log. return; } + if (this._optInto.includes('All')) { + // Only if 'All' is not in optOut then check if it is in Opt In. + traceInfo(Experiments.inGroup('All')); - // 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(experiment.toString())) { - return 'optOut'; + // Similar to the opt out case. If user is opting into to all experiments we short + // circuit the experiment checks. So, skip printing any additional details to the logs. + return; } - if (this._optInto.includes(experiment.toString())) { - return 'optIn'; + // Log experiments that users manually opt out, these are experiments which are added using the exp framework. + this._optOutFrom + .filter((exp) => exp !== 'All' && exp.toLowerCase().startsWith('python')) + .forEach((exp) => { + traceInfo(Experiments.notInGroup(exp)); + }); + + // Log experiments that users manually opt into, these are experiments which are added using the exp framework. + this._optInto + .filter((exp) => exp !== 'All' && exp.toLowerCase().startsWith('python')) + .forEach((exp) => { + traceInfo(Experiments.inGroup(exp)); + }); + + if (!experimentsDisabled) { + // Log experiments that users are added to by the exp framework + this.globalState.get<{ features: string[] }>(EXP_MEMENTO_KEY, { features: [] }).features.forEach((exp) => { + // Filter out experiment groups that are not from the Python extension. + // Filter out experiment groups that are not already opted out or opted into. + if ( + exp.toLowerCase().startsWith('jupyter') && + !this._optOutFrom.includes(exp) && + !this._optInto.includes(exp) + ) { + traceInfo(Experiments.inGroup(exp)); + } + }); } } - private getExperimentsUserHasManuallyOptedInto(): ExperimentGroups[] { - return Object.values(ExperimentGroups).filter( - (experiment) => this.getOptInOptOutStatus(experiment as unknown as ExperimentGroups) === 'optIn' - ) as unknown as ExperimentGroups[]; - } } diff --git a/src/platform/common/experiments/service.unit.test.ts b/src/platform/common/experiments/service.unit.test.ts index d0849a6cf7b..cebac9b0f97 100644 --- a/src/platform/common/experiments/service.unit.test.ts +++ b/src/platform/common/experiments/service.unit.test.ts @@ -13,22 +13,20 @@ import { ExperimentService } from './service'; import { IConfigurationService } from '../types'; import * as Telemetry from '../../telemetry/index'; import { JVSC_EXTENSION_ID_FOR_TESTS } from '../../../test/constants.node'; -import { MockOutputChannel } from '../../../test/mockClasses'; import { MockMemento } from '../../../test/mocks/mementos'; +import { Experiments } from '../types'; suite('Experimentation service', () => { const extensionVersion = '1.2.3'; let configurationService: IConfigurationService; let appEnvironment: IApplicationEnvironment; let globalMemento: MockMemento; - let outputChannel: MockOutputChannel; let workspace: IWorkspaceService; setup(() => { configurationService = mock(ConfigurationService); appEnvironment = mock(ApplicationEnvironment); globalMemento = new MockMemento(); - outputChannel = new MockOutputChannel(''); workspace = mock(WorkspaceService); when(workspace.getConfiguration(anything(), anything())).thenReturn({ get: () => [], @@ -70,9 +68,9 @@ suite('Experimentation service', () => { // eslint-disable-next-line @typescript-eslint/no-unused-expressions new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); sinon.assert.calledWithExactly( @@ -94,9 +92,9 @@ suite('Experimentation service', () => { // eslint-disable-next-line @typescript-eslint/no-unused-expressions new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); sinon.assert.calledWithExactly( @@ -117,9 +115,9 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); assert.deepEqual(experimentService._optInto, ['Foo - experiment']); @@ -132,9 +130,9 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); assert.deepEqual(experimentService._optOutFrom, ['Foo - experiment']); @@ -145,7 +143,6 @@ suite('Experimentation service', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const experiment: any = 'Test Experiment - experiment'; let telemetryEvents: { eventName: string; properties: object | undefined }[] = []; - let isCachedFlightEnabledStub: sinon.SinonStub; let sendTelemetryEventStub: sinon.SinonStub; setup(() => { @@ -156,9 +153,8 @@ suite('Experimentation service', () => { telemetryEvents.push(telemetry); }); - isCachedFlightEnabledStub = sinon.stub().returns(Promise.resolve(true)); sinon.stub(tasClient, 'getExperimentationService').returns({ - isCachedFlightEnabled: isCachedFlightEnabledStub + getTreatmentVariable: () => true // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); @@ -174,15 +170,14 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + 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 () => { @@ -190,15 +185,14 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + 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 the experiment name, inExperiment should return true', async () => { @@ -206,14 +200,13 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); const result = await experimentService.inExperiment(experiment); assert.isTrue(result); - sinon.assert.calledOnce(isCachedFlightEnabledStub); }); test('If the opt-out setting contains the experiment name, inExperiment should return false', async () => { @@ -221,25 +214,22 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); const result = await experimentService.inExperiment(experiment); assert.isFalse(result); - sinon.assert.notCalled(isCachedFlightEnabledStub); }); }); suite('Experiment value retrieval', () => { - const experiment = 'Test Experiment - experiment'; - let getTreatmentVariableAsyncStub: sinon.SinonStub; + const experiment = 'Test Experiment - experiment' as unknown as Experiments; setup(() => { - getTreatmentVariableAsyncStub = sinon.stub().returns(Promise.resolve('value')); sinon.stub(tasClient, 'getExperimentationService').returns({ - getTreatmentVariableAsync: getTreatmentVariableAsyncStub + getTreatmentVariable: () => 'value' // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); @@ -251,14 +241,13 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); const result = await experimentService.getExperimentValue(experiment); assert.equal(result, 'value'); - sinon.assert.calledOnce(getTreatmentVariableAsyncStub); }); test('If the experiment setting is disabled, getExperimentValue should return undefined', async () => { @@ -266,29 +255,27 @@ suite('Experimentation service', () => { const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); const result = await experimentService.getExperimentValue(experiment); assert.isUndefined(result); - sinon.assert.notCalled(getTreatmentVariableAsyncStub); }); test('If the opt-out setting contains the experiment name, igetExperimentValue should return undefined', async () => { - configureSettings(true, [], [experiment]); + configureSettings(true, [], [experiment as any]); const experimentService = new ExperimentService( instance(configurationService), + instance(workspace), instance(appEnvironment), - globalMemento, - outputChannel + globalMemento ); const result = await experimentService.getExperimentValue(experiment); assert.isUndefined(result); - sinon.assert.notCalled(getTreatmentVariableAsyncStub); }); }); }); diff --git a/src/platform/common/types.ts b/src/platform/common/types.ts index 23f00050f47..362b9ebc7a1 100644 --- a/src/platform/common/types.ts +++ b/src/platform/common/types.ts @@ -6,7 +6,6 @@ import { ConfigurationTarget, Disposable, Event, Extension, ExtensionContext, Ou import { PythonEnvironment } from '../pythonEnvironments/info'; import { CommandIds } from '../../commands'; import { ICommandManager } from './application/types'; -import { Experiments } from './experiments/groups'; import { ISystemVariables } from './variables/types'; export const IsCodeSpace = Symbol('IsCodeSpace'); @@ -295,6 +294,8 @@ export interface IAsyncDisposableRegistry extends IAsyncDisposable { push(disposable: IDisposable | IAsyncDisposable): void; } +export enum Experiments {} + /** * Experiment service leveraging VS Code's experiment framework. */ @@ -302,8 +303,8 @@ export const IExperimentService = Symbol('IExperimentService'); export interface IExperimentService { activate(): Promise; inExperiment(experimentName: Experiments): Promise; - getExperimentValue(experimentName: string): Promise; - logExperiments(): void; + inExperimentSync(experimentName: Experiments): boolean; + getExperimentValue(experimentName: Experiments): Promise; } export type InterpreterUri = Resource | PythonEnvironment; diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts index c5939b92774..91aa0efb255 100644 --- a/src/platform/common/utils/localize.ts +++ b/src/platform/common/utils/localize.ts @@ -35,6 +35,8 @@ export namespace Common { } export namespace Experiments { + export const notInGroup = (groupName: string) => + l10n.t("User does not belong to experiment group '{0}'", groupName); export const inGroup = (groupName: string) => l10n.t("User belongs to experiment group '{0}'", groupName); } export namespace OutputChannelNames { diff --git a/src/platform/interpreter/environmentActivationService.node.ts b/src/platform/interpreter/environmentActivationService.node.ts index e104a6d2e82..965ffcfd7f2 100644 --- a/src/platform/interpreter/environmentActivationService.node.ts +++ b/src/platform/interpreter/environmentActivationService.node.ts @@ -109,11 +109,6 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi if (token?.isCancellationRequested) { return; } - traceVerbose( - `Got env vars with python ${getDisplayPath(interpreter?.uri)} in ${stopWatch.elapsedTime}ms with ${ - Object.keys(env || {}).length - } variables` - ); return env; }) .catch((ex) => {