Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added telemetry for usage of activateEnvInCurrentTerminal setting #8654

Merged
merged 15 commits into from
Nov 25, 2019
1 change: 1 addition & 0 deletions news/1 Enhancements/8004.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add telemetry for usage of activateEnvInCurrentTerminal setting.
17 changes: 5 additions & 12 deletions src/client/activation/activationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
'use strict';

import { inject, injectable, multiInject } from 'inversify';
import { TextDocument, workspace } from 'vscode';
import { TextDocument } from 'vscode';
import { IApplicationDiagnostics } from '../application/types';
import { IDocumentManager, IWorkspaceService } from '../common/application/types';
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../common/application/types';
import { PYTHON_LANGUAGE } from '../common/constants';
import { traceDecorators } from '../common/logger';
import { IDisposable, Resource } from '../common/types';
Expand All @@ -26,7 +26,8 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IInterpreterAutoSelectionService) private readonly autoSelection: IInterpreterAutoSelectionService,
@inject(IApplicationDiagnostics) private readonly appDiagnostics: IApplicationDiagnostics,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService
) { }

public dispose() {
Expand All @@ -44,7 +45,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
// Activate all activation services together.
await Promise.all([
Promise.all(this.singleActivationServices.map(item => item.activate())),
this.activateWorkspace(this.getActiveResource())
this.activateWorkspace(this.activeResourceService.getActiveResource())
]);
await this.autoSelection.autoSelectInterpreter(undefined);
}
Expand Down Expand Up @@ -114,12 +115,4 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
protected getWorkspaceKey(resource: Resource) {
return this.workspaceService.getWorkspaceFolderIdentifier(resource, '');
}
private getActiveResource(): Resource {
if (this.documentManager.activeTextEditor && !this.documentManager.activeTextEditor.document.isUntitled) {
return this.documentManager.activeTextEditor.document.uri;
}
return Array.isArray(this.workspaceService.workspaceFolders) && workspace.workspaceFolders!.length > 0
? workspace.workspaceFolders![0].uri
: undefined;
}
}
3 changes: 3 additions & 0 deletions src/client/activation/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

'use strict';

import { ActiveResourceService } from '../common/application/activeResource';
import { IActiveResourceService } from '../common/application/types';
import { INugetRepository } from '../common/nuget/types';
import { BANNER_NAME_DS_SURVEY, BANNER_NAME_INTERACTIVE_SHIFTENTER, BANNER_NAME_LS_SURVEY, BANNER_NAME_PROPOSE_LS, IPythonExtensionBanner } from '../common/types';
import { DataScienceSurveyBanner } from '../datascience/dataScienceSurveyBanner';
Expand Down Expand Up @@ -60,5 +62,6 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.add<ILanguageServerManager>(ILanguageServerManager, LanguageServerManager);
serviceManager.addSingleton<ILanguageServerOutputChannel>(ILanguageServerOutputChannel, LanguageServerOutputChannel);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, ExtensionSurveyPrompt);
serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, AATesting);
}
26 changes: 26 additions & 0 deletions src/client/common/application/activeResource.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

'use strict';

import { inject, injectable } from 'inversify';
import { Resource } from '../types';
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from './types';

@injectable()
export class ActiveResourceService implements IActiveResourceService {
constructor(
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService
) { }

public getActiveResource(): Resource {
const editor = this.documentManager.activeTextEditor;
if (editor && !editor.document.isUntitled) {
return editor.document.uri;
}
return Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0
? this.workspaceService.workspaceFolders[0].uri
: undefined;
}
}
8 changes: 8 additions & 0 deletions src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1017,3 +1017,11 @@ export interface ILanguageService {
}

export type Channel = 'stable' | 'insiders';

/**
* Wraps the `ActiveResourceService` API class. Created for injecting and mocking class methods in testing
*/
export const IActiveResourceService = Symbol('IActiveResourceService');
export interface IActiveResourceService {
getActiveResource(): Resource;
}
18 changes: 5 additions & 13 deletions src/client/providers/replProvider.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Disposable, Uri } from 'vscode';
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types';
import { Disposable } from 'vscode';
import { IActiveResourceService, ICommandManager } from '../common/application/types';
import { Commands } from '../common/constants';
import { IServiceContainer } from '../ioc/types';
import { captureTelemetry } from '../telemetry';
Expand All @@ -8,7 +8,9 @@ import { ICodeExecutionService } from '../terminals/types';

