Skip to content

Commit 0cc20fe

Browse files
jfeingold35jfeingold35
committed
@W-17526120@ Added improved telemetry for A4D events
1 parent 42f009a commit 0cc20fe

9 files changed

+420
-12
lines changed

‎src/extension.ts

+28-9
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import {SfCli} from './lib/sf-cli';
1414

1515
import {Displayable, ProgressNotification, UxDisplay} from './lib/display';
1616
import {DiagnosticManager, DiagnosticConvertible, DiagnosticManagerImpl} from './lib/diagnostics';
17+
import {DiffCreateAction} from './lib/actions/diff-create-action';
18+
import { DiffAcceptAction } from './lib/actions/diff-accept-action';
19+
import { DiffRejectAction } from './lib/actions/diff-reject-action';
1720
import {ScannerAction} from './lib/actions/scanner-action';
1821
import { CliScannerV4Strategy } from './lib/scanner-strategies/v4-scanner';
1922
import { CliScannerV5Strategy } from './lib/scanner-strategies/v5-scanner';
@@ -225,21 +228,31 @@ export async function activate(context: vscode.ExtensionContext): Promise<vscode
225228
context.subscriptions.push(documentSaveListener);
226229

227230
telemetryService.sendExtensionActivationEvent(extensionHrStart);
228-
setupUnifiedDiff(context, diagnosticManager);
231+
setupUnifiedDiff(context, diagnosticManager, telemetryService);
229232
outputChannel.appendLine(`Extension sfdx-code-analyzer-vscode activated.`);
230233
return Promise.resolve(context);
231234
}
232235

233236

