Skip to content

Commit

Permalink
chore: telemetry service refactor (#5226)
Browse files Browse the repository at this point in the history
* feat: telemetryProvider

* fix: typos

* chore: compatible with old e4d telemetryService usage

* chore: unit tests for telemetry

* chore: lint

* chore: polish getInstance

* chore: header

* chore: header

* chore: remove change in prettierrc

* chore: rename TelemetryProvider

* chore: use getInstance from TelemetryService instead

* chore: revert change in configUtil

* chore: revert change in prettierrc

* chore: update telemetry reporter name

* chore: add explanation

* chore: unit test

* chore: save pack name in constants

* fix: delete unused env

* Update packages/salesforcedx-utils-vscode/src/telemetry/telemetry.ts

Co-authored-by: peternhale <[email protected]>

---------

Co-authored-by: peternhale <[email protected]>
  • Loading branch information
mingxuanzhangsfdx and peternhale authored Dec 1, 2023
1 parent 9a4bae6 commit 5cde4b6
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ import { ChannelService } from './channelService';
import { notificationService, ProgressNotification } from './index';

export abstract class SfdxCommandletExecutor<T>
implements CommandletExecutor<T> {
implements CommandletExecutor<T>
{
private outputChannel?: vscode.OutputChannel;
protected showChannelOutput = true;
protected executionCwd = getRootWorkspacePath();
protected onDidFinishExecutionEventEmitter = new vscode.EventEmitter<
[number, number]
>();
public readonly onDidFinishExecution: vscode.Event<[number, number]> = this
.onDidFinishExecutionEventEmitter.event;
public readonly onDidFinishExecution: vscode.Event<[number, number]> =
this.onDidFinishExecutionEventEmitter.event;

constructor(outputChannel?: vscode.OutputChannel) {
this.outputChannel = outputChannel;
Expand Down Expand Up @@ -121,7 +122,8 @@ export abstract class SfdxCommandletExecutor<T>
}

export abstract class LibraryCommandletExecutor<T>
implements CommandletExecutor<T> {
implements CommandletExecutor<T>
{
protected cancellable: boolean = false;
private cancelled: boolean = false;
private readonly executionName: string;
Expand Down
48 changes: 27 additions & 21 deletions packages/salesforcedx-utils-vscode/src/config/configUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import {
StateAggregator
} from '@salesforce/core';
import { workspaceUtils } from '..';
import { SF_CONFIG_DISABLE_TELEMETRY, TARGET_DEV_HUB_KEY, TARGET_ORG_KEY } from '../constants';
import {
SF_CONFIG_DISABLE_TELEMETRY,
TARGET_DEV_HUB_KEY,
TARGET_ORG_KEY
} from '../constants';
import { ConfigAggregatorProvider } from '../providers';
import { TelemetryService } from '../telemetry/telemetry';

Expand All @@ -25,7 +29,8 @@ export enum ConfigSource {

export class ConfigUtil {
public static async getConfigSource(key: string): Promise<ConfigSource> {
const configAggregator = await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const configAggregator =
await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const configSource = configAggregator.getLocation(key);
switch (configSource) {
case ConfigAggregator.Location.LOCAL:
Expand All @@ -47,7 +52,8 @@ export class ConfigUtil {
public static async getUserConfiguredApiVersion(): Promise<
string | undefined
> {
const configAggregator = await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const configAggregator =
await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const apiVersion = configAggregator.getPropertyValue(
OrgConfigProperties.ORG_API_VERSION
);
Expand All @@ -56,10 +62,10 @@ export class ConfigUtil {

public static async getDefaultUsernameOrAlias(): Promise<string | undefined> {
try {
const configAggregator = await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const defaultUsernameOrAlias = configAggregator.getPropertyValue(
TARGET_ORG_KEY
);
const configAggregator =
await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const defaultUsernameOrAlias =
configAggregator.getPropertyValue(TARGET_ORG_KEY);
if (!defaultUsernameOrAlias) {
return undefined;
}
Expand All @@ -73,27 +79,28 @@ export class ConfigUtil {
err.message
);
}
throw(err);
throw err;
}
}

public static async isGlobalDefaultUsername(): Promise<boolean> {
const configSource: ConfigSource = await ConfigUtil.getConfigSource(
TARGET_ORG_KEY
);
const configSource: ConfigSource =
await ConfigUtil.getConfigSource(TARGET_ORG_KEY);
return configSource === ConfigSource.Global;
}

public static async getTemplatesDirectory(): Promise<string | undefined> {
const configAggregator = await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const configAggregator =
await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const templatesDirectory = configAggregator.getPropertyValue(
OrgConfigProperties.ORG_CUSTOM_METADATA_TEMPLATES
);
return templatesDirectory ? String(templatesDirectory) : undefined;
}

public static async isTelemetryDisabled(): Promise<boolean> {
const configAggregator = await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const configAggregator =
await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const isTelemetryDisabled = configAggregator.getPropertyValue(
SF_CONFIG_DISABLE_TELEMETRY
);
Expand All @@ -103,20 +110,18 @@ export class ConfigUtil {
public static async getDefaultDevHubUsernameOrAlias(): Promise<
string | undefined
> {
const configAggregator = await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const defaultDevHubUserName = configAggregator.getPropertyValue(
TARGET_DEV_HUB_KEY
);
const configAggregator =
await ConfigAggregatorProvider.getInstance().getConfigAggregator();
const defaultDevHubUserName =
configAggregator.getPropertyValue(TARGET_DEV_HUB_KEY);
return defaultDevHubUserName ? String(defaultDevHubUserName) : undefined;
}

public static async getGlobalDefaultDevHubUsernameOrAlias(): Promise<
string | undefined
> {
const globalConfig = await Config.create({ isGlobal: true });
const defaultGlobalDevHubUserName = globalConfig.get(
TARGET_DEV_HUB_KEY
);
const defaultGlobalDevHubUserName = globalConfig.get(TARGET_DEV_HUB_KEY);

return defaultGlobalDevHubUserName
? String(defaultGlobalDevHubUserName)
Expand Down Expand Up @@ -154,7 +159,8 @@ export class ConfigUtil {
* Org if it exists.
*/
public static async getDevHubUsername(): Promise<string | undefined> {
const defaultDevHubUsernameOrAlias = await ConfigUtil.getDefaultDevHubUsernameOrAlias();
const defaultDevHubUsernameOrAlias =
await ConfigUtil.getDefaultDevHubUsernameOrAlias();
if (!defaultDevHubUsernameOrAlias) {
return;
}
Expand Down
1 change: 1 addition & 0 deletions packages/salesforcedx-utils-vscode/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export const TARGET_DEV_HUB_KEY = 'target-dev-hub';
export const TARGET_ORG_KEY = 'target-org';
export const SF_CONFIG_DISABLE_TELEMETRY = 'disable-telemetry';
export const DEFAULT_AIKEY = 'ec3632a4-df47-47a4-98dc-8134cacbaf7e';
export const SFDX_EXTENSION_PACK_NAME = 'salesforcedx-vscode';
59 changes: 45 additions & 14 deletions packages/salesforcedx-utils-vscode/src/telemetry/telemetry.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
/*
* Copyright (c) 2018, salesforce.com, inc.
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import * as util from 'util';
import { env, ExtensionContext, ExtensionMode, workspace } from 'vscode';
import { DEFAULT_AIKEY, SFDX_CORE_CONFIGURATION_NAME } from '../constants';
import { ExtensionContext, ExtensionMode, workspace } from 'vscode';
import {
DEFAULT_AIKEY,
SFDX_CORE_CONFIGURATION_NAME,
SFDX_CORE_EXTENSION_NAME,
SFDX_EXTENSION_PACK_NAME
} from '../constants';
import { disableCLITelemetry, isCLITelemetryAllowed } from './cliConfiguration';
import { TelemetryReporter } from './telemetryReporter';

Expand Down Expand Up @@ -52,25 +57,41 @@ export class TelemetryBuilder {
}
}

// export only for unit test
export class TelemetryServiceProvider {
public static instances = new Map<string, TelemetryService>(); // public only for unit test
public static getInstance(extensionName?: string): TelemetryService {
// default if not present
const name = extensionName || SFDX_CORE_EXTENSION_NAME;
let service = TelemetryServiceProvider.instances.get(name);
if (!service) {
service = new TelemetryService();
TelemetryServiceProvider.instances.set(name, service);
}
return service;

}
}

export class TelemetryService {
private static instance: TelemetryService;
private extensionContext: ExtensionContext | undefined;
private reporter: TelemetryReporter | undefined;
private aiKey = DEFAULT_AIKEY;
private version: string = '';
/**
* Retrieve Telemetry Service according to the extension name.
* If no extension name provided, return the instance for core extension by default
* @param extensionName extension name
*/
public static getInstance(extensionName?: string) {
return TelemetryServiceProvider.getInstance(extensionName);
}
/**
* Cached promise to check if CLI telemetry config is enabled
*/
private cliAllowsTelemetryPromise?: Promise<boolean> = undefined;
public extensionName: string = 'unknown';

public static getInstance() {
if (!TelemetryService.instance) {
TelemetryService.instance = new TelemetryService();
}
return TelemetryService.instance;
}

/**
* Initialize Telemetry Service during extension activation.
* @param extensionContext extension context
Expand Down Expand Up @@ -101,8 +122,6 @@ export class TelemetryService {
console.log('Error initializing telemetry service: ' + error);
});

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const machineId = env ? env.machineId : 'someValue.machineId';
const isDevMode =
extensionContext.extensionMode !== ExtensionMode.Production;

Expand All @@ -113,7 +132,7 @@ export class TelemetryService {
!isDevMode
) {
this.reporter = new TelemetryReporter(
'salesforcedx-vscode',
this.getTelemetryReporterName(),
this.version,
this.aiKey,
true
Expand All @@ -122,6 +141,18 @@ export class TelemetryService {
}
}

/**
* Helper to get the name for telemetryReporter
* if the extension from extension pack, use salesforcedx-vscode
* otherwise use the extension name
* exported only for unit test
*/
public getTelemetryReporterName(): string {
return this.extensionName.startsWith(SFDX_EXTENSION_PACK_NAME)
? SFDX_EXTENSION_PACK_NAME
: this.extensionName;
}

public getReporter(): TelemetryReporter | undefined {
return this.reporter;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { TelemetryService } from '../../../src';
import { SFDX_CORE_EXTENSION_NAME } from '../../../src/constants';
import { TelemetryServiceProvider } from '../../../src/telemetry/telemetry';

describe('Telemetry', () => {
describe('Telemetry Provider', () => {
afterEach(() => {
// Clear instances after each test to avoid state leakage.
TelemetryServiceProvider.instances.clear();
});
it('getInstance should return a TelemetryService instance for core extension when no name is provided', () => {
const instance = TelemetryServiceProvider.getInstance();
expect(instance).toBeInstanceOf(TelemetryService);
expect(
TelemetryServiceProvider.instances.has(SFDX_CORE_EXTENSION_NAME)
).toBeTruthy();
});

it('getInstance should return the same TelemetryService instance for core extension on subsequent calls', () => {
const firstInstance = TelemetryServiceProvider.getInstance();
const secondInstance = TelemetryServiceProvider.getInstance();
expect(secondInstance).toBe(firstInstance);
});

it('getInstance should return a TelemetryService instance for a named extension', () => {
const extensionName = 'someExtension';
const instance = TelemetryServiceProvider.getInstance(extensionName);
expect(instance).toBeInstanceOf(TelemetryService);
expect(
TelemetryServiceProvider.instances.has(extensionName)
).toBeTruthy();
});

it('getInstance should return the same TelemetryService instance for a named extension on subsequent calls', () => {
const extensionName = 'someExtension';
const firstInstance = TelemetryServiceProvider.getInstance(extensionName);
const secondInstance =
TelemetryServiceProvider.getInstance(extensionName);
expect(secondInstance).toBe(firstInstance);
});

it('getInstance should return different instances for different extension names', () => {
const firstExtensionName = 'extensionOne';
const secondExtensionName = 'extensionTwo';
const firstInstance =
TelemetryServiceProvider.getInstance(firstExtensionName);
const secondInstance =
TelemetryServiceProvider.getInstance(secondExtensionName);
expect(firstInstance).not.toBe(secondInstance);
});
});

describe('Telemetry Service', () => {
it('getInstance should return the core instance if no extension name provided', () => {
const firstInstance = TelemetryService.getInstance();
const secondInstance = TelemetryServiceProvider.getInstance(
SFDX_CORE_EXTENSION_NAME
);
expect(firstInstance).toBe(secondInstance);
});
it('getInstance should return the same TelemetryService instance for a named extension on subsequent calls', () => {
const extensionName = 'someExtension';
const firstInstance = TelemetryService.getInstance(extensionName);
const secondInstance =
TelemetryServiceProvider.getInstance(extensionName);
expect(secondInstance).toBe(firstInstance);
});
});

describe('getTelemetryReporterName', () => {
let telemetryService: TelemetryService;
beforeEach(() => {
telemetryService = new TelemetryService();
});

it('should return "salesforcedx-vscode" when extensionName starts with "salesforcedx-vscode"', () => {
telemetryService.extensionName = 'salesforcedx-vscode-core';
const result = telemetryService.getTelemetryReporterName();
expect(result).toBe('salesforcedx-vscode');
});

it('should return the actual extensionName when it does not start with "salesforcedx-vscode"', () => {
telemetryService.extensionName = 'salesforcedx-einstein-gpt';
const result = telemetryService.getTelemetryReporterName();
expect(result).toBe(telemetryService.extensionName);
});
});
});

0 comments on commit 5cde4b6

Please sign in to comment.