Improve opt-in/out telemetry (#15946)

* Add telemetry event declaration
* Send telemetry
* Add tests

Author: kimadeline

src/client/common/experiments/service.ts

@@ -6,6 +6,8 @@
 import { inject, injectable, 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, IWorkspaceService } from '../application/types';
 import { PVSC_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../constants';
 import { GLOBAL_MEMENTO, IExperimentService, IMemento, IOutputChannel } from '../types';
@@ -81,6 +83,7 @@ export class ExperimentService implements IExperimentService {
             await this.experimentationService.initializePromise;
             await this.experimentationService.initialFetch;
         }
+        sendOptInOptOutTelemetry(this._optInto, this._optOutFrom, this.appEnvironment.packageJson);
     }
 
     public async inExperiment(experiment: string): Promise<boolean> {
@@ -162,3 +165,47 @@ export class ExperimentService implements IExperimentService {
         });
     }
 }
+
+/**
+ * Read accepted experiment settings values from the extension's package.json.
+ * This function assumes that the `setting` argument is a string array that has a specific set of accepted values.
+ *
+ * Accessing the values is done via these keys:
+ * <root> -> "contributes" -> "configuration" -> "properties" -> <setting name> -> "items" -> "enum"
+ *
+ * @param setting The setting we want to read the values of.
+ * @param packageJson The content of `package.json`, as a JSON object.
+ *
+ * @returns An array containing all accepted values for the setting, or [] if there were none.
+ */
+function readEnumValues(setting: string, packageJson: Record<string, unknown>): string[] {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const settingProperties = (packageJson.contributes as any).configuration.properties[setting];
+
+    if (settingProperties) {
+        return settingProperties.items.enum ?? [];
+    }
+
+    return [];
+}
+
+/**
+ * Send telemetry on experiments that have been manually opted into or opted-out from.
+ * The telemetry will only contain values that are present in the list of accepted values for these settings.
+ *
+ * @param optedIn The list of experiments opted into.
+ * @param optedOut The list of experiments opted out from.
+ * @param packageJson The content of `package.json`, as a JSON object.
+ */
+function sendOptInOptOutTelemetry(optedIn: string[], optedOut: string[], packageJson: Record<string, unknown>): void {
+    const optedInEnumValues = readEnumValues('python.experiments.optInto', packageJson);
+    const optedOutEnumValues = readEnumValues('python.experiments.optOutFrom', packageJson);
+
+    const sanitizedOptedIn = optedIn.filter((exp) => optedInEnumValues.includes(exp));
+    const sanitizedOptedOut = optedOut.filter((exp) => optedOutEnumValues.includes(exp));
+
+    sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS, undefined, {
+        optedInto: sanitizedOptedIn,
+        optedOutFrom: sanitizedOptedOut,
+    });
+}

src/client/telemetry/constants.ts

@@ -76,6 +76,7 @@ export enum EventName {
     PYTHON_EXPERIMENTS = 'PYTHON_EXPERIMENTS',
     PYTHON_EXPERIMENTS_DISABLED = 'PYTHON_EXPERIMENTS_DISABLED',
     PYTHON_EXPERIMENTS_DOWNLOAD_SUCCESS_RATE = 'PYTHON_EXPERIMENTS_DOWNLOAD_SUCCESS_RATE',
+    PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS = 'PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS',
     PYTHON_WEB_APP_RELOAD = 'PYTHON_WEB_APP.RELOAD',
     EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT',
     ACTIVATION_TIP_PROMPT = 'ACTIVATION_TIP_PROMPT',

src/client/telemetry/index.ts

@@ -1274,6 +1274,21 @@ export interface IEventNamePropertyMapping {
          */
         error?: string;
     };
+    /**
+     * Telemetry event sent once on session start with details on which experiments are opted into and opted out from.
+     */
+    [EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS]: {
+        /**
+         * List of valid experiments in the python.experiments.optInto setting
+         * @type {string[]}
+         */
+        optedInto: string[];
+        /**
+         * List of valid experiments in the python.experiments.optOutFrom setting
+         * @type {string[]}
+         */
+        optedOutFrom: string[];
+    };
     /**
      * Telemetry event sent when LS is started for workspace (workspace folder in case of multi-root)
      */

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

@@ -14,6 +14,7 @@ import { WorkspaceService } from '../../../client/common/application/workspace';
 import { ExperimentService } from '../../../client/common/experiments/service';
 import { Experiments } from '../../../client/common/utils/localize';
 import * as Telemetry from '../../../client/telemetry';
+import { EventName } from '../../../client/telemetry/constants';
 import { PVSC_EXTENSION_ID_FOR_TESTS } from '../../constants';
 import { MockOutputChannel } from '../../mockClasses';
 import { MockMemento } from '../../mocks/mementos';
@@ -56,10 +57,10 @@ suite('Experimentation service', () => {
         } as any);
     }
 
