From c035148a95f5be7560466d5290d427ca1a1b476a Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Fri, 13 Aug 2021 10:28:44 -0700 Subject: [PATCH 1/4] News file --- news/3 Code Health/16764.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/3 Code Health/16764.md diff --git a/news/3 Code Health/16764.md b/news/3 Code Health/16764.md new file mode 100644 index 000000000000..bb42c793a233 --- /dev/null +++ b/news/3 Code Health/16764.md @@ -0,0 +1 @@ +Add telemetry when an interpreter gets auto-selected. From 3db80eebcc2f818b16b1b43e1332be79a76a8dbb Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 17 Aug 2021 08:23:56 -0700 Subject: [PATCH 2/4] Update news file --- news/3 Code Health/16764.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/3 Code Health/16764.md b/news/3 Code Health/16764.md index bb42c793a233..39c9cd34e545 100644 --- a/news/3 Code Health/16764.md +++ b/news/3 Code Health/16764.md @@ -1 +1 @@ -Add telemetry when an interpreter gets auto-selected. +Add telemetry for when an interpreter gets auto-selected. From e9d96dc6f6934079b75b13bd0a24e57930e785c4 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 17 Aug 2021 08:24:20 -0700 Subject: [PATCH 3/4] Reuse existing event --- src/client/interpreter/autoSelection/index.ts | 15 +++++++++------ src/client/telemetry/index.ts | 19 +++++-------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 28dec3aa4f22..9d6af2b303e2 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -12,7 +12,7 @@ import { IPersistentState, IPersistentStateFactory, Resource } from '../../commo import { createDeferred, Deferred } from '../../common/utils/async'; import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/pythonVersion'; import { PythonEnvironment } from '../../pythonEnvironments/info'; -import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; +import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { EnvTypeHeuristic, getEnvTypeHeuristic } from '../configuration/environmentTypeComparer'; import { IInterpreterComparer } from '../configuration/types'; @@ -50,14 +50,14 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio } /** - * If there's a cached auto-selected interpreter -> return it. - * If not, check if we are in the env sorting experiment, and use the appropriate auto-selection logic. + * Auto-select a Python environment from the list returned by environment discovery. + * If there's a cached auto-selected environment -> return it. */ - @captureTelemetry(EventName.PYTHON_INTERPRETER_AUTO_SELECTION, {}, true) public async autoSelectInterpreter(resource: Resource): Promise { const key = this.getWorkspacePathKey(resource); + const useCachedInterpreter = this.autoSelectedWorkspacePromises.has(key); - if (!this.autoSelectedWorkspacePromises.has(key)) { + if (!useCachedInterpreter) { const deferred = createDeferred(); this.autoSelectedWorkspacePromises.set(key, deferred); @@ -68,6 +68,10 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio deferred.resolve(); } + sendTelemetryEvent(EventName.PYTHON_INTERPRETER_AUTO_SELECTION, undefined, { + useCachedInterpreter, + }); + return this.autoSelectedWorkspacePromises.get(key)!.promise; } @@ -103,7 +107,6 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio protected async clearWorkspaceStoreIfInvalid(resource: Resource): Promise { const stateStore = this.getWorkspaceState(resource); if (stateStore && stateStore.value && !(await this.fs.fileExists(stateStore.value.path))) { - sendTelemetryEvent(EventName.PYTHON_INTERPRETER_AUTO_SELECTION, {}, { interpreterMissing: true }); await stateStore.updateValue(undefined); } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 937fc439a5bd..5e6abca7f6d9 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1049,25 +1049,16 @@ export interface IEventNamePropertyMapping { */ interpreterType: EnvironmentType; }; + /** + * Telemetry event sent when auto-selection is called. + */ [EventName.PYTHON_INTERPRETER_AUTO_SELECTION]: { /** - * If cached interpreter no longer exists or is invalid - * - * @type {boolean} - */ - interpreterMissing?: boolean; - /** - * Carries `true` if next rule is identified for autoselecting interpreter - * - * @type {boolean} - */ - identified?: boolean; - /** - * Carries `true` if cached interpreter is updated to use the current interpreter, `false` otherwise + * If auto-selection has been run earlier in this session, and this call returned a cached value. * * @type {boolean} */ - updated?: boolean; + useCachedInterpreter?: boolean; }; /** * Sends information regarding discovered python environments (virtualenv, conda, pipenv etc.) From 2bf45588ab8d2332fab2ef9e3ed1a129f8a7be67 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 17 Aug 2021 08:24:25 -0700 Subject: [PATCH 4/4] Add tests --- .../autoSelection/index.unit.test.ts | 113 +++++++++++++++++- 1 file changed, 112 insertions(+), 1 deletion(-) diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index 6c89db234a4f..09ed1a4cccb4 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -6,6 +6,7 @@ import { expect } from 'chai'; import * as path from 'path'; import { SemVer } from 'semver'; +import * as sinon from 'sinon'; import { anyString, anything, instance, mock, verify, when } from 'ts-mockito'; import { Uri } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; @@ -28,6 +29,8 @@ import { import { InterpreterHelper } from '../../../client/interpreter/helpers'; import { InterpreterService } from '../../../client/interpreter/interpreterService'; import { EnvironmentType, PythonEnvironment } from '../../../client/pythonEnvironments/info'; +import * as Telemetry from '../../../client/telemetry'; +import { EventName } from '../../../client/telemetry/constants'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -42,7 +45,9 @@ suite('Interpreters - Auto Selection', () => { let helper: IInterpreterHelper; let proxy: IInterpreterAutoSelectionProxyService; let interpreterService: IInterpreterService; + let sendTelemetryEventStub: sinon.SinonStub; let options: GetInterpreterOptions[] = []; + let telemetryEvents: { eventName: string; properties: Record }[] = []; class InterpreterAutoSelectionServiceTest extends InterpreterAutoSelectionService { public initializeStore(resource: Resource): Promise { return super.initializeStore(resource); @@ -93,17 +98,27 @@ suite('Interpreters - Auto Selection', () => { } as PythonEnvironment, ]); }); + + sendTelemetryEventStub = sinon + .stub(Telemetry, 'sendTelemetryEvent') + .callsFake((eventName: string, _, properties: Record) => { + const telemetry = { eventName, properties }; + telemetryEvents.push(telemetry); + }); }); teardown(() => { + sinon.restore(); + Telemetry._resetSharedProperties(); options = []; + telemetryEvents = []; }); test('Instance is registered in proxy', () => { verify(proxy.registerInstance!(autoSelectionService)).once(); }); - suite('When using locator-based auto-selection', () => { + suite('Test locator-based auto-selection method', () => { let workspacePath: string; let resource: Uri; let eventFired: boolean; @@ -284,6 +299,102 @@ suite('Interpreters - Auto Selection', () => { verify(interpreterService.getInterpreters(resource, anything())).once(); expect(options).to.deep.equal([{ ignoreCache: false }], 'getInterpreters options are different'); }); + + test('Telemetry event is sent with useCachedInterpreter set to false if auto-selection has not been run before', async () => { + const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); + + when(interpreterService.getInterpreters(resource, anything())).thenCall(() => + Promise.resolve([ + { + envType: EnvironmentType.Conda, + envPath: path.join('some', 'conda', 'env'), + version: { major: 3, minor: 7, patch: 2 }, + } as PythonEnvironment, + { + envType: EnvironmentType.Pipenv, + envPath: path.join('some', 'pipenv', 'env'), + version: { major: 3, minor: 10, patch: 0 }, + } as PythonEnvironment, + ]), + ); + + autoSelectionService = new InterpreterAutoSelectionServiceTest( + instance(workspaceService), + instance(stateFactory), + instance(fs), + instance(interpreterService), + interpreterComparer, + instance(proxy), + instance(helper), + ); + + autoSelectionService.initializeStore = () => Promise.resolve(); + + await autoSelectionService.autoSelectInterpreter(resource); + + verify(interpreterService.getInterpreters(resource, anything())).once(); + sinon.assert.calledOnce(sendTelemetryEventStub); + expect(telemetryEvents).to.deep.equal( + [ + { + eventName: EventName.PYTHON_INTERPRETER_AUTO_SELECTION, + properties: { useCachedInterpreter: false }, + }, + ], + 'Telemetry event properties are different', + ); + }); + + test('Telemetry event is sent with useCachedInterpreter set to true if auto-selection has been run before', async () => { + const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); + + when(interpreterService.getInterpreters(resource, anything())).thenCall(() => + Promise.resolve([ + { + envType: EnvironmentType.Conda, + envPath: path.join('some', 'conda', 'env'), + version: { major: 3, minor: 7, patch: 2 }, + } as PythonEnvironment, + { + envType: EnvironmentType.Pipenv, + envPath: path.join('some', 'pipenv', 'env'), + version: { major: 3, minor: 10, patch: 0 }, + } as PythonEnvironment, + ]), + ); + + autoSelectionService = new InterpreterAutoSelectionServiceTest( + instance(workspaceService), + instance(stateFactory), + instance(fs), + instance(interpreterService), + interpreterComparer, + instance(proxy), + instance(helper), + ); + + autoSelectionService.initializeStore = () => Promise.resolve(); + + await autoSelectionService.autoSelectInterpreter(resource); + + await autoSelectionService.autoSelectInterpreter(resource); + + verify(interpreterService.getInterpreters(resource, anything())).once(); + sinon.assert.calledTwice(sendTelemetryEventStub); + expect(telemetryEvents).to.deep.equal( + [ + { + eventName: EventName.PYTHON_INTERPRETER_AUTO_SELECTION, + properties: { useCachedInterpreter: false }, + }, + { + eventName: EventName.PYTHON_INTERPRETER_AUTO_SELECTION, + properties: { useCachedInterpreter: true }, + }, + ], + 'Telemetry event properties are different', + ); + }); }); test('Initialize the store', async () => {