Skip to content

Commit 68ef910

Browse files
author
Kartik Raj
authored
Added telemetry for usage of activateEnvInCurrentTerminal setting (#8654)
* Added telemetry and tests * News entry * Apply suggestions from code review Co-Authored-By: Eric Snow <[email protected]> * Modified telemetry + code reviews * Oops * Added tests * Move getActiveResource to a common place * Refactor more stuff * Moved into common/application * Apply suggestions from code review Co-Authored-By: Eric Snow <[email protected]> * Update src/test/activation/activeResource.unit.test.ts Co-Authored-By: Eric Snow <[email protected]> * Code reviews * Don't overuse tslint any * Add description * Better format
1 parent 18fa5b5 commit 68ef910

16 files changed

+277
-235
lines changed

‎news/1 Enhancements/8004.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add telemetry for usage of activateEnvInCurrentTerminal setting.

‎src/client/activation/activationManager.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
'use strict';
55

66
import { inject, injectable, multiInject } from 'inversify';
7-
import { TextDocument, workspace } from 'vscode';
7+
import { TextDocument } from 'vscode';
88
import { IApplicationDiagnostics } from '../application/types';
9-
import { IDocumentManager, IWorkspaceService } from '../common/application/types';
9+
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../common/application/types';
1010
import { PYTHON_LANGUAGE } from '../common/constants';
1111
import { traceDecorators } from '../common/logger';
1212
import { IDisposable, Resource } from '../common/types';
@@ -26,7 +26,8 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
2626
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
2727
@inject(IInterpreterAutoSelectionService) private readonly autoSelection: IInterpreterAutoSelectionService,
2828
@inject(IApplicationDiagnostics) private readonly appDiagnostics: IApplicationDiagnostics,
29-
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService
29+
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
30+
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService
3031
) { }
3132

3233
public dispose() {
@@ -44,7 +45,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
4445
// Activate all activation services together.
4546
await Promise.all([
4647
Promise.all(this.singleActivationServices.map(item => item.activate())),
47-
this.activateWorkspace(this.getActiveResource())
48+
this.activateWorkspace(this.activeResourceService.getActiveResource())
4849
]);
4950
await this.autoSelection.autoSelectInterpreter(undefined);
5051
}
@@ -114,12 +115,4 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
114115
protected getWorkspaceKey(resource: Resource) {
115116
return this.workspaceService.getWorkspaceFolderIdentifier(resource, '');
116117
}
117-
private getActiveResource(): Resource {
118-
if (this.documentManager.activeTextEditor && !this.documentManager.activeTextEditor.document.isUntitled) {
119-
return this.documentManager.activeTextEditor.document.uri;
120-
}
121-
return Array.isArray(this.workspaceService.workspaceFolders) && workspace.workspaceFolders!.length > 0
122-
? workspace.workspaceFolders![0].uri
123-
: undefined;
124-
}
125118
}

‎src/client/activation/serviceRegistry.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
'use strict';
55

6+
import { ActiveResourceService } from '../common/application/activeResource';
7+
import { IActiveResourceService } from '../common/application/types';
68
import { INugetRepository } from '../common/nuget/types';
79
import { BANNER_NAME_DS_SURVEY, BANNER_NAME_INTERACTIVE_SHIFTENTER, BANNER_NAME_LS_SURVEY, BANNER_NAME_PROPOSE_LS, IPythonExtensionBanner } from '../common/types';
810
import { DataScienceSurveyBanner } from '../datascience/dataScienceSurveyBanner';
@@ -60,5 +62,6 @@ export function registerTypes(serviceManager: IServiceManager) {
6062
serviceManager.add<ILanguageServerManager>(ILanguageServerManager, LanguageServerManager);
6163
serviceManager.addSingleton<ILanguageServerOutputChannel>(ILanguageServerOutputChannel, LanguageServerOutputChannel);
6264
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, ExtensionSurveyPrompt);
65+
serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
6366
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, AATesting);
6467
}

‎src/client/common/application/activeResource.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { inject, injectable } from 'inversify';
7+
import { Resource } from '../types';
8+
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from './types';
9+
10+
@injectable()
11+
export class ActiveResourceService implements IActiveResourceService {
12+
constructor(
13+
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
14+
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService
15+
) { }
16+
17+
public getActiveResource(): Resource {
18+
const editor = this.documentManager.activeTextEditor;
19+
if (editor && !editor.document.isUntitled) {
20+
return editor.document.uri;
21+
}
22+
return Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0
23+
? this.workspaceService.workspaceFolders[0].uri
24+
: undefined;
25+
}
26+
}

‎src/client/common/application/types.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,3 +1017,11 @@ export interface ILanguageService {
10171017
}
10181018

10191019
export type Channel = 'stable' | 'insiders';
1020+
1021+
/**
1022+
* Wraps the `ActiveResourceService` API class. Created for injecting and mocking class methods in testing
1023+
*/
1024+
export const IActiveResourceService = Symbol('IActiveResourceService');
1025+
export interface IActiveResourceService {
1026+
getActiveResource(): Resource;
1027+
}