-    function configureApplicationEnvironment(channel: Channel, version: string) {
+    function configureApplicationEnvironment(channel: Channel, version: string, contributes?: Record<string, unknown>) {
         when(appEnvironment.extensionChannel).thenReturn(channel);
         when(appEnvironment.extensionName).thenReturn(PVSC_EXTENSION_ID_FOR_TESTS);
-        when(appEnvironment.packageJson).thenReturn({ version });
+        when(appEnvironment.packageJson).thenReturn({ version, contributes });
     }
 
     suite('Initialization', () => {
@@ -149,7 +150,7 @@ suite('Experimentation service', () => {
             );
             const output = `${Experiments.inGroup().format('pythonExperiment')}\n`;
 
-            assert.equal(outputChannel.output, output);
+            assert.strictEqual(outputChannel.output, output);
         });
     });
 
@@ -223,7 +224,7 @@ suite('Experimentation service', () => {
             const result = await experimentService.inExperiment(experiment);
 
             assert.isTrue(result);
-            assert.equal(telemetryEvents.length, 0);
+            assert.strictEqual(telemetryEvents.length, 0);
         });
 
         test('If the opt-in setting contains `All`, inExperiment should check the value cached by the experiment service', async () => {
@@ -270,7 +271,7 @@ suite('Experimentation service', () => {
             const result = await experimentService.inExperiment(experiment);
 
             assert.isTrue(result);
-            assert.equal(telemetryEvents.length, 0);
+            assert.strictEqual(telemetryEvents.length, 0);
             sinon.assert.calledOnce(isCachedFlightEnabledStub);
         });
 
@@ -318,7 +319,7 @@ suite('Experimentation service', () => {
             const result = await experimentService.inExperiment(experiment);
 
             assert.isFalse(result);
-            assert.equal(telemetryEvents.length, 0);
+            assert.strictEqual(telemetryEvents.length, 0);
             sinon.assert.notCalled(isCachedFlightEnabledStub);
         });
     });
@@ -347,7 +348,7 @@ suite('Experimentation service', () => {
             );
             const result = await experimentService.getExperimentValue(experiment);
 
-            assert.equal(result, 'value');
+            assert.strictEqual(result, 'value');
             sinon.assert.calledOnce(getTreatmentVariableAsyncStub);
         });
 
@@ -396,4 +397,196 @@ suite('Experimentation service', () => {
             sinon.assert.notCalled(getTreatmentVariableAsyncStub);
         });
     });