234-
function setupUnifiedDiff(context: vscode.ExtensionContext, diagnosticManager: DiagnosticManager) {
237+
function setupUnifiedDiff(context: vscode.ExtensionContext, diagnosticManager: DiagnosticManager, telemetryService: TelemetryService) {
235238
context.subscriptions.push(
236239
vscode.commands.registerCommand(Constants.UNIFIED_DIFF, async (code: string, file?: string) => {
240+
await (new DiffCreateAction(Constants.UNIFIED_DIFF, {
241+
callback: (code: string, file?: string) => VSCodeUnifiedDiff.singleton.unifiedDiff(code, file),
242+
telemetryService
243+
})).run(code, file);
237244
await VSCodeUnifiedDiff.singleton.unifiedDiff(code, file);
238245
})
239246
);
240247
context.subscriptions.push(
241248
vscode.commands.registerCommand(CODEGENIE_UNIFIED_DIFF_ACCEPT, async (hunk: DiffHunk) => {
242-
await VSCodeUnifiedDiff.singleton.unifiedDiffAccept(hunk);
249+
await (new DiffAcceptAction(CODEGENIE_UNIFIED_DIFF_ACCEPT, {
250+
callback: async (diffHunk: DiffHunk) => {
251+
await VSCodeUnifiedDiff.singleton.unifiedDiffAccept(diffHunk);
252+
return diffHunk.lines.length;
253+
},
254+
telemetryService
255+
})).run(hunk);
243256
// For accept & accept all, it is tricky to track the diagnostics and the changed lines as multiple fixes are requested.
244257
// Hence, we save the file and rerun the scan instead.
245258
await vscode.window.activeTextEditor.document.save();
@@ -252,12 +265,18 @@ function setupUnifiedDiff(context: vscode.ExtensionContext, diagnosticManager: D
252265
);
253266
context.subscriptions.push(
254267
vscode.commands.registerCommand(CODEGENIE_UNIFIED_DIFF_REJECT, async (hunk: DiffHunk) => {
255-
await VSCodeUnifiedDiff.singleton.unifiedDiffReject(hunk);
268+
await (new DiffRejectAction(CODEGENIE_UNIFIED_DIFF_REJECT, {
269+
callback: (diffHunk: DiffHunk) => VSCodeUnifiedDiff.singleton.unifiedDiffReject(diffHunk),
270+
telemetryService
271+
})).run(hunk);
256272
})
257273
);
258274
context.subscriptions.push(
259275
vscode.commands.registerCommand(CODEGENIE_UNIFIED_DIFF_ACCEPT_ALL, async () => {
260-
await VSCodeUnifiedDiff.singleton.unifiedDiffAcceptAll();
276+
await (new DiffAcceptAction(CODEGENIE_UNIFIED_DIFF_ACCEPT_ALL, {
277+
callback: () => VSCodeUnifiedDiff.singleton.unifiedDiffAcceptAll(),
278+
telemetryService
279+
})).run();
261280
await vscode.window.activeTextEditor.document.save();
262281
return _runAndDisplayScanner(Constants.COMMAND_RUN_ON_ACTIVE_FILE, [vscode.window.activeTextEditor.document.fileName], {
263282
telemetryService,
@@ -268,7 +287,10 @@ function setupUnifiedDiff(context: vscode.ExtensionContext, diagnosticManager: D
268287
);
269288
context.subscriptions.push(
270289
vscode.commands.registerCommand(CODEGENIE_UNIFIED_DIFF_REJECT_ALL, async () => {
271-
await VSCodeUnifiedDiff.singleton.unifiedDiffRejectAll();
290+
await (new DiffRejectAction(CODEGENIE_UNIFIED_DIFF_REJECT_ALL, {
291+
callback: () => VSCodeUnifiedDiff.singleton.unifiedDiffRejectAll(),
292+
telemetryService
293+
})).run();
272294
})
273295
);
274296
VSCodeUnifiedDiff.singleton.activate(context);
@@ -498,7 +520,6 @@ export async function _runAndDisplayScanner(commandName: string, targets: string
498520
});
499521
} catch (e) {
500522
const errMsg = e instanceof Error ? e.message : e as string;
501-
console.log(errMsg);
502523
telemetryService.sendException(Constants.TELEM_FAILED_STATIC_ANALYSIS, errMsg, {
503524
executedCommand: commandName,
504525
duration: (Date.now() - startTime).toString()
@@ -547,7 +568,6 @@ export async function _runAndDisplayDfa(context:vscode.ExtensionContext ,runInfo
547568
})
548569
} catch (e) {
549570
const errMsg = e instanceof Error ? e.message : e as string;
550-
console.log(errMsg);
551571
telemetryService.sendException(Constants.TELEM_FAILED_DFA_ANALYSIS, errMsg, {
552572
executedCommand: commandName,
553573
duration: (Date.now() - startTime).toString()
@@ -600,7 +620,6 @@ export async function _clearDiagnosticsForSelectedFiles(selections: vscode.Uri[]
600620
});
601621
} catch (e) {
602622
const errMsg = e instanceof Error ? e.message : e as string;
603-
console.log(errMsg);
604623
telemetryService.sendException(Constants.TELEM_FAILED_STATIC_ANALYSIS, errMsg, {
605624
executedCommand: commandName,
606625
duration: (Date.now() - startTime).toString()

‎src/lib/actions/diff-accept-action.ts

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { DiffHunk } from "../../shared/UnifiedDiff";
2+
import { TelemetryService } from "../core-extension-service";
3+
import * as Constants from '../constants';
4+
5+
export type DiffAcceptCallback = (diffHunk?: DiffHunk) => Promise<number>;
6+
7+
export type DiffAcceptDependencies = {
8+
callback: DiffAcceptCallback;
9+
telemetryService: TelemetryService;
10+
};
11+
12+
export class DiffAcceptAction {
13+
private readonly commandName: string;
14+
private readonly callback: DiffAcceptCallback;
15+
private readonly telemetryService: TelemetryService;
16+
17+
public constructor(commandName: string, dependencies: DiffAcceptDependencies) {
18+
this.commandName = commandName;
19+
this.callback = dependencies.callback;
20+
this.telemetryService = dependencies.telemetryService;
21+
}
22+
23+
public async run(diffHunk?: DiffHunk): Promise<void> {
24+
const startTime = Date.now();
25+
try {
26+
const lines: number = await this.callback(diffHunk);
27+
this.telemetryService.sendCommandEvent(Constants.TELEM_DIFF_ACCEPT, {
28+
commandName: this.commandName,
29+
commandSource: Constants.A4D_COMMAND_SOURCE,
30+
completionNumLines: lines.toString(),
31+
languageType: 'apex' // Apex is the only language A4D codegen supports at present.
32+
});
33+
} catch (e) {
34+
const errMsg = e instanceof Error ? e.message : e as string;
35+
this.telemetryService.sendException(Constants.TELEM_DIFF_ACCEPT_FAILED, errMsg, {
36+
executedCommand: this.commandName,
37+
duration: (Date.now() - startTime).toString()
38+
})
39+
}
40+
}
41+
}

‎src/lib/actions/diff-create-action.ts

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import {TelemetryService} from '../core-extension-service';
2+
import * as Constants from '../constants';
3+
4+
export type DiffCreateCallback = (code: string, file?: string) => Promise<void>;
5+
6+
export type DiffCreateDependencies = {
7+
callback: DiffCreateCallback;
8+
telemetryService: TelemetryService;
9+
};
10+
11+
export class DiffCreateAction {
12+
private readonly commandName: string;
13+
private readonly callback: DiffCreateCallback;
14+
private readonly telemetryService: TelemetryService;
15+
16+
public constructor(commandName: string, dependencies: DiffCreateDependencies) {
17+
this.commandName = commandName;
18+
this.callback = dependencies.callback;
19+
this.telemetryService = dependencies.telemetryService;
20+
}
21+
22+
public async run(code: string, file?: string): Promise<void> {
23+
const startTime = Date.now();
24+
try {
25+
await this.callback(code, file);
26+
this.telemetryService.sendCommandEvent(Constants.TELEM_DIFF_SUGGESTION, {
27+
commandName: this.commandName,
28+
commandSource: Constants.A4D_COMMAND_SOURCE, // NOT SURE WHAT SHOULD GO HERE.
29+
languageType: 'apex' // Apex is the only language A4D codegen supports at present.
30+
});
31+
} catch (e) {
32+
const errMsg = e instanceof Error ? e.message : e as string;
33+
this.telemetryService.sendException(Constants.TELEM_DIFF_SUGGESTION_FAILED, errMsg, {
34+
executedCommand: this.commandName,
35+
duration: (Date.now() - startTime).toString()
36+
});
37+
}
38+
}
39+
}

‎src/lib/actions/diff-reject-action.ts

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { DiffHunk } from "../../shared/UnifiedDiff";
2+
import { TelemetryService } from "../core-extension-service";
3+
import * as Constants from '../constants';
4+
5+
export type DiffRejectCallback = (diffHunk?: DiffHunk) => Promise<void>;
6+
7+
export type DiffRejectDependencies = {
8+
callback: DiffRejectCallback;
9+
telemetryService: TelemetryService;
10+
};
11+
12+
export class DiffRejectAction {
13+
private readonly commandName: string;
14+
private readonly callback: DiffRejectCallback;
15+
private readonly telemetryService: TelemetryService;
16+
17+
public constructor(commandName: string, dependencies: DiffRejectDependencies) {
18+
this.commandName = commandName;
19+
this.callback = dependencies.callback;
20+
this.telemetryService = dependencies.telemetryService;
21+
}
22+
23+
public async run(diffHunk?: DiffHunk): Promise<void> {
24+
const startTime = Date.now();
25+
try {
26+
await this.callback(diffHunk);
27+
this.telemetryService.sendCommandEvent(Constants.TELEM_DIFF_REJECT, {
28+
commandName: this.commandName,
29+
commandSource: Constants.A4D_COMMAND_SOURCE,
30+
languageType: 'apex' // Apex is the only language A4D codegen supports at present.
31+
});
32+
} catch (e) {
33+
const errMsg = e instanceof Error ? e.message : e as string;
34+
this.telemetryService.sendException(Constants.TELEM_DIFF_REJECT_FAILED, errMsg, {
35+
executedCommand: this.commandName,
36+
duration: (Date.now() - startTime).toString()
37+
})
38+
}
39+
}
40+
}

‎src/lib/constants.ts

+7
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ export const TELEM_FAILED_STATIC_ANALYSIS = 'sfdx__codeanalyzer_static_run_faile
2828
export const TELEM_SUCCESSFUL_DFA_ANALYSIS = 'sfdx__codeanalyzer_dfa_run_complete';
2929
export const TELEM_FAILED_DFA_ANALYSIS = 'sfdx__codeanalyzer_dfa_run_failed';
3030
export const TELEM_SUCCESSFUL_APEX_GURU_FILE_ANALYSIS = 'sfdx__apexguru_file_run_complete';
31+
export const TELEM_DIFF_SUGGESTION = 'sfdx__eGPT_suggest';
32+
export const TELEM_DIFF_SUGGESTION_FAILED = 'sfdx__eGPT_suggest_failure';
33+
export const TELEM_DIFF_ACCEPT = 'sfdx__eGPT_accept';
34+
export const TELEM_DIFF_ACCEPT_FAILED = 'sfdx__eGPT_accept_failure';
35+
export const TELEM_DIFF_REJECT = 'sfdx__eGPT_reject';
36+
export const TELEM_DIFF_REJECT_FAILED = 'sfdx__eGPT_reject_failure';
3137

3238
// versioning
3339
export const MINIMUM_REQUIRED_VERSION_CORE_EXTENSION = '58.4.1';
@@ -46,3 +52,4 @@ export const APEX_GURU_RETRY_INTERVAL_MILLIS = 1000;
4652
export const ENABLE_A4D_INTEGRATION = false;
4753
export const A4D_FIX_AVAILABLE_RULES = ['ApexCRUDViolation', 'ApexSharingViolations', 'EmptyCatchBlock', 'EmptyTryOrFinallyBlock', 'EmptyWhileStmt', 'EmptyIfStmt'];
4854
export const UNIFIED_DIFF = 'unifiedDiff';
55+
export const A4D_COMMAND_SOURCE = 'SalesforceCodeAnalyzer';

‎src/shared/UnifiedDiff.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -461,14 +461,16 @@ export class VSCodeUnifiedDiff implements vscode.CodeLensProvider, vscode.CodeAc
461461
/**
462462
* Accept all changes in the unified diff.
463463
*/
464-
public async unifiedDiffAcceptAll() {
464+
public async unifiedDiffAcceptAll(): Promise<number> {
465465
const editor = vscode.window.activeTextEditor;
466-
if (!editor) return;
466+
if (!editor) return 0;
467467
const diff = this.unifiedDiffs.get(editor.document.uri.toString());
468-
if (!diff) return;
468+
if (!diff) return 0;
469+
const diffLines: number = diff.getHunks().reduce((prev, curr) => prev + curr.lines.length, 0);
469470
diff.setSourceCode(diff.getTargetCode());
470471
await this.renderUnifiedDiff(editor.document);
471472
this.checkRedundantUnifiedDiff(editor.document);
473+
return diffLines;
472474
}
473475

474476
/**

‎src/test/unit/actions/diff-accept-action.test.ts

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { Properties, TelemetryService } from "../../../lib/core-extension-service";
2+
import { DiffHunk } from "../../../shared/UnifiedDiff";
3+
import { DiffAcceptAction } from '../../../lib/actions/diff-accept-action';
4+
import * as Constants from '../../../lib/constants';
5+
6+
7+
describe('DiffAcceptAction', () => {
8+
describe('Telemetry events', () => {
9+
it('Generates telemetry event for failed suggestion acceptance', async () => {
10+
const stubTelemetryService: StubTelemetryService = new StubTelemetryService();
11+
const diffAcceptAction: DiffAcceptAction = new DiffAcceptAction('fakeName', {
12+
callback: (_diffHunk?: DiffHunk) => Promise.reject(new Error('Forced error')),
13+
telemetryService: stubTelemetryService
14+
});
15+
await diffAcceptAction.run();
16+
17+
expect(stubTelemetryService.getSentCommandEvents()).toHaveLength(0);
18+
expect(stubTelemetryService.getSentExceptions()).toHaveLength(1);
19+
expect(stubTelemetryService.getSentExceptions()[0].name).toEqual(Constants.TELEM_DIFF_ACCEPT_FAILED);
20+
expect(stubTelemetryService.getSentExceptions()[0].message).toEqual('Forced error');
21+
expect(stubTelemetryService.getSentExceptions()[0].data).toHaveProperty('executedCommand', 'fakeName');
22+
});
23+
24+
it('Generates telemetry event for successful suggestion acceptance', async () => {
25+
const stubTelemetryService: StubTelemetryService = new StubTelemetryService();
26+
const diffAcceptAction: DiffAcceptAction = new DiffAcceptAction('fakeName', {
27+
callback: (_diffHunk?: DiffHunk) => Promise.resolve(5),
28+
telemetryService: stubTelemetryService
29+
});
30+
await diffAcceptAction.run();
31+
32+
expect(stubTelemetryService.getSentExceptions()).toHaveLength(0);
33+
expect(stubTelemetryService.getSentCommandEvents()).toHaveLength(1);
34+
expect(stubTelemetryService.getSentCommandEvents()[0].key).toEqual(Constants.TELEM_DIFF_ACCEPT);
35+
expect(stubTelemetryService.getSentCommandEvents()[0].data!.commandName).toEqual('fakeName');
36+
expect(stubTelemetryService.getSentCommandEvents()[0].data!.commandSource).toEqual(Constants.A4D_COMMAND_SOURCE);
37+
expect(stubTelemetryService.getSentCommandEvents()[0].data!.completionNumLines).toEqual('5');
38+
expect(stubTelemetryService.getSentCommandEvents()[0].data!.languageType).toEqual('apex');
39+
});
40+
});
41+
});
42+
43+
type TelemetryCommandEventData = {
44+
key: string;
45+
data?: Properties;
46+
};
47+
48+
type TelemetryExceptionData = {
49+
name: string;
50+
message: string;
51+
data?: Record<string, string>;
52+
};
53+
54+
class StubTelemetryService implements TelemetryService {
55+
private commandEventCalls: TelemetryCommandEventData[] = [];
56+
private exceptionCalls: TelemetryExceptionData[] = [];
57+
58+
public sendExtensionActivationEvent(_hrStart: [number, number]): void {
59+
// NO-OP
60+
}
61+
62+
public sendCommandEvent(key: string, data: Properties): void {
63+
this.commandEventCalls.push({
64+
key,
65+
data
66+
});
67+
}
68+
69+
public sendException(name: string, message: string, data?: Record<string, string>): void {
70+
this.exceptionCalls.push({
71+
name,
72+
message,
73+
data
74+
});
75+
}
76+
77+
public getSentCommandEvents(): TelemetryCommandEventData[] {
78+
return this.commandEventCalls;
79+
}
80+
81+
public getSentExceptions(): TelemetryExceptionData[] {
82+
return this.exceptionCalls;
83+
}
84+
85+
public dispose(): void {
86+
// NO-OP
87+
}
88+
}

0 commit comments

Comments
 (0)