‎src/client/providers/replProvider.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { Disposable, Uri } from 'vscode';
2-
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types';
1+
import { Disposable } from 'vscode';
2+
import { IActiveResourceService, ICommandManager } from '../common/application/types';
33
import { Commands } from '../common/constants';
44
import { IServiceContainer } from '../ioc/types';
55
import { captureTelemetry } from '../telemetry';
@@ -8,7 +8,9 @@ import { ICodeExecutionService } from '../terminals/types';
88

99
export class ReplProvider implements Disposable {
1010
private readonly disposables: Disposable[] = [];
11+
private activeResourceService: IActiveResourceService;
1112
constructor(private serviceContainer: IServiceContainer) {
13+
this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
1214
this.registerCommand();
1315
}
1416
public dispose() {
@@ -21,18 +23,8 @@ export class ReplProvider implements Disposable {
2123
}
2224
@captureTelemetry(EventName.REPL)
2325
private async commandHandler() {
24-
const resource = this.getActiveResourceUri();
26+
const resource = this.activeResourceService.getActiveResource();
2527
const replProvider = this.serviceContainer.get<ICodeExecutionService>(ICodeExecutionService, 'repl');
2628
await replProvider.initializeRepl(resource);
2729
}
28-
private getActiveResourceUri(): Uri | undefined {
29-
const documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
30-
if (documentManager.activeTextEditor && !documentManager.activeTextEditor!.document.isUntitled) {
31-
return documentManager.activeTextEditor!.document.uri;
32-
}
33-
const workspace = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
34-
if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) {
35-
return workspace.workspaceFolders[0].uri;
36-
}
37-
}
3830
}

‎src/client/providers/terminalProvider.ts

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,37 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { Disposable, Terminal, Uri } from 'vscode';
5-
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types';
4+
import { Disposable, Terminal } from 'vscode';
5+
import { IActiveResourceService, ICommandManager } from '../common/application/types';
66
import { Commands } from '../common/constants';
77
import { ITerminalActivator, ITerminalServiceFactory } from '../common/terminal/types';
88
import { IConfigurationService } from '../common/types';
9+
import { swallowExceptions } from '../common/utils/decorators';
910
import { IServiceContainer } from '../ioc/types';
10-
import { captureTelemetry } from '../telemetry';
11+
import { captureTelemetry, sendTelemetryEvent } from '../telemetry';
1112
import { EventName } from '../telemetry/constants';
1213

