Skip to content

Commit

Permalink
Fix experiment telemetry related to optInto/optOutFrom settings (#22241)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj authored Oct 17, 2023
1 parent 10b98d3 commit ebaf8fe
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 11 deletions.
6 changes: 4 additions & 2 deletions src/client/common/experiments/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,10 @@ function sendOptInOptOutTelemetry(optedIn: string[], optedOut: string[], package
const sanitizedOptedIn = optedIn.filter((exp) => optedInEnumValues.includes(exp));
const sanitizedOptedOut = optedOut.filter((exp) => optedOutEnumValues.includes(exp));

JSON.stringify(sanitizedOptedIn.sort());

sendTelemetryEvent(EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS, undefined, {
optedInto: sanitizedOptedIn,
optedOutFrom: sanitizedOptedOut,
optedInto: JSON.stringify(sanitizedOptedIn.sort()),
optedOutFrom: JSON.stringify(sanitizedOptedOut.sort()),
});
}
8 changes: 4 additions & 4 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1405,14 +1405,14 @@ export interface IEventNamePropertyMapping {
[EventName.PYTHON_EXPERIMENTS_OPT_IN_OPT_OUT_SETTINGS]: {
/**
* List of valid experiments in the python.experiments.optInto setting
* @type {string[]}
* @type {string}
*/
optedInto: string[];
optedInto: string;
/**
* List of valid experiments in the python.experiments.optOutFrom setting
* @type {string[]}
* @type {string}
*/
optedOutFrom: string[];
optedOutFrom: string;
};
/**
* Telemetry event sent when LS is started for workspace (workspace folder in case of multi-root)
Expand Down
16 changes: 11 additions & 5 deletions src/test/common/experiments/service.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,10 @@ suite('Experimentation service', () => {
await experimentService.activate();

const { properties } = telemetryEvents[1];
assert.deepStrictEqual(properties, { optedInto: ['foo'], optedOutFrom: ['bar'] });
assert.deepStrictEqual(properties, {
optedInto: JSON.stringify(['foo']),
optedOutFrom: JSON.stringify(['bar']),
});
});

test('Set telemetry properties to empty arrays if no experiments have been opted into or out from', async () => {
Expand Down Expand Up @@ -523,7 +526,7 @@ suite('Experimentation service', () => {
await experimentService.activate();

const { properties } = telemetryEvents[1];
assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] });
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 () => {
Expand Down Expand Up @@ -555,7 +558,10 @@ suite('Experimentation service', () => {
await experimentService.activate();

const { properties } = telemetryEvents[0];
assert.deepStrictEqual(properties, { optedInto: ['All'], optedOutFrom: ['All'] });
assert.deepStrictEqual(properties, {
optedInto: JSON.stringify(['All']),
optedOutFrom: JSON.stringify(['All']),
});
});

// This is an unlikely scenario.
Expand All @@ -577,7 +583,7 @@ suite('Experimentation service', () => {
await experimentService.activate();

const { properties } = telemetryEvents[1];
assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] });
assert.deepStrictEqual(properties, { optedInto: '[]', optedOutFrom: '[]' });
});

// This is also an unlikely scenario.
Expand Down Expand Up @@ -608,7 +614,7 @@ suite('Experimentation service', () => {
await experimentService.activate();

const { properties } = telemetryEvents[1];
assert.deepStrictEqual(properties, { optedInto: [], optedOutFrom: [] });
assert.deepStrictEqual(properties, { optedInto: '[]', optedOutFrom: '[]' });
});
});
});

0 comments on commit ebaf8fe

Please sign in to comment.