+
+    suite('Opt-in/out telemetry', () => {
+        let telemetryEvents: { eventName: string; properties: Record<string, unknown> }[] = [];
+        let sendTelemetryEventStub: sinon.SinonStub;
+
+        setup(() => {
+            sendTelemetryEventStub = sinon
+                .stub(Telemetry, 'sendTelemetryEvent')
+                .callsFake((eventName: string, _, properties: Record<string, unknown>) => {
+                    const telemetry = { eventName, properties };
+                    telemetryEvents.push(telemetry);
+                });
+
+            configureApplicationEnvironment('stable', extensionVersion);
+        });
+
+        teardown(() => {
+            telemetryEvents = [];
+        });
+
+        test('Telemetry should be sent when activating the ExperimentService instance', async () => {
+            configureSettings(true, [], []);
+            configureApplicationEnvironment('stable', extensionVersion, { configuration: { properties: {} } });
+
+            const experimentService = new ExperimentService(
+                instance(workspaceService),
+                instance(appEnvironment),
+                globalMemento,
+                outputChannel,
+            );
+
+            await experimentService.activate();
+
+            assert.strictEqual(telemetryEvents.length, 1);
+            assert.strictEqual(telemetryEvents[0].eventName, EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS);
+            sinon.assert.calledOnce(sendTelemetryEventStub);
+        });
+
+        test('The telemetry event properties should only be populated with valid experiment values', async () => {
+            const contributes = {
+                configuration: {
+                    properties: {
+                        'python.experiments.optInto': {
+                            items: {
+                                enum: ['foo', 'bar'],
+                            },
+                        },
+                        'python.experiments.optOutFrom': {
+                            items: {
+                                enum: ['foo', 'bar'],
+                            },
+                        },
+                    },
+                },
+            };
+            configureSettings(true, ['foo', 'baz'], ['bar', 'invalid']);
+            configureApplicationEnvironment('stable', extensionVersion, contributes);
+
+            const experimentService = new ExperimentService(
+                instance(workspaceService),
+                instance(appEnvironment),
+                globalMemento,
+                outputChannel,
+            );
+
+            await experimentService.activate();
+
+            const { properties } = telemetryEvents[0];
+            assert.deepStrictEqual(properties, { optedInto: ['foo'], optedOutFrom: ['bar'] });
+        });
+
+        test('Set telemetry properties to empty arrays if no experiments have been opted into or out from', async () => {
+            const contributes = {
+                configuration: {
+                    properties: {
+                        'python.experiments.optInto': {
+                            items: {
+                                enum: ['foo', 'bar'],
+                            },
+                        },
+                        'python.experiments.optOutFrom': {
+                            items: {
+                                enum: ['foo', 'bar'],
+                            },
+                        },
+                    },
+                },
+            };
+            configureSettings(true, [], []);
+            configureApplicationEnvironment('stable', extensionVersion, contributes);
+
+            const experimentService = new ExperimentService(
+                instance(workspaceService),
+                instance(appEnvironment),
+                globalMemento,
+                outputChannel,
+            );
+
+            await experimentService.activate();
+
+            const { properties } = telemetryEvents[0];
+            assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] });
+        });
+
+        test('If the entered value for a setting contains "All", do not expand it to be a list of all experiments, and pass it as-is', async () => {
+            const contributes = {
+                configuration: {
+                    properties: {
+                        'python.experiments.optInto': {
+                            items: {
+                                enum: ['foo', 'bar', 'All'],
+                            },
+                        },
+                        'python.experiments.optOutFrom': {
+                            items: {
+                                enum: ['foo', 'bar', 'All'],
+                            },
+                        },
+                    },
+                },
+            };
+            configureSettings(true, ['All'], ['All']);
+            configureApplicationEnvironment('stable', extensionVersion, contributes);
+
+            const experimentService = new ExperimentService(
+                instance(workspaceService),
+                instance(appEnvironment),
+                globalMemento,
+                outputChannel,
+            );
+
+            await experimentService.activate();
+
+            const { properties } = telemetryEvents[0];
+            assert.deepStrictEqual(properties, { optedInto: ['All'], optedOutFrom: ['All'] });
+        });
+
+        // This is an unlikely scenario.
+        test('If a setting is not in package.json, set the corresponding telemetry property to an empty array', async () => {
+            const contributes = {
+                configuration: {
+                    properties: {},
+                },
+            };
+            configureSettings(true, ['something'], ['another']);
+            configureApplicationEnvironment('stable', extensionVersion, contributes);
+
+            const experimentService = new ExperimentService(
+                instance(workspaceService),
+                instance(appEnvironment),
+                globalMemento,
+                outputChannel,
+            );
+
+            await experimentService.activate();
+
+            const { properties } = telemetryEvents[0];
+            assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] });
+        });
+
+        // This is also an unlikely scenario.
+        test('If a setting does not have an enum of valid values, set the corresponding telemetry property to an empty array', async () => {
+            const contributes = {
+                configuration: {
+                    properties: {
+                        'python.experiments.optInto': {
+                            items: {},
+                        },
+                        'python.experiments.optOutFrom': {
+                            items: {
+                                enum: ['foo', 'bar', 'All'],
+                            },
+                        },
+                    },
+                },
+            };
+            configureSettings(true, ['something'], []);
+            configureApplicationEnvironment('stable', extensionVersion, contributes);
+
+            const experimentService = new ExperimentService(
+                instance(workspaceService),
+                instance(appEnvironment),
+                globalMemento,
+                outputChannel,
+            );
+
+            await experimentService.activate();
+
+            const { properties } = telemetryEvents[0];
+            assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] });
+        });
+    });
 });