Merge pull request #274 from forcedotcom/sm/standlone-enabled-check

feat: standalone check for telemetry enabled

Author: mdonnalley

.mocharc.json

@@ -1,8 +1,8 @@
 {
-    "require": "ts-node/register,source-map-support/register",
-    "watch-extensions": "ts",
-    "recursive": true,
-    "reporter": "spec",
-    "timeout": 5000
-  }
-  
+  "require": "ts-node/register,source-map-support/register",
+  "watch-extensions": "ts",
+  "watch-files": ["src/**/*.ts", "test/**/*.ts"],
+  "recursive": true,
+  "reporter": "spec",
+  "timeout": 5000
+}

package.json

@@ -1,8 +1,12 @@
 {
   "name": "@salesforce/telemetry",
-  "version": "4.0.16",
+  "version": "4.0.17-qa.0",
   "description": "Library for application insights",
   "main": "lib/exported",
+  "exports": {
+    "./enabledCheck": "./lib/enabledCheck.js",
+    ".": "./lib/exported.js"
+  },
   "repository": "forcedotcom/telemetry",
   "author": "Salesforce",
   "license": "BSD-3-Clause",

src/enabledCheck.ts

@@ -0,0 +1,28 @@
+/*
+ * 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
+ */
+
+// deep imports to avoid requiring the ENTIRE package (which will also pull in jsforce) until we get ESM done
+import { ConfigAggregator } from '@salesforce/core/lib/config/configAggregator';
+import { SfConfigProperties } from '@salesforce/core/lib/config/config';
+
+// store the result to reduce checks
+let enabled: boolean | undefined;
+
+/**
+ *
+ * Check ConfigAggregator once for telemetry opt-out.  Returns true unless config/env has opt-out
+ * If you don't pass in a ConfigAggregator, one will be constructed for you
+ * memoized: only runs once
+ *
+ * */
+export const isEnabled = async (configAggregator?: ConfigAggregator): Promise<boolean> => {
+  if (enabled === undefined) {
+    const agg = configAggregator ?? (await ConfigAggregator.create({}));
+    enabled = agg.getPropertyValue<string>(SfConfigProperties.DISABLE_TELEMETRY) !== 'true';
+  }
+  return enabled;
+};

src/exported.ts

@@ -9,4 +9,5 @@
 import { TelemetryReporter } from './telemetryReporter';
 
 export * from './telemetryReporter';
+export { isEnabled } from './enabledCheck';
 export default TelemetryReporter;

src/telemetryReporter.ts

@@ -5,26 +5,24 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 import * as os from 'os';
-import { Logger, ConfigAggregator, SfConfigProperties } from '@salesforce/core';
+import { Logger, SfConfigProperties } from '@salesforce/core';
 import { AsyncCreatable, env } from '@salesforce/kit';
 
 import got from 'got';
 import { ProxyAgent } from 'proxy-agent';
-import { AppInsights, Attributes, Properties, TelemetryOptions } from './appInsights';
+import { AppInsights, type Attributes, type Properties, type TelemetryOptions } from './appInsights';
 import { TelemetryClient } from './exported';
+import { isEnabled } from './enabledCheck';
 
 export { TelemetryOptions, Attributes, Properties, TelemetryClient } from './appInsights';
 
 /**
  * Reports telemetry events to app insights. We do not send if the config 'disableTelemetry' is set.
  */
 export class TelemetryReporter extends AsyncCreatable<TelemetryOptions> {
-  // Keep a cache of config aggregator so we aren't loading it every time.
-  private static config: ConfigAggregator;
-
+  private enabled = false;
   private options: TelemetryOptions;
   private logger!: Logger;
-  private config!: ConfigAggregator;
   private reporter!: AppInsights;
 
   public constructor(options: TelemetryOptions) {
@@ -33,25 +31,17 @@ export class TelemetryReporter extends AsyncCreatable<TelemetryOptions> {
   }
 
   /**
+   * @deprecated Use the standalone function isEnabled() instead.
    * Determine if the telemetry event should be logged.
    * Setting the disableTelemetry config var to true will disable insights for errors and diagnostics.
    */
   public static async determineSfdxTelemetryEnabled(): Promise<boolean> {
-    if (!TelemetryReporter.config) {
-      TelemetryReporter.config = await ConfigAggregator.create({});
-    }
-    const configValue = TelemetryReporter.config.getPropertyValue(SfConfigProperties.DISABLE_TELEMETRY);
-    // SF_DISABLE_TELEMETRY is the proper name for this env that will be cheked by config.getPropertyValue. SFDX_DISABLE_INSIGHTS is present for backwards compatibility
-    const sfdxDisableInsights = configValue === 'true' || env.getBoolean('SFDX_DISABLE_INSIGHTS');
-    return !sfdxDisableInsights;
+    return isEnabled();
   }
 
   public async init(): Promise<void> {
+    this.enabled = await isEnabled();
     this.logger = await Logger.child('TelemetryReporter');
-    if (!TelemetryReporter.config) {
-      TelemetryReporter.config = await ConfigAggregator.create({});
-    }
-    this.config = TelemetryReporter.config;
     if (this.options.waitForConnection) await this.waitForConnection();
     this.reporter = await AppInsights.create(this.options);
   }
@@ -165,15 +155,11 @@ export class TelemetryReporter extends AsyncCreatable<TelemetryOptions> {
    * Setting the disableTelemetry config var to true will disable insights for errors and diagnostics.
    */
   public isSfdxTelemetryEnabled(): boolean {
-    const configValue = this.config.getPropertyValue(SfConfigProperties.DISABLE_TELEMETRY);
-    const sfdxDisableInsights = configValue === 'true' || env.getBoolean('SFDX_DISABLE_INSIGHTS');
-    // isEnabled = !sfdxDisableInsights
-    return !sfdxDisableInsights;
+    return this.enabled;
   }
 
   public logTelemetryStatus(): void {
-    const isEnabled = this.isSfdxTelemetryEnabled();
-    if (isEnabled) {
+    if (this.enabled) {
       this.logger.warn(
         `Telemetry is enabled. This can be disabled by running sfdx force:config:set ${SfConfigProperties.DISABLE_TELEMETRY}=true`
       );

test/unit/telemetryReporter.test.ts

@@ -11,6 +11,7 @@ import { expect } from 'chai';
 import * as sinon from 'sinon';
 import { AppInsights } from '../../src/appInsights';
 import { TelemetryReporter } from '../../src/telemetryReporter';
+import * as enabledStubs from '../../src/enabledCheck';
 
 describe('TelemetryReporter', () => {
   const key = 'foo-bar-123';
@@ -75,7 +76,7 @@ describe('TelemetryReporter', () => {
   });
 
   it('should not send a telemetry event when disabled', async () => {
-    sandbox.stub(ConfigAggregator.prototype, 'getPropertyValue').returns('true');
+    sandbox.stub(enabledStubs, 'isEnabled').resolves(false);
     const options = { project, key };
     const reporter = await TelemetryReporter.create(options);
     const sendStub = sandbox.stub(reporter.getTelemetryClient(), 'trackEvent').callsFake(() => {});
@@ -85,7 +86,7 @@ describe('TelemetryReporter', () => {
   });
 
   it('should not send a telemetry exception when disabled', async () => {
-    sandbox.stub(ConfigAggregator.prototype, 'getPropertyValue').returns('true');
+    sandbox.stub(enabledStubs, 'isEnabled').resolves(false);
     const options = { project, key };
     const reporter = await TelemetryReporter.create(options);
     const sendStub = sandbox.stub(reporter.getTelemetryClient(), 'trackException').callsFake(() => {});
@@ -95,7 +96,7 @@ describe('TelemetryReporter', () => {
   });
 
   it('should not send a telemetry trace when disabled', async () => {
-    sandbox.stub(ConfigAggregator.prototype, 'getPropertyValue').returns('true');
+    sandbox.stub(enabledStubs, 'isEnabled').resolves(false);
     const options = { project, key };
     const reporter = await TelemetryReporter.create(options);
     const sendStub = sandbox.stub(reporter.getTelemetryClient(), 'trackTrace').callsFake(() => {});
@@ -105,7 +106,7 @@ describe('TelemetryReporter', () => {
   });
 
   it('should not send a telemetry metric when disabled', async () => {
-    sandbox.stub(ConfigAggregator.prototype, 'getPropertyValue').returns('true');
+    sandbox.stub(enabledStubs, 'isEnabled').resolves(false);
     const options = { project, key };
     const reporter = await TelemetryReporter.create(options);
     const sendStub = sandbox.stub(reporter.getTelemetryClient(), 'trackMetric').callsFake(() => {});
@@ -115,7 +116,7 @@ describe('TelemetryReporter', () => {
   });
 
   it('should log to enable telemetry metric when disabled', async () => {
-    sandbox.stub(ConfigAggregator.prototype, 'getPropertyValue').returns('true');
+    sandbox.stub(enabledStubs, 'isEnabled').resolves(false);
     const warn = sandbox.stub();
     // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
     sandbox.stub(Logger, 'child').resolves({ warn, debug: sandbox.stub() } as any);