Deprecate inExperiment API in favor of inExperimentSync (#16848)

* Deprecate inExperiment API in favour of inExperimentSync

* Ensure experiments are activated before they are used

* News entry

Author: Kartik Raj

news/2 Fixes/16768.md

@@ -0,0 +1 @@
+Fix random delay before several actions.

src/client/common/experiments/service.ts

@@ -88,30 +88,7 @@ export class ExperimentService implements IExperimentService {
     }
 
     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 manually.
-        if (this._optOutFrom.includes('All') || this._optOutFrom.includes(experiment)) {
-            return false;
-        }
-
-        if (this._optInto.includes('All') || this._optInto.includes(experiment)) {
-            // 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.
-            await this.experimentationService.getTreatmentVariableAsync(EXP_CONFIG_ID, experiment, true);
-            return true;
-        }
-
-        const treatmentVariable = await this.experimentationService.getTreatmentVariableAsync(
-            EXP_CONFIG_ID,
-            experiment,
-            true,
-        );
-        return treatmentVariable !== undefined;
+        return this.inExperimentSync(experiment);
     }
 
     public inExperimentSync(experiment: string): boolean {

src/client/extension.ts

@@ -30,7 +30,7 @@ import { ProgressLocation, ProgressOptions, window } from 'vscode';
 import { buildApi, IExtensionApi } from './api';
 import { IApplicationShell } from './common/application/types';
 import { traceError } from './common/logger';
-import { IAsyncDisposableRegistry, IExtensionContext } from './common/types';
+import { IAsyncDisposableRegistry, IExperimentService, IExtensionContext } from './common/types';
 import { createDeferred } from './common/utils/async';
 import { Common } from './common/utils/localize';
 import { activateComponents } from './extensionActivation';
@@ -103,6 +103,10 @@ async function activateUnsafe(
     // Note standard utils especially experiment and platform code are fundamental to the extension
     // and should be available before we activate anything else.Hence register them first.
     initializeStandard(ext);
+    // We need to activate experiments before initializing components as objects are created or not created based on experiments.
+    const experimentService = activatedServiceContainer.get<IExperimentService>(IExperimentService);
+    // This guarantees that all experiment information has loaded & all telemetry will contain experiment info.
+    await experimentService.activate();
     const components = await initializeComponents(ext);
 
     // Then we finish activating.

src/client/extensionActivation.ts

@@ -123,8 +123,6 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
     tensorBoardRegisterTypes(serviceManager);
 
     const experimentService = serviceContainer.get<IExperimentService>(IExperimentService);
-    // This guarantees that all experiment information has loaded & all telemetry will contain experiment info.
-    await experimentService.activate();
 
     const workspaceService = serviceContainer.get<IWorkspaceService>(IWorkspaceService);
     const extensions = serviceContainer.get<IExtensions>(IExtensions);

src/test/common/experiments/service.unit.test.ts

@@ -155,176 +155,6 @@ suite('Experimentation service', () => {
         });
     });
 
-    suite('In-experiment check', () => {
-        const experiment = 'Test Experiment - experiment';
-        let telemetryEvents: { eventName: string; properties: Record<string, unknown> }[] = [];
-        let getTreatmentVariableAsyncStub: sinon.SinonStub;
-        let sendTelemetryEventStub: sinon.SinonStub;
-
-        setup(() => {
-            sendTelemetryEventStub = sinon
-                .stub(Telemetry, 'sendTelemetryEvent')
-                .callsFake((eventName: string, _, properties: Record<string, unknown>) => {
-                    const telemetry = { eventName, properties };
-                    telemetryEvents.push(telemetry);
-                });
-
-            getTreatmentVariableAsyncStub = sinon.stub().returns(Promise.resolve(true));
-            sinon.stub(tasClient, 'getExperimentationService').returns(({
-                getTreatmentVariableAsync: getTreatmentVariableAsyncStub,
-            } as unknown) as tasClient.IExperimentationService);
-
-            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(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isTrue(result);
-            sinon.assert.notCalled(sendTelemetryEventStub);
-            sinon.assert.calledOnce(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the experiment setting is disabled, inExperiment should return false', async () => {
-            configureSettings(false, [], []);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isFalse(result);
-            sinon.assert.notCalled(sendTelemetryEventStub);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the opt-in setting contains "All", inExperiment should return true', async () => {
-            configureSettings(true, ['All'], []);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isTrue(result);
-            assert.strictEqual(telemetryEvents.length, 0);
-        });
-
-        test('If the opt-in setting contains `All`, inExperiment should check the value cached by the experiment service', async () => {
-            configureSettings(true, ['All'], []);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isTrue(result);
-            sinon.assert.notCalled(sendTelemetryEventStub);
-            sinon.assert.calledOnce(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the opt-in setting contains `All` and the experiment setting is disabled, inExperiment should return false', async () => {
-            configureSettings(false, ['All'], []);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isFalse(result);
-            sinon.assert.notCalled(sendTelemetryEventStub);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the opt-in setting contains the experiment name, inExperiment should return true', async () => {
-            configureSettings(true, [experiment], []);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isTrue(result);
-            assert.strictEqual(telemetryEvents.length, 0);
-            sinon.assert.calledOnce(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the opt-out setting contains "All", inExperiment should return false', async () => {
-            configureSettings(true, [], ['All']);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isFalse(result);
-            sinon.assert.notCalled(sendTelemetryEventStub);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the opt-out setting contains "All" and the experiment setting is enabled, inExperiment should return false', async () => {
-            configureSettings(true, [], ['All']);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isFalse(result);
-            sinon.assert.notCalled(sendTelemetryEventStub);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
-        });
-
-        test('If the opt-out setting contains the experiment name, inExperiment should return false', async () => {
-            configureSettings(true, [], [experiment]);
-
-            const experimentService = new ExperimentService(
-                instance(workspaceService),
-                instance(appEnvironment),
-                globalMemento,
-                outputChannel,
-            );
-            const result = await experimentService.inExperiment(experiment);
-
-            assert.isFalse(result);
-            assert.strictEqual(telemetryEvents.length, 0);
-            sinon.assert.notCalled(getTreatmentVariableAsyncStub);
-        });
-    });
-
     suite('In-experiment-sync check', () => {
         const experiment = 'Test Experiment - experiment';
         let telemetryEvents: { eventName: string; properties: Record<string, unknown> }[] = [];