Skip to content

Commit 4d539ba

Browse files
kimadelinekimadeline
authored
Send telemetry when an interpreter was selected, or the quickpick dismissed (#15223)
* Replace shouldResume logic in multistep input * Send telemetry event on selection or dismissal * Add telemetry tests * Linting
1 parent 568e098 commit 4d539ba

6 files changed

Lines changed: 125 additions & 102 deletions

File tree

‎.eslintignore‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ src/test/interpreters/display/interpreterSelectionTip.unit.test.ts
6363
src/test/interpreters/display.unit.test.ts
6464

6565
src/test/configuration/interpreterSelector/interpreterSelector.unit.test.ts
66-
src/test/configuration/interpreterSelector/commands/setInterpreter.unit.test.ts
6766

6867
src/test/install/channelManager.channels.test.ts
6968
src/test/install/channelManager.messages.test.ts
@@ -322,7 +321,6 @@ src/test/workspaceSymbols/generator.unit.test.ts
322321
src/client/interpreter/interpreterService.ts
323322
src/client/interpreter/configuration/interpreterComparer.ts
324323
src/client/interpreter/configuration/interpreterSelector/commands/base.ts
325-
src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts
326324
src/client/interpreter/configuration/interpreterSelector/commands/resetInterpreter.ts
327325
src/client/interpreter/configuration/interpreterSelector/commands/setShebangInterpreter.ts
328326
src/client/interpreter/configuration/interpreterSelector/interpreterSelector.ts
@@ -570,7 +568,6 @@ src/client/common/utils/enum.ts
570568
src/client/common/utils/async.ts
571569
src/client/common/utils/localize.ts
572570
src/client/common/utils/platform.ts
573-
src/client/common/utils/multiStepInput.ts
574571
src/client/common/utils/stopWatch.ts
575572
src/client/common/utils/random.ts
576573
src/client/common/utils/serializers.ts

‎src/client/common/utils/multiStepInput.ts‎

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

4+
/* eslint-disable max-classes-per-file */
5+
46
'use strict';
57

68
import { inject, injectable } from 'inversify';
@@ -12,11 +14,17 @@ import { IApplicationShell } from '../application/types';
1214

1315
export class InputFlowAction {
1416
public static back = new InputFlowAction();
17+
1518
public static cancel = new InputFlowAction();
19+
1620
public static resume = new InputFlowAction();
17-
private constructor() {}
21+
22+
private constructor() {
23+
/** No body. */
24+
}
1825
}
1926

27+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2028
export type InputStep<T extends any> = (input: MultiStepInput<T>, state: T) => Promise<InputStep<T> | void>;
2129

2230
export interface IQuickPickParameters<T extends QuickPickItem> {
@@ -31,7 +39,6 @@ export interface IQuickPickParameters<T extends QuickPickItem> {
3139
matchOnDescription?: boolean;
3240
matchOnDetail?: boolean;
3341
acceptFilterBoxTextAsSelection?: boolean;
34-
shouldResume?(): Promise<boolean>;
3542
}
3643

3744
export interface InputBoxParameters {
@@ -43,11 +50,10 @@ export interface InputBoxParameters {
4350
prompt: string;
4451
buttons?: QuickInputButton[];
4552
validate(value: string): Promise<string | undefined>;
46-
shouldResume?(): Promise<boolean>;
4753
}
4854

49-
type MultiStepInputQuickPicResponseType<T, P> = T | (P extends { buttons: (infer I)[] } ? I : never);
50-
type MultiStepInputInputBoxResponseType<P> = string | (P extends { buttons: (infer I)[] } ? I : never);
55+
type MultiStepInputQuickPicResponseType<T, P> = T | (P extends { buttons: (infer I)[] } ? I : never) | undefined;
56+
type MultiStepInputInputBoxResponseType<P> = string | (P extends { buttons: (infer I)[] } ? I : never) | undefined;
5157
export interface IMultiStepInput<S> {
5258
run(start: InputStep<S>, state: S): Promise<void>;
5359
showQuickPick<T extends QuickPickItem, P extends IQuickPickParameters<T>>({
@@ -58,7 +64,6 @@ export interface IMultiStepInput<S> {
5864
activeItem,
5965
placeholder,
6066
buttons,
61-
shouldResume,
6267
}: P): Promise<MultiStepInputQuickPicResponseType<T, P>>;
6368
showInputBox<P extends InputBoxParameters>({
6469
title,
@@ -68,15 +73,17 @@ export interface IMultiStepInput<S> {
6873
prompt,
6974
validate,
7075
buttons,
71-
shouldResume,
7276
}: P): Promise<MultiStepInputInputBoxResponseType<P>>;
7377
}
7478

7579
export class MultiStepInput<S> implements IMultiStepInput<S> {
7680
private current?: QuickInput;
81+
7782
private steps: InputStep<S>[] = [];
83+
7884
constructor(private readonly shell: IApplicationShell) {}
79-
public run(start: InputStep<S>, state: S) {
85+
86+
public run(start: InputStep<S>, state: S): Promise<void> {
8087
return this.stepThrough(start, state);
8188
}
8289

@@ -88,7 +95,6 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
8895
activeItem,
8996
placeholder,
9097
buttons,
91-
shouldResume,
9298
matchOnDescription,
9399
matchOnDetail,
94100
acceptFilterBoxTextAsSelection,
@@ -116,24 +122,20 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
116122
if (item === QuickInputButtons.Back) {
117123
reject(InputFlowAction.back);
118124
} else {
119-
resolve(<any>item);
125+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
126+
resolve(item as any);
120127
}
121128
}),
122129
input.onDidChangeSelection((selectedItems) => resolve(selectedItems[0])),
123130
input.onDidHide(() => {
124-
(async () => {
125-
reject(
126-
shouldResume && (await shouldResume())
127-
? InputFlowAction.resume
128-
: InputFlowAction.cancel,
129-
);
130-
})().catch(reject);
131+
resolve(undefined);
131132
}),
132133
);
133134
if (acceptFilterBoxTextAsSelection) {
134135
disposables.push(
135136
input.onDidAccept(() => {
136-
resolve(<any>input.value);
137+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
138+
resolve(input.value as any);
137139
}),
138140
);
139141
}
@@ -157,7 +159,6 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
157159
validate,
158160
password,
159161
buttons,
160-
shouldResume,
161162
}: P): Promise<MultiStepInputInputBoxResponseType<P>> {
162163
const disposables: Disposable[] = [];
163164
try {
@@ -166,7 +167,7 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
166167
input.title = title;
167168
input.step = step;
168169
input.totalSteps = totalSteps;
169-
input.password = password ? true : false;
170+
input.password = !!password;
170171
input.value = value || '';
171172
input.prompt = prompt;
172173
input.ignoreFocusOut = true;
@@ -177,7 +178,8 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
177178
if (item === QuickInputButtons.Back) {
178179
reject(InputFlowAction.back);
179180
} else {
180-
resolve(<any>item);
181+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
182+
resolve(item as any);
181183
}
182184
}),
183185
input.onDidAccept(async () => {
@@ -199,13 +201,7 @@ export class MultiStepInput<S> implements IMultiStepInput<S> {
199201
}
200202
}),
201203
input.onDidHide(() => {
202-
(async () => {
203-
reject(
204-
shouldResume && (await shouldResume())
205-
? InputFlowAction.resume
206-
: InputFlowAction.cancel,
207-
);
208-
})().catch(reject);
204+
resolve(undefined);
209205
}),
210206
);
211207
if (this.current) {
@@ -254,6 +250,7 @@ export interface IMultiStepInputFactory {
254250
@injectable()
255251
export class MultiStepInputFactory {
256252
constructor(@inject(IApplicationShell) private readonly shell: IApplicationShell) {}
253+
257254
public create<S>(): IMultiStepInput<S> {
258255
return new MultiStepInput<S>(this.shell);
259256
}

‎src/client/interpreter/configuration/interpreterSelector/commands/setInterpreter.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
3939
super(pythonPathUpdaterService, commandManager, applicationShell, workspaceService);
4040
}
4141

42-
public async activate() {
42+
public async activate(): Promise<void> {
4343
this.disposables.push(
4444
this.commandManager.registerCommand(Commands.Set_Interpreter, this.setInterpreter.bind(this)),
4545
);
@@ -74,12 +74,15 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
7474
});
7575

7676
if (selection === undefined) {
77-
return;
77+
sendTelemetryEvent(EventName.SELECT_INTERPRETER_SELECTED, undefined, { action: 'escape' });
7878
} else if (selection.label === enterInterpreterPathSuggestion.label) {
7979
return this._enterOrBrowseInterpreterPath(input, state);
8080
} else {
81+
sendTelemetryEvent(EventName.SELECT_INTERPRETER_SELECTED, undefined, { action: 'selected' });
8182
state.path = (selection as IInterpreterQuickPickItem).path;
8283
}
84+
85+
return undefined;
8386
}
8487

8588
@captureTelemetry(EventName.SELECT_INTERPRETER_ENTER_BUTTON)
@@ -122,16 +125,18 @@ export class SetInterpreterCommand extends BaseInterpreterSelectorCommand {
122125
}
123126

124127
@captureTelemetry(EventName.SELECT_INTERPRETER)
125-
public async setInterpreter() {
128+
public async setInterpreter(): Promise<void> {
126129
const targetConfig = await this.getConfigTarget();
127130
if (!targetConfig) {
128131
return;
129132
}
130-
const configTarget = targetConfig.configTarget;
133+
134+
const { configTarget } = targetConfig;
131135
const wkspace = targetConfig.folderUri;
132136
const interpreterState: InterpreterStateArgs = { path: undefined, workspace: wkspace };
133137
const multiStep = this.multiStepFactory.create<InterpreterStateArgs>();
134138
await multiStep.run((input, s) => this._pickInterpreter(input, s), interpreterState);
139+
135140
if (interpreterState.path !== undefined) {
136141
// User may choose to have an empty string stored, so variable `interpreterState.path` may be
137142
// an empty string, in which case we should update.

‎src/client/telemetry/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export enum EventName {
2424
SELECT_INTERPRETER = 'SELECT_INTERPRETER',
2525
SELECT_INTERPRETER_ENTER_BUTTON = 'SELECT_INTERPRETER_ENTER_BUTTON',
2626
SELECT_INTERPRETER_ENTER_CHOICE = 'SELECT_INTERPRETER_ENTER_CHOICE',
27+
SELECT_INTERPRETER_SELECTED = 'SELECT_INTERPRETER_SELECTED',
2728
PYTHON_INTERPRETER = 'PYTHON_INTERPRETER',
2829
PYTHON_INSTALL_PACKAGE = 'PYTHON_INSTALL_PACKAGE',
2930
PYTHON_INTERPRETER_DISCOVERY = 'PYTHON_INTERPRETER_DISCOVERY',

‎src/client/telemetry/index.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,17 @@ export interface IEventNamePropertyMapping {
910910
*/
911911
choice: 'enter' | 'browse';
912912
};
913+
/**
914+
* Telemetry event sent after an action has been taken while the interpreter quickpick was displayed,
915+
* and if the action was not 'Enter interpreter path'.
916+
*/
917+
[EventName.SELECT_INTERPRETER_SELECTED]: {
918+
/**
919+
* 'escape' if the quickpick was dismissed.
920+
* 'selected' if an interpreter was selected.
921+
*/
922+
action: 'escape' | 'selected';
923+
};
913924
/**
914925
* Telemetry event sent with details after updating the python interpreter
915926
*/

0 commit comments

Comments
 (0)