From 36a22096a3e4aeeae35ad6b704201e09a6a29c6a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 8 Apr 2019 12:41:57 -0700 Subject: [PATCH 1/3] Capture telemetry when switching LS --- src/client/activation/activationService.ts | 24 +++++-- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 1 + .../activation/activationService.unit.test.ts | 67 +++++++++++++++---- 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index c48bd17e5034..7859ccc0c33c 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -10,7 +10,8 @@ import { IDiagnosticsService } from '../application/diagnostics/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; import '../common/extensions'; -import { IConfigurationService, IDisposableRegistry, IOutputChannel, IPythonSettings, Resource } from '../common/types'; +import { IConfigurationService, IDisposableRegistry, IOutputChannel, IPersistentStateFactory, IPythonSettings, Resource } from '../common/types'; +import { swallowExceptions } from '../common/utils/decorators'; import { IServiceContainer } from '../ioc/types'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; @@ -31,7 +32,8 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv private readonly lsNotSupportedDiagnosticService: IDiagnosticsService; private resource!: Resource; - constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, + @inject(IPersistentStateFactory) private stateFactory: IPersistentStateFactory) { this.workspaceService = this.serviceContainer.get(IWorkspaceService); this.output = this.serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); this.appShell = this.serviceContainer.get(IApplicationShell); @@ -98,7 +100,19 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv this.currentActivator.activator.dispose(); } } - + @swallowExceptions('Swtich Language Server') + public async trackLangaugeServerSwitch(jediEnabled: boolean): Promise { + const state = this.stateFactory.createGlobalPersistentState('SWITCH_LS', undefined); + if (typeof state.value !== 'boolean') { + await state.updateValue(jediEnabled); + return; + } + if (state.value !== jediEnabled) { + await state.updateValue(jediEnabled); + const message = jediEnabled ? 'Switch to Jedi from LS' : 'Switch to LS from Jedi'; + sendTelemetryEvent(EventName.PYTHON_LANGUAGE_SERVER_SWITCHED, undefined, { change: message }); + } + } protected onWorkspaceFoldersChanged() { //If an activated workspace folder was removed, dispose its activator const workspaceKeys = this.workspaceService.workspaceFolders!.map(workspaceFolder => this.getWorkspacePathKey(workspaceFolder.uri)); @@ -141,7 +155,9 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv } private useJedi(): boolean { const configurationService = this.serviceContainer.get(IConfigurationService); - return configurationService.getSettings(this.resource).jediEnabled; + const enabled = configurationService.getSettings(this.resource).jediEnabled; + this.trackLangaugeServerSwitch(enabled).ignoreErrors(); + return enabled; } private getWorkspacePathKey(resource: Resource): string { return this.workspaceService.getWorkspaceFolderIdentifier(resource, workspacePathNameForGlobalWorkspaces); diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index c75f9a2be95b..6bfb8b605fe1 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -48,6 +48,7 @@ export enum EventName { UNITTEST_NAVIGATE_TEST_FUNCTION = 'UNITTEST.NAVIGATE.TEST_FUNCTION', UNITTEST_NAVIGATE_TEST_SUITE = 'UNITTEST.NAVIGATE.TEST_SUITE', UNITTEST_EXPLORER_WORK_SPACE_COUNT = 'UNITTEST.TEST_EXPLORER.WORK_SPACE_COUNT', + PYTHON_LANGUAGE_SERVER_SWITCHED = 'PYTHON_LANGUAGE_SERVER.SWITCHED', PYTHON_LANGUAGE_SERVER_ANALYSISTIME = 'PYTHON_LANGUAGE_SERVER.ANALYSIS_TIME', PYTHON_LANGUAGE_SERVER_ENABLED = 'PYTHON_LANGUAGE_SERVER.ENABLED', PYTHON_LANGUAGE_SERVER_EXTRACTED = 'PYTHON_LANGUAGE_SERVER.EXTRACTED', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 1d3a76172e36..a7c106d2af04 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -284,6 +284,7 @@ export interface IEventNamePropertyMapping { [EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL]: InterpreterActivation; [EventName.PYTHON_INTERPRETER_AUTO_SELECTION]: InterpreterAutoSelection; [EventName.PYTHON_INTERPRETER_DISCOVERY]: InterpreterDiscovery; + [EventName.PYTHON_LANGUAGE_SERVER_SWITCHED]: { change: 'Switch to Jedi from LS' | 'Switch to LS from Jedi' }; [EventName.PYTHON_LANGUAGE_SERVER_ANALYSISTIME]: { success: boolean }; [EventName.PYTHON_LANGUAGE_SERVER_DOWNLOADED]: LanguageServerVersionTelemetry; [EventName.PYTHON_LANGUAGE_SERVER_ENABLED]: never | undefined; diff --git a/src/test/activation/activationService.unit.test.ts b/src/test/activation/activationService.unit.test.ts index db9943b3c221..39d4aad15b21 100644 --- a/src/test/activation/activationService.unit.test.ts +++ b/src/test/activation/activationService.unit.test.ts @@ -21,9 +21,11 @@ import { LSNotSupportedDiagnosticServiceId } from '../../client/application/diag import { IDiagnostic, IDiagnosticsService } from '../../client/application/diagnostics/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../client/common/application/types'; import { IPlatformService } from '../../client/common/platform/types'; -import { IConfigurationService, IDisposable, IDisposableRegistry, IOutputChannel, IPythonSettings, Resource } from '../../client/common/types'; +import { IConfigurationService, IDisposable, IDisposableRegistry, IOutputChannel, IPersistentState, IPersistentStateFactory, IPythonSettings, Resource } from '../../client/common/types'; import { IServiceContainer } from '../../client/ioc/types'; +// tslint:disable:no-any + suite('Activation - ActivationService', () => { [true, false].forEach(jediIsEnabled => { suite(`Jedi is ${jediIsEnabled ? 'enabled' : 'disabled'}`, () => { @@ -34,12 +36,16 @@ suite('Activation - ActivationService', () => { let workspaceService: TypeMoq.IMock; let platformService: TypeMoq.IMock; let lsNotSupportedDiagnosticService: TypeMoq.IMock; + let stateFactory: TypeMoq.IMock; + let state: TypeMoq.IMock>; setup(() => { serviceContainer = TypeMoq.Mock.ofType(); appShell = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); cmdManager = TypeMoq.Mock.ofType(); platformService = TypeMoq.Mock.ofType(); + stateFactory = TypeMoq.Mock.ofType(); + state = TypeMoq.Mock.ofType>(); const configService = TypeMoq.Mock.ofType(); pythonSettings = TypeMoq.Mock.ofType(); const langFolderServiceMock = TypeMoq.Mock.ofType(); @@ -54,7 +60,10 @@ suite('Activation - ActivationService', () => { langFolderServiceMock .setup(l => l.getCurrentLanguageServerDirectory()) .returns(() => Promise.resolve(folderVer)); - + stateFactory.setup(f => f.createGlobalPersistentState(TypeMoq.It.isValue('SWITCH_LS'), TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(() => state.object); + state.setup(s => s.value).returns(() => undefined); + state.setup(s => s.updateValue(TypeMoq.It.isAny())).returns(() => Promise.resolve()); const output = TypeMoq.Mock.ofType(); serviceContainer .setup(c => c.get(TypeMoq.It.isValue(IOutputChannel), TypeMoq.It.isAny())) @@ -101,7 +110,7 @@ suite('Activation - ActivationService', () => { if (lsSupported && !jediIsEnabled) { activatorName = LanguageServerActivator.DotNet; } - let diagnostics : IDiagnostic[]; + let diagnostics: IDiagnostic[]; if (!lsSupported && !jediIsEnabled) { diagnostics = [TypeMoq.It.isAny()]; } else { @@ -127,14 +136,14 @@ suite('Activation - ActivationService', () => { test('LS is supported', async () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); await testActivation(activationService, activator, true); }); test('LS is not supported', async () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); await testActivation(activationService, activator, false); }); @@ -142,14 +151,14 @@ suite('Activation - ActivationService', () => { test('Activatory must be activated', async () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); await testActivation(activationService, activator); }); test('Activatory must be deactivated', async () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); await testActivation(activationService, activator); @@ -171,7 +180,7 @@ suite('Activation - ActivationService', () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabledValueInSetting); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); workspaceService.verifyAll(); await testActivation(activationService, activator); @@ -208,7 +217,7 @@ suite('Activation - ActivationService', () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabledValueInSetting); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); workspaceService.verifyAll(); await testActivation(activationService, activator); @@ -244,7 +253,7 @@ suite('Activation - ActivationService', () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); workspaceService.verifyAll(); await testActivation(activationService, activator); @@ -279,7 +288,7 @@ suite('Activation - ActivationService', () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); workspaceService.verifyAll(); await testActivation(activationService, activator); @@ -304,12 +313,42 @@ suite('Activation - ActivationService', () => { appShell.verifyAll(); cmdManager.verifyAll(); }); + test('Track current LS usage for first usage', async () => { + state.reset(); + state.setup(s => s.value).returns(() => undefined).verifiable(TypeMoq.Times.once()); + state.setup(s => s.updateValue(TypeMoq.It.isValue(true))).returns(() => Promise.resolve()).verifiable(TypeMoq.Times.once()); + + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); + await activationService.trackLangaugeServerSwitch(true); + + state.verifyAll(); + }); + test('Track switch to LS', async () => { + state.reset(); + state.setup(s => s.value).returns(() => true).verifiable(TypeMoq.Times.once()); + state.setup(s => s.updateValue(TypeMoq.It.isValue(false))).returns(() => Promise.resolve()).verifiable(TypeMoq.Times.once()); + + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); + await activationService.trackLangaugeServerSwitch(false); + + state.verify(s => s.updateValue(TypeMoq.It.isValue(false)), TypeMoq.Times.once()); + }); + test('Track switch to Jedi', async () => { + state.reset(); + state.setup(s => s.value).returns(() => false).verifiable(TypeMoq.Times.once()); + state.setup(s => s.updateValue(TypeMoq.It.isValue(true))).returns(() => Promise.resolve()).verifiable(TypeMoq.Times.once()); + + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); + await activationService.trackLangaugeServerSwitch(true); + + state.verify(s => s.updateValue(TypeMoq.It.isValue(true)), TypeMoq.Times.once()); + }); if (!jediIsEnabled) { test('Revert to jedi when LS activation fails', async () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activatorDotNet = TypeMoq.Mock.ofType(); const activatorJedi = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); const diagnostics: IDiagnostic[] = []; lsNotSupportedDiagnosticService .setup(l => l.diagnose(undefined)) @@ -388,7 +427,7 @@ suite('Activation - ActivationService', () => { .callback(cb => (workspaceFoldersChangedHandler = cb)) .returns(() => TypeMoq.Mock.ofType().object) .verifiable(TypeMoq.Times.once()); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); workspaceService.verifyAll(); expect(workspaceFoldersChangedHandler).not.to.be.equal(undefined, 'Handler not set'); const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; @@ -430,7 +469,7 @@ suite('Activation - ActivationService', () => { test('Jedi is only activated once', async () => { pythonSettings.setup(p => p.jediEnabled).returns(() => jediIsEnabled); const activator1 = TypeMoq.Mock.ofType(); - const activationService = new LanguageServerExtensionActivationService(serviceContainer.object); + const activationService = new LanguageServerExtensionActivationService(serviceContainer.object, stateFactory.object); const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 }; const folder2 = { name: 'two', uri: Uri.parse('two'), index: 2 }; serviceContainer From c3387678c02626cac6b342f2f3e797de841e865e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 8 Apr 2019 12:43:52 -0700 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bc9e873e1ff..519ac865632a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ * Fix issues related to debugging Django templagtes. 1. Update the Python language server to 0.2.44. +### Code Health + +1. Capture telemetry to track switching to and from the Language Server. + ([#5162](https://github.com/Microsoft/vscode-python/issues/5162)) + + ## 2019.3.2 (2 April 2019) ### Fixes From 04606ba8179c0c5b2ecb7c3b893c5882288c715c Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Mon, 8 Apr 2019 13:07:34 -0700 Subject: [PATCH 3/3] Update src/client/activation/activationService.ts Co-Authored-By: DonJayamanne --- src/client/activation/activationService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 7859ccc0c33c..2ffe608cd757 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -100,7 +100,7 @@ export class LanguageServerExtensionActivationService implements IExtensionActiv this.currentActivator.activator.dispose(); } } - @swallowExceptions('Swtich Language Server') + @swallowExceptions('Switch Language Server') public async trackLangaugeServerSwitch(jediEnabled: boolean): Promise { const state = this.stateFactory.createGlobalPersistentState('SWITCH_LS', undefined); if (typeof state.value !== 'boolean') {