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

@W-14219242@: Fixing behavioral issues with telemetry. #27

Merged
merged 1 commit into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ import {RuleResult} from './types';
import {DiagnosticManager} from './lib/diagnostics';
import {messages} from './lib/messages';
import {Fixer} from './lib/fixer';
import { CoreExtensionService } from './lib/core-extension-service';
import { TelemetryService } from './lib/telemetry';
import { CoreExtensionService, TelemetryService } from './lib/core-extension-service';
import * as Constants from './lib/constants';

type RunInfo = {
Expand Down Expand Up @@ -85,8 +84,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<vscode
});
});
context.subscriptions.push(runOnActiveFile, runOnSelected, runDfaOnSelectedMethod);
const telemetryService = CoreExtensionService.getTelemetryService();
telemetryService.sendExtensionActivationEvent(extensionHrStart);
TelemetryService.sendExtensionActivationEvent(extensionHrStart);
return Promise.resolve(context);
}

Expand Down Expand Up @@ -240,6 +238,5 @@ async function summarizeResultsAsToast(targets: string[], results: RuleResult[])

// This method is called when your extension is deactivated
export function deactivate() {
const telemetryService = CoreExtensionService.getTelemetryService();
telemetryService.dispose();
TelemetryService.dispose();
}
34 changes: 27 additions & 7 deletions src/lib/core-extension-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { CORE_EXTENSION_ID, MINIMUM_REQUIRED_VERSION_CORE_EXTENSION } from './co
*/
export class CoreExtensionService {
private static initialized = false;
private static telemetryService: TelemetryService;
private static telemetryService: CoreTelemetryService;

public static async loadDependencies(context: vscode.ExtensionContext): Promise<void> {
if (!CoreExtensionService.initialized) {
Expand Down Expand Up @@ -61,11 +61,11 @@ export class CoreExtensionService {
}

/**
* Initializes a {@link TelemetryService} instance of the provided class, or a {@link NoOpTelemetryService} if none was provided
* Initializes a {@link CoreTelemetryService} instance of the provided class, or uses null if none was provided.
* @param telemetryService
* @param context
*/
private static async initializeTelemetryService(telemetryService: TelemetryService | undefined, context: vscode.ExtensionContext): Promise<void> {
private static async initializeTelemetryService(telemetryService: CoreTelemetryService | undefined, context: vscode.ExtensionContext): Promise<void> {
if (!telemetryService) {
console.log(`Telemetry service not present in core dependency API. Using null instead.`);
CoreExtensionService.telemetryService = null;
Expand All @@ -79,14 +79,34 @@ export class CoreExtensionService {
*
* @returns The {@link TelemetryService} object exported by the Core Extension if available, else null.
*/
public static getTelemetryService(): TelemetryService|null {
public static _getTelemetryService(): CoreTelemetryService|null {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The leading underscore is a JS social convention indicating that the method should be treated by developers as private even though it's technically public.

if (CoreExtensionService.initialized) {
return CoreExtensionService.telemetryService;
}
throw new Error(messages.error.coreExtensionServiceUninitialized);
}
}

export class TelemetryService {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving TelemetryService into core-extension-service.ts allowed me to make it so all telemetry events have to go through this class instead of calling CoreExtensionService._getTelemetryService() directly.


public static sendExtensionActivationEvent(hrStart: [number, number]): void {
CoreExtensionService._getTelemetryService()?.sendExtensionActivationEvent(hrStart);
}

public static sendCommandEvent(key: string, data: Properties): void {
CoreExtensionService._getTelemetryService()?.sendCommandEvent(key, undefined, data);
}

public static sendException(name: string, message: string, data?: Record<string, string>): void {
const fullMessage = data ? message + JSON.stringify(data) : message;
CoreExtensionService._getTelemetryService()?.sendException(name, fullMessage);
}

public static dispose(): void {
CoreExtensionService._getTelemetryService()?.dispose();
}
}


interface Measurements {
[key: string]: number;
Expand All @@ -99,10 +119,10 @@ export interface Properties {
/**
* This interface is a subset of the TelemetryService interface from the salesforcedx-utils-vscode package.
*/
interface TelemetryService {
interface CoreTelemetryService {
extensionName: string;
isTelemetryEnabled(): boolean;
getInstance(): TelemetryService;
getInstance(): CoreTelemetryService;
initializeService(extensionContext: vscode.ExtensionContext): Promise<void>;
sendExtensionActivationEvent(hrstart: [number, number]): void;
sendExtensionDeactivationEvent(): void;
Expand All @@ -118,6 +138,6 @@ interface TelemetryService {

interface CoreExtensionApi {
services: {
TelemetryService: TelemetryService;
TelemetryService: CoreTelemetryService;
}
}
24 changes: 0 additions & 24 deletions src/lib/telemetry.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/test/suite/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {SfCli} from '../../lib/sf-cli';
import Sinon = require('sinon');
import { _runAndDisplayPathless, _runAndDisplayDfa, _clearDiagnostics } from '../../extension';
import {messages} from '../../lib/messages';
import {TelemetryService} from '../../lib/telemetry';
import {TelemetryService} from '../../lib/core-extension-service';
import * as Constants from '../../lib/constants';

// You can import and use all API from the 'vscode' module
Expand Down