1314
export class TerminalProvider implements Disposable {
1415
private disposables: Disposable[] = [];
16+
private activeResourceService: IActiveResourceService;
1517
constructor(private serviceContainer: IServiceContainer) {
1618
this.registerCommands();
19+
this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
1720
}
21+
22+
@swallowExceptions('Failed to initialize terminal provider')
1823
public async initialize(currentTerminal: Terminal | undefined) {
1924
const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
20-
const pythonSettings = configuration.getSettings();
25+
const pythonSettings = configuration.getSettings(
26+
this.activeResourceService.getActiveResource()
27+
);
2128

22-
if (pythonSettings.terminal.activateEnvInCurrentTerminal && currentTerminal) {
23-
const terminalActivator = this.serviceContainer.get<ITerminalActivator>(ITerminalActivator);
24-
await terminalActivator.activateEnvironmentInTerminal(currentTerminal, undefined, true);
29+
if (pythonSettings.terminal.activateEnvInCurrentTerminal) {
30+
if (currentTerminal) {
31+
const terminalActivator = this.serviceContainer.get<ITerminalActivator>(ITerminalActivator);
32+
await terminalActivator.activateEnvironmentInTerminal(currentTerminal, undefined, true);
33+
}
34+
sendTelemetryEvent(EventName.ACTIVATE_ENV_IN_CURRENT_TERMINAL, undefined, { isTerminalVisible: currentTerminal ? true : false });
2535
}
2636
}
2737
public dispose() {
@@ -36,15 +46,7 @@ export class TerminalProvider implements Disposable {
3646
@captureTelemetry(EventName.TERMINAL_CREATE, { triggeredBy: 'commandpalette' })
3747
private async onCreateTerminal() {
3848
const terminalService = this.serviceContainer.get<ITerminalServiceFactory>(ITerminalServiceFactory);
39-
const activeResource = this.getActiveResource();
49+
const activeResource = this.activeResourceService.getActiveResource();
4050
await terminalService.createTerminalService(activeResource, 'Python').show(false);
4151
}
42-
private getActiveResource(): Uri | undefined {
43-
const documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
44-
if (documentManager.activeTextEditor && !documentManager.activeTextEditor.document.isUntitled) {
45-
return documentManager.activeTextEditor.document.uri;
46-
}
47-
const workspace = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
48-
return Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0 ? workspace.workspaceFolders[0].uri : undefined;
49-
}
5052
}

‎src/client/telemetry/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export enum EventName {
7676
EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT',
7777

7878
TERMINAL_CREATE = 'TERMINAL.CREATE',
79+
ACTIVATE_ENV_IN_CURRENT_TERMINAL = 'ACTIVATE_ENV_IN_CURRENT_TERMINAL',
7980
PYTHON_LANGUAGE_SERVER_LIST_BLOB_STORE_PACKAGES = 'PYTHON_LANGUAGE_SERVER.LIST_BLOB_PACKAGES',
8081
DIAGNOSTICS_ACTION = 'DIAGNOSTICS.ACTION',
8182
DIAGNOSTICS_MESSAGE = 'DIAGNOSTICS.MESSAGE',

‎src/client/telemetry/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,16 @@ export interface IEventNamePropertyMapping {
12351235
*/
12361236
failed: boolean;
12371237
};
1238+
/**
1239+
* Telemetry event sent when the extension is activated, if an active terminal is present and
1240+
* the `python.terminal.activateEnvInCurrentTerminal` setting is set to `true`.
1241+
*/
1242+
[EventName.ACTIVATE_ENV_IN_CURRENT_TERMINAL]: {
1243+
/**
1244+
* Carries boolean `true` if an active terminal is present (terminal is visible), `false` otherwise
1245+
*/
1246+
isTerminalVisible?: boolean;
1247+
};
12381248
/**
12391249
* Telemetry event sent with details when a terminal is created
12401250
*/

‎src/client/terminals/activation.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
'use strict';
55

66
import { inject, injectable } from 'inversify';
7-
import { Terminal, Uri } from 'vscode';
7+
import { Terminal } from 'vscode';
88
import { IExtensionSingleActivationService } from '../activation/types';
99
import {
10-
ICommandManager, IDocumentManager, ITerminalManager, IWorkspaceService
10+
IActiveResourceService, ICommandManager, ITerminalManager
1111
} from '../common/application/types';
1212
import { CODE_RUNNER_EXTENSION_ID } from '../common/constants';
1313
import { ITerminalActivator } from '../common/terminal/types';
@@ -49,9 +49,8 @@ export class TerminalAutoActivation implements ITerminalAutoActivation {
4949
constructor(
5050
@inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
5151
@inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry,
52-
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
5352
@inject(ITerminalActivator) private readonly activator: ITerminalActivator,
54-
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService
53+
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService
5554
) {
5655
disposableRegistry.push(this);
5756
}
@@ -70,14 +69,6 @@ export class TerminalAutoActivation implements ITerminalAutoActivation {
7069
private async activateTerminal(terminal: Terminal): Promise<void> {
7170
// If we have just one workspace, then pass that as the resource.
7271
// Until upstream VSC issue is resolved https://github.com/Microsoft/vscode/issues/63052.
73-
await this.activator.activateEnvironmentInTerminal(terminal, this.getActiveResource());
74-
}
75-
76-
private getActiveResource(): Uri | undefined {
77-
if (this.documentManager.activeTextEditor && !this.documentManager.activeTextEditor.document.isUntitled) {
78-
return this.documentManager.activeTextEditor.document.uri;
79-
}
80-
81-
return Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0 ? this.workspaceService.workspaceFolders[0].uri : undefined;
72+
await this.activator.activateEnvironmentInTerminal(terminal, this.activeResourceService.getActiveResource());
8273
}
8374
}

‎src/test/activation/activationManager.unit.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import { ExtensionActivationManager } from '../../client/activation/activationMa
1111
import { LanguageServerExtensionActivationService } from '../../client/activation/activationService';
1212
import { IExtensionActivationService } from '../../client/activation/types';
1313
import { IApplicationDiagnostics } from '../../client/application/types';
14-
import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
14+
import { ActiveResourceService } from '../../client/common/application/activeResource';
15+
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
1516
import { WorkspaceService } from '../../client/common/application/workspace';
1617
import { PYTHON_LANGUAGE } from '../../client/common/constants';
1718
import { IDisposable } from '../../client/common/types';
@@ -41,11 +42,13 @@ suite('Activation - ActivationManager', () => {
4142
let appDiagnostics: typemoq.IMock<IApplicationDiagnostics>;
4243
let autoSelection: typemoq.IMock<IInterpreterAutoSelectionService>;
4344
let interpreterService: IInterpreterService;
45+
let activeResourceService: IActiveResourceService;
4446
let documentManager: typemoq.IMock<IDocumentManager>;
4547
let activationService1: IExtensionActivationService;
4648
let activationService2: IExtensionActivationService;
4749
setup(() => {
4850
workspaceService = mock(WorkspaceService);
51+
activeResourceService = mock(ActiveResourceService);
4952
appDiagnostics = typemoq.Mock.ofType<IApplicationDiagnostics>();
5053
autoSelection = typemoq.Mock.ofType<IInterpreterAutoSelectionService>();
5154
interpreterService = mock(InterpreterService);
@@ -58,7 +61,8 @@ suite('Activation - ActivationManager', () => {
5861
instance(interpreterService),
5962
autoSelection.object,
6063
appDiagnostics.object,
61-
instance(workspaceService)
64+
instance(workspaceService),
65+
instance(activeResourceService)
6266
);
6367
});
6468
test('Initialize will add event handlers and will dispose them when running dispose', async () => {

0 commit comments

Comments
 (0)