From caf4da649f9a157bc3fbe9d4a1b7a8b14e620926 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 27 Aug 2020 22:50:37 -0700 Subject: [PATCH 1/5] Add extension install telemetry. --- src/client/extensionActivation.ts | 6 ++++ .../telemetry/extensionInstallTelemetry.ts | 36 +++++++++++++++++++ src/client/telemetry/index.ts | 6 ++++ 3 files changed, 48 insertions(+) create mode 100644 src/client/telemetry/extensionInstallTelemetry.ts diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index b842440db208..b2c7704812c5 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -17,6 +17,7 @@ import { Commands, PYTHON, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL, UseProposed import { registerTypes as installerRegisterTypes } from './common/installer/serviceRegistry'; import { traceError } from './common/logger'; import { registerTypes as platformRegisterTypes } from './common/platform/serviceRegistry'; +import { IFileSystem } from './common/platform/types'; import { registerTypes as processRegisterTypes } from './common/process/serviceRegistry'; import { registerTypes as commonRegisterTypes } from './common/serviceRegistry'; import { @@ -57,6 +58,7 @@ import { registerTypes as providersRegisterTypes } from './providers/serviceRegi import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; import { TerminalProvider } from './providers/terminalProvider'; import { ISortImportsEditingProvider } from './providers/types'; +import { setExtensionInstallTelemetryProperties } from './telemetry/extensionInstallTelemetry'; import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; import { ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; import { TEST_OUTPUT_CHANNEL } from './testing/common/constants'; @@ -104,6 +106,10 @@ async function activateLegacy( platformRegisterTypes(serviceManager); processRegisterTypes(serviceManager); + // We need to setup this property before any telemetry is sent + const fs = serviceManager.get(IFileSystem); + await setExtensionInstallTelemetryProperties(fs); + const applicationEnv = serviceManager.get(IApplicationEnvironment); const enableProposedApi = applicationEnv.packageJson.enableProposedApi; serviceManager.addSingletonInstance(UseProposedApi, enableProposedApi); diff --git a/src/client/telemetry/extensionInstallTelemetry.ts b/src/client/telemetry/extensionInstallTelemetry.ts new file mode 100644 index 000000000000..c5b41d4adfe9 --- /dev/null +++ b/src/client/telemetry/extensionInstallTelemetry.ts @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { setSharedProperty } from '.'; +import { IFileSystem } from '../common/platform/types'; +import { EXTENSION_ROOT_DIR } from '../constants'; + +/** + * Sets shared telemetry property about where the extension was installed from + * currently we only detect installations from the Python coding pack installer. + * Those installations get the 'pythonCodingPack'. Otherwise assume the default + * case as 'MarketPlace'. + * + */ +export async function setExtensionInstallTelemetryProperties(fs: IFileSystem) { + // Look for PythonCodingPack file under `%USERPROFILE%/.vscode/extensions` + // folder. If that file exists treat this extension as installed from coding + // pack. + // + // Use parent of EXTENSION_ROOT_DIR to access %USERPROFILE%/.vscode/extensions + // If you use join (%USERPROFILE%, .vscode, extensions), then if user has both + // ~/.vscode and ~./vscode-insiders, and launches Python extension via VS Code + // insiders, it will look for ~/.vscode/extensions/PythonCodingPack which is + // wrong. This is a rare scenario, and it can be easily avoided by using parent + // of EXTENSION_ROOT_DIR. Ii you use EXTENSION_ROOT_DIR then, in the case of + // VS Code insiders it will be, ~/.vscode-insiders/extensions/. That folder will + // NOT have PythonCodingPack file and we can treat the insiders case as MarkerPlace. + const fileToCheck = path.join(path.dirname(EXTENSION_ROOT_DIR), 'PythonCodingPack'); + if (await fs.fileExists(fileToCheck)) { + setSharedProperty('installSource', 'pythonCodingPack'); + } else { + // We did not file the `PythonCodingPack` file, assume market place install. + setSharedProperty('installSource', 'marketPlace'); + } +} diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 52294d80c1a5..fc72620503dd 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -332,6 +332,12 @@ export interface ISharedPropertyMapping { * For every DS telemetry we would like to know the type of Notebook Editor used when doing something. */ ['ds_notebookeditor']: undefined | 'old' | 'custom' | 'native'; + + /** + * For every telemetry event from the extension we want to make sure we can associate it with install + * source. We took this approach to work around very limiting query performance issues. + */ + ['installSource']: undefined | 'marketPlace' | 'pythonCodingPack'; } // Map all events to their properties From fb0e91370e5cc37bcee7860d5a4f548409e1c48b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 27 Aug 2020 22:50:52 -0700 Subject: [PATCH 2/5] Add tests. --- .../extensionInstallTelemetery.unit.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/test/telemetry/extensionInstallTelemetery.unit.test.ts diff --git a/src/test/telemetry/extensionInstallTelemetery.unit.test.ts b/src/test/telemetry/extensionInstallTelemetery.unit.test.ts new file mode 100644 index 000000000000..621f63a2c8e6 --- /dev/null +++ b/src/test/telemetry/extensionInstallTelemetery.unit.test.ts @@ -0,0 +1,28 @@ +import * as sinon from 'sinon'; +import { anyString, instance, mock, when } from 'ts-mockito'; +import { FileSystem } from '../../client/common/platform/fileSystem'; +import { IFileSystem } from '../../client/common/platform/types'; +import * as Telemetry from '../../client/telemetry'; +import { setExtensionInstallTelemetryProperties } from '../../client/telemetry/extensionInstallTelemetry'; + +suite('Extension Install Telemetry', () => { + let fs: IFileSystem; + let telemetryPropertyStub: sinon.SinonStub; + setup(() => { + fs = mock(FileSystem); + telemetryPropertyStub = sinon.stub(Telemetry, 'setSharedProperty'); + }); + teardown(() => { + telemetryPropertyStub.restore(); + }); + test('PythonCodingPack exists', async () => { + when(fs.fileExists(anyString())).thenResolve(true); + await setExtensionInstallTelemetryProperties(instance(fs)); + telemetryPropertyStub.calledOnceWithExactly('installSource', 'pythonCodingPack'); + }); + test('PythonCodingPack does not exists', async () => { + when(fs.fileExists(anyString())).thenResolve(false); + await setExtensionInstallTelemetryProperties(instance(fs)); + telemetryPropertyStub.calledOnceWithExactly('installSource', 'markerPlace'); + }); +}); From c02ab65284a6583f60d6c7fc3153c7ef76f77931 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 27 Aug 2020 22:58:56 -0700 Subject: [PATCH 3/5] Fix typo --- src/client/telemetry/extensionInstallTelemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/telemetry/extensionInstallTelemetry.ts b/src/client/telemetry/extensionInstallTelemetry.ts index c5b41d4adfe9..e69d68ef95e1 100644 --- a/src/client/telemetry/extensionInstallTelemetry.ts +++ b/src/client/telemetry/extensionInstallTelemetry.ts @@ -23,7 +23,7 @@ export async function setExtensionInstallTelemetryProperties(fs: IFileSystem) { // ~/.vscode and ~./vscode-insiders, and launches Python extension via VS Code // insiders, it will look for ~/.vscode/extensions/PythonCodingPack which is // wrong. This is a rare scenario, and it can be easily avoided by using parent - // of EXTENSION_ROOT_DIR. Ii you use EXTENSION_ROOT_DIR then, in the case of + // of EXTENSION_ROOT_DIR. If you use EXTENSION_ROOT_DIR then, in the case of // VS Code insiders it will be, ~/.vscode-insiders/extensions/. That folder will // NOT have PythonCodingPack file and we can treat the insiders case as MarkerPlace. const fileToCheck = path.join(path.dirname(EXTENSION_ROOT_DIR), 'PythonCodingPack'); From cd539c8c9d6982ae5a07ab1c4d9235141af16c5d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 28 Aug 2020 10:21:46 -0700 Subject: [PATCH 4/5] Fix tests. --- src/test/telemetry/extensionInstallTelemetery.unit.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/telemetry/extensionInstallTelemetery.unit.test.ts b/src/test/telemetry/extensionInstallTelemetery.unit.test.ts index 621f63a2c8e6..47e25eca05fa 100644 --- a/src/test/telemetry/extensionInstallTelemetery.unit.test.ts +++ b/src/test/telemetry/extensionInstallTelemetery.unit.test.ts @@ -1,3 +1,4 @@ +import * as assert from 'assert'; import * as sinon from 'sinon'; import { anyString, instance, mock, when } from 'ts-mockito'; import { FileSystem } from '../../client/common/platform/fileSystem'; @@ -18,11 +19,11 @@ suite('Extension Install Telemetry', () => { test('PythonCodingPack exists', async () => { when(fs.fileExists(anyString())).thenResolve(true); await setExtensionInstallTelemetryProperties(instance(fs)); - telemetryPropertyStub.calledOnceWithExactly('installSource', 'pythonCodingPack'); + assert.ok(telemetryPropertyStub.calledOnceWithExactly('installSource', 'pythonCodingPack')); }); test('PythonCodingPack does not exists', async () => { when(fs.fileExists(anyString())).thenResolve(false); await setExtensionInstallTelemetryProperties(instance(fs)); - telemetryPropertyStub.calledOnceWithExactly('installSource', 'markerPlace'); + assert.ok(telemetryPropertyStub.calledOnceWithExactly('installSource', 'marketPlace')); }); }); From 51ccd8285cf14543365de984f8ff14ae043ece38 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 28 Aug 2020 11:58:06 -0700 Subject: [PATCH 5/5] Update comment to reflect what the installed actually does. --- src/client/telemetry/extensionInstallTelemetry.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/client/telemetry/extensionInstallTelemetry.ts b/src/client/telemetry/extensionInstallTelemetry.ts index e69d68ef95e1..87e6ec50e3ea 100644 --- a/src/client/telemetry/extensionInstallTelemetry.ts +++ b/src/client/telemetry/extensionInstallTelemetry.ts @@ -19,13 +19,10 @@ export async function setExtensionInstallTelemetryProperties(fs: IFileSystem) { // pack. // // Use parent of EXTENSION_ROOT_DIR to access %USERPROFILE%/.vscode/extensions - // If you use join (%USERPROFILE%, .vscode, extensions), then if user has both - // ~/.vscode and ~./vscode-insiders, and launches Python extension via VS Code - // insiders, it will look for ~/.vscode/extensions/PythonCodingPack which is - // wrong. This is a rare scenario, and it can be easily avoided by using parent - // of EXTENSION_ROOT_DIR. If you use EXTENSION_ROOT_DIR then, in the case of - // VS Code insiders it will be, ~/.vscode-insiders/extensions/. That folder will - // NOT have PythonCodingPack file and we can treat the insiders case as MarkerPlace. + // this is because the installer will add PythonCodingPack to %USERPROFILE%/.vscode/extensions + // or %USERPROFILE%/.vscode-insiders/extensions depending on what was installed + // previously by the user. If we always join (, .vscode, extensions), we will + // end up looking at the wrong place, with respect to the extension that was launched. const fileToCheck = path.join(path.dirname(EXTENSION_ROOT_DIR), 'PythonCodingPack'); if (await fs.fileExists(fileToCheck)) { setSharedProperty('installSource', 'pythonCodingPack');