export class ReplProvider implements Disposable {
private readonly disposables: Disposable[] = [];
private activeResourceService: IActiveResourceService;
constructor(private serviceContainer: IServiceContainer) {
this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
this.registerCommand();
}
public dispose() {
Expand All @@ -21,18 +23,8 @@ export class ReplProvider implements Disposable {
}
@captureTelemetry(EventName.REPL)
private async commandHandler() {
const resource = this.getActiveResourceUri();
const resource = this.activeResourceService.getActiveResource();
const replProvider = this.serviceContainer.get<ICodeExecutionService>(ICodeExecutionService, 'repl');
await replProvider.initializeRepl(resource);
}
private getActiveResourceUri(): Uri | undefined {
const documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
if (documentManager.activeTextEditor && !documentManager.activeTextEditor!.document.isUntitled) {
return documentManager.activeTextEditor!.document.uri;
}
const workspace = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
if (Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0) {
return workspace.workspaceFolders[0].uri;
}
}
}
34 changes: 18 additions & 16 deletions src/client/providers/terminalProvider.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,37 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { Disposable, Terminal, Uri } from 'vscode';
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types';
import { Disposable, Terminal } from 'vscode';
import { IActiveResourceService, ICommandManager } from '../common/application/types';
import { Commands } from '../common/constants';
import { ITerminalActivator, ITerminalServiceFactory } from '../common/terminal/types';
import { IConfigurationService } from '../common/types';
import { swallowExceptions } from '../common/utils/decorators';
import { IServiceContainer } from '../ioc/types';
import { captureTelemetry } from '../telemetry';
import { captureTelemetry, sendTelemetryEvent } from '../telemetry';
import { EventName } from '../telemetry/constants';

export class TerminalProvider implements Disposable {
private disposables: Disposable[] = [];
private activeResourceService: IActiveResourceService;
constructor(private serviceContainer: IServiceContainer) {
this.registerCommands();
this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
}

@swallowExceptions('Failed to initialize terminal provider')
public async initialize(currentTerminal: Terminal | undefined) {
const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
const pythonSettings = configuration.getSettings();
const pythonSettings = configuration.getSettings(
this.activeResourceService.getActiveResource()
);

if (pythonSettings.terminal.activateEnvInCurrentTerminal && currentTerminal) {
const terminalActivator = this.serviceContainer.get<ITerminalActivator>(ITerminalActivator);
await terminalActivator.activateEnvironmentInTerminal(currentTerminal, undefined, true);
if (pythonSettings.terminal.activateEnvInCurrentTerminal) {
if (currentTerminal) {
const terminalActivator = this.serviceContainer.get<ITerminalActivator>(ITerminalActivator);
await terminalActivator.activateEnvironmentInTerminal(currentTerminal, undefined, true);
}
sendTelemetryEvent(EventName.ACTIVATE_ENV_IN_CURRENT_TERMINAL, undefined, { isTerminalVisible: currentTerminal ? true : false });
karrtikr marked this conversation as resolved.
Show resolved Hide resolved
}
}
public dispose() {
Expand All @@ -36,15 +46,7 @@ export class TerminalProvider implements Disposable {
@captureTelemetry(EventName.TERMINAL_CREATE, { triggeredBy: 'commandpalette' })
private async onCreateTerminal() {
const terminalService = this.serviceContainer.get<ITerminalServiceFactory>(ITerminalServiceFactory);
const activeResource = this.getActiveResource();
const activeResource = this.activeResourceService.getActiveResource();
await terminalService.createTerminalService(activeResource, 'Python').show(false);
}
private getActiveResource(): Uri | undefined {
const documentManager = this.serviceContainer.get<IDocumentManager>(IDocumentManager);
if (documentManager.activeTextEditor && !documentManager.activeTextEditor.document.isUntitled) {
return documentManager.activeTextEditor.document.uri;
}
const workspace = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
return Array.isArray(workspace.workspaceFolders) && workspace.workspaceFolders.length > 0 ? workspace.workspaceFolders[0].uri : undefined;
}
}
1 change: 1 addition & 0 deletions src/client/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export enum EventName {
EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT',

TERMINAL_CREATE = 'TERMINAL.CREATE',
ACTIVATE_ENV_IN_CURRENT_TERMINAL = 'ACTIVATE_ENV_IN_CURRENT_TERMINAL',
PYTHON_LANGUAGE_SERVER_LIST_BLOB_STORE_PACKAGES = 'PYTHON_LANGUAGE_SERVER.LIST_BLOB_PACKAGES',
DIAGNOSTICS_ACTION = 'DIAGNOSTICS.ACTION',
DIAGNOSTICS_MESSAGE = 'DIAGNOSTICS.MESSAGE',
Expand Down
10 changes: 10 additions & 0 deletions src/client/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,16 @@ export interface IEventNamePropertyMapping {
*/
failed: boolean;
};
/**
* Telemetry event sent when the extension is activated, if an active terminal is present and
* the `python.terminal.activateEnvInCurrentTerminal` setting is set to `true`.
*/
[EventName.ACTIVATE_ENV_IN_CURRENT_TERMINAL]: {
/**
* Carries boolean `true` if an active terminal is present (terminal is visible), `false` otherwise
*/
isTerminalVisible?: boolean;
};
/**
* Telemetry event sent with details when a terminal is created
*/
Expand Down
17 changes: 4 additions & 13 deletions src/client/terminals/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
'use strict';

import { inject, injectable } from 'inversify';
import { Terminal, Uri } from 'vscode';
import { Terminal } from 'vscode';
import { IExtensionSingleActivationService } from '../activation/types';
import {
ICommandManager, IDocumentManager, ITerminalManager, IWorkspaceService
IActiveResourceService, ICommandManager, ITerminalManager
} from '../common/application/types';
import { CODE_RUNNER_EXTENSION_ID } from '../common/constants';
import { ITerminalActivator } from '../common/terminal/types';
Expand Down Expand Up @@ -49,9 +49,8 @@ export class TerminalAutoActivation implements ITerminalAutoActivation {
constructor(
@inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
@inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry,
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
@inject(ITerminalActivator) private readonly activator: ITerminalActivator,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService
) {
disposableRegistry.push(this);
}
Expand All @@ -70,14 +69,6 @@ export class TerminalAutoActivation implements ITerminalAutoActivation {
private async activateTerminal(terminal: Terminal): Promise<void> {
// If we have just one workspace, then pass that as the resource.
// Until upstream VSC issue is resolved https://github.com/Microsoft/vscode/issues/63052.
await this.activator.activateEnvironmentInTerminal(terminal, this.getActiveResource());
}

private getActiveResource(): Uri | undefined {
if (this.documentManager.activeTextEditor && !this.documentManager.activeTextEditor.document.isUntitled) {
return this.documentManager.activeTextEditor.document.uri;
}

return Array.isArray(this.workspaceService.workspaceFolders) && this.workspaceService.workspaceFolders.length > 0 ? this.workspaceService.workspaceFolders[0].uri : undefined;
await this.activator.activateEnvironmentInTerminal(terminal, this.activeResourceService.getActiveResource());
}
}
8 changes: 6 additions & 2 deletions src/test/activation/activationManager.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import { ExtensionActivationManager } from '../../client/activation/activationMa
import { LanguageServerExtensionActivationService } from '../../client/activation/activationService';
import { IExtensionActivationService } from '../../client/activation/types';
import { IApplicationDiagnostics } from '../../client/application/types';
import { IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
import { ActiveResourceService } from '../../client/common/application/activeResource';
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../../client/common/application/types';
import { WorkspaceService } from '../../client/common/application/workspace';
import { PYTHON_LANGUAGE } from '../../client/common/constants';
import { IDisposable } from '../../client/common/types';
Expand Down Expand Up @@ -41,11 +42,13 @@ suite('Activation - ActivationManager', () => {
let appDiagnostics: typemoq.IMock<IApplicationDiagnostics>;
let autoSelection: typemoq.IMock<IInterpreterAutoSelectionService>;
let interpreterService: IInterpreterService;
let activeResourceService: IActiveResourceService;
let documentManager: typemoq.IMock<IDocumentManager>;
let activationService1: IExtensionActivationService;
let activationService2: IExtensionActivationService;
setup(() => {
workspaceService = mock(WorkspaceService);
activeResourceService = mock(ActiveResourceService);
appDiagnostics = typemoq.Mock.ofType<IApplicationDiagnostics>();
autoSelection = typemoq.Mock.ofType<IInterpreterAutoSelectionService>();
interpreterService = mock(InterpreterService);
Expand All @@ -58,7 +61,8 @@ suite('Activation - ActivationManager', () => {
instance(interpreterService),
autoSelection.object,
appDiagnostics.object,
instance(workspaceService)
instance(workspaceService),
instance(activeResourceService)
);
});
test('Initialize will add event handlers and will dispose them when running dispose', async () => {
Expand Down
Loading