Skip to content

Commit 50d7e23

Browse files
Mikhail Arkhipovkarthiknadig
Mikhail Arkhipov
authored andcommitted
Add telemetry for switch to Pylance acceptance (microsoft#13526)
* Fix path * Actually fix settings * Add news * Add test * Format * Suppress 'jediEnabled' removal * Drop survey first launch threshold * Initial * Add test * PR feedback * Rename property * Rename constant * Define property * Moar rename * casing
1 parent 7a1021c commit 50d7e23

File tree

5 files changed

+65
-3
lines changed

5 files changed

+65
-3
lines changed

src/client/languageServices/proposeLanguageServerBanner.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {
1717
IPythonExtensionBanner
1818
} from '../common/types';
1919
import { Common, Pylance } from '../common/utils/localize';
20+
import { sendTelemetryEvent } from '../telemetry';
21+
import { EventName } from '../telemetry/constants';
2022

2123
export function getPylanceExtensionUri(appEnv: IApplicationEnvironment): string {
2224
return `${appEnv.uriScheme}:extension/${PYLANCE_EXTENSION_ID}`;
@@ -72,14 +74,19 @@ export class ProposePylanceBanner implements IPythonExtensionBanner {
7274
Pylance.remindMeLater()
7375
);
7476

77+
let userAction: string;
7578
if (response === Pylance.tryItNow()) {
7679
this.appShell.openUrl(getPylanceExtensionUri(this.appEnv));
80+
userAction = 'yes';
7781
await this.disable();
7882
} else if (response === Common.bannerLabelNo()) {
7983
await this.disable();
84+
userAction = 'no';
8085
} else {
8186
this.disabledInCurrentSession = true;
87+
userAction = 'later';
8288
}
89+
sendTelemetryEvent(EventName.LANGUAGE_SERVER_TRY_PYLANCE, undefined, { userAction });
8390
}
8491

8592
public async shouldShowBanner(): Promise<boolean> {

src/client/telemetry/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ export enum EventName {
9393
LANGUAGE_SERVER_READY = 'LANGUAGE_SERVER.READY',
9494
LANGUAGE_SERVER_TELEMETRY = 'LANGUAGE_SERVER.EVENT',
9595
LANGUAGE_SERVER_REQUEST = 'LANGUAGE_SERVER.REQUEST',
96+
LANGUAGE_SERVER_TRY_PYLANCE = 'LANGUAGE_SERVER.TRY_PYLANCE',
9697

9798
TERMINAL_CREATE = 'TERMINAL.CREATE',
9899
ACTIVATE_ENV_IN_CURRENT_TERMINAL = 'ACTIVATE_ENV_IN_CURRENT_TERMINAL',

src/client/telemetry/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,16 @@ export interface IEventNamePropertyMapping {
13621362
* Telemetry sent when the client makes a request to the Node.js server
13631363
*/
13641364
[EventName.LANGUAGE_SERVER_REQUEST]: any;
1365+
/**
1366+
* Telemetry sent on user response to 'Try Pylance' prompt.
1367+
*/
1368+
[EventName.LANGUAGE_SERVER_TRY_PYLANCE]: {
1369+
/**
1370+
* User response to the prompt.
1371+
* @type {string}
1372+
*/
1373+
userAction: string;
1374+
};
13651375
/**
13661376
* Telemetry captured for enabling reload.
13671377
*/

src/test/.vscode/settings.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
"python.linting.banditEnabled": false,
2525
"python.formatting.provider": "yapf",
2626
"python.linting.pylintUseMinimalCheckers": false,
27-
"python.pythonPath": "python",
2827
// Do not set this to "Microsoft", else it will result in LS being downloaded on CI
2928
// and that slows down tests significantly. We have other tests on CI for testing
3029
// downloading of LS with this setting enabled.

src/test/testing/banners/proposeNewLanguageServerBanner.unit.test.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// Licensed under the MIT License.
33

44
'use strict';
5-
6-
import { expect } from 'chai';
5+
import { assert, expect } from 'chai';
6+
import * as sinon from 'sinon';
77
import * as typemoq from 'typemoq';
88
import { Extension } from 'vscode';
99
import { LanguageServerType } from '../../../client/activation/types';
@@ -24,6 +24,8 @@ import {
2424
ProposeLSStateKeys,
2525
ProposePylanceBanner
2626
} from '../../../client/languageServices/proposeLanguageServerBanner';
27+
import * as Telemetry from '../../../client/telemetry';
28+
import { EventName } from '../../../client/telemetry/constants';
2729

2830
interface IExperimentLsCombination {
2931
inExperiment: boolean;
@@ -46,6 +48,8 @@ suite('Propose Pylance Banner', () => {
4648
let appShell: typemoq.IMock<IApplicationShell>;
4749
let appEnv: typemoq.IMock<IApplicationEnvironment>;
4850
let settings: typemoq.IMock<IPythonSettings>;
51+
let sendTelemetryStub: sinon.SinonStub;
52+
let telemetryEvent: { eventName: EventName; properties: { userAction: string } } | undefined;
4953

5054
const message = Pylance.proposePylanceMessage();
5155
const yes = Pylance.tryItNow();
@@ -59,7 +63,23 @@ suite('Propose Pylance Banner', () => {
5963
appShell = typemoq.Mock.ofType<IApplicationShell>();
6064
appEnv = typemoq.Mock.ofType<IApplicationEnvironment>();
6165
appEnv.setup((x) => x.uriScheme).returns(() => 'scheme');
66+
67+
sendTelemetryStub = sinon
68+
.stub(Telemetry, 'sendTelemetryEvent')
69+
.callsFake((eventName: EventName, _, properties: { userAction: string }) => {
70+
telemetryEvent = {
71+
eventName,
72+
properties
73+
};
74+
});
75+
});
76+
77+
teardown(() => {
78+
telemetryEvent = undefined;
79+
sinon.restore();
80+
Telemetry._resetSharedProperties();
6281
});
82+
6383
testData.forEach((t) => {
6484
test(`${t.inExperiment ? 'In' : 'Not in'} experiment and "python.languageServer": "${t.lsType}" should ${
6585
t.shouldShowBanner ? 'show' : 'not show'
@@ -109,8 +129,15 @@ suite('Propose Pylance Banner', () => {
109129

110130
const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false);
111131
await testBanner.showBanner();
132+
112133
expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled when user clicked No');
113134
appShell.verifyAll();
135+
136+
sinon.assert.calledOnce(sendTelemetryStub);
137+
assert.deepEqual(telemetryEvent, {
138+
eventName: EventName.LANGUAGE_SERVER_TRY_PYLANCE,
139+
properties: { userAction: 'no' }
140+
});
114141
});
115142
test('Clicking Later should disable banner in session', async () => {
116143
appShell
@@ -128,11 +155,20 @@ suite('Propose Pylance Banner', () => {
128155

129156
const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false);
130157
await testBanner.showBanner();
158+
131159
expect(testBanner.enabled).to.be.equal(
132160
true,
133161
'Banner should not be permanently disabled when user clicked Later'
134162
);
135163
appShell.verifyAll();
164+
165+
sinon.assert.calledOnce(sendTelemetryStub);
166+
assert.deepEqual(telemetryEvent, {
167+
eventName: EventName.LANGUAGE_SERVER_TRY_PYLANCE,
168+
properties: {
169+
userAction: 'later'
170+
}
171+
});
136172
});
137173
test('Clicking Yes opens the extension marketplace entry', async () => {
138174
appShell
@@ -150,8 +186,17 @@ suite('Propose Pylance Banner', () => {
150186

151187
const testBanner = preparePopup(true, appShell.object, appEnv.object, config.object, true, false);
152188
await testBanner.showBanner();
189+
153190
expect(testBanner.enabled).to.be.equal(false, 'Banner should be permanently disabled after opening store URL');
154191
appShell.verifyAll();
192+
193+
sinon.assert.calledOnce(sendTelemetryStub);
194+
assert.deepEqual(telemetryEvent, {
195+
eventName: EventName.LANGUAGE_SERVER_TRY_PYLANCE,
196+
properties: {
197+
userAction: 'yes'
198+
}
199+
});
155200
});
156201
});
157202

0 commit comments

Comments
 (0)