Repository: microsoft/ApplicationInsights-node.js
Pull request #1292 (2024-02-29 06:39:10)

[Beta] Update Warning Tests and Add Console Logging by Default
* Fixes our console warning tests to be updated using the `diag` logger from OTel.
* Automatically sets the default logger in the shim to the console.


Patch:
diff --git a/src/shim/applicationinsights.ts b/src/shim/applicationinsights.ts
index 055424f..ce4b14a 100644
--- a/src/shim/applicationinsights.ts
+++ b/src/shim/applicationinsights.ts
@@ -3,13 +3,13 @@
 
 import * as http from "http";
 import * as azureFunctionsTypes from "@azure/functions";
-import { SpanContext, diag } from "@opentelemetry/api";
+import { DiagConsoleLogger, SpanContext, diag } from "@opentelemetry/api";
 import { Span } from "@opentelemetry/sdk-trace-base";
 import { CorrelationContextManager } from "./correlationContextManager";
 import { ICorrelationContext, HttpRequest, DistributedTracingModes } from "./types";
 import { TelemetryClient } from "./telemetryClient";
 import * as Contracts from "../declarations/contracts";
-
+import { Util } from "../shared/util";
 
 // We export these imports so that SDK users may use these classes directly.
 // They're exposed using "export import" so that types are passed along as expected
@@ -32,7 +32,11 @@ export let defaultClient: TelemetryClient;
  * and start the SDK.
  */
 export function setup(setupString?: string) {
-    defaultClient = new TelemetryClient(setupString);
+    if (!defaultClient) {
+        defaultClient = new TelemetryClient(setupString);
+    } else {
+        defaultClient.pushWarningToLog("Setup has already been called once. To set up a new client, please use TelemetryClient instead.")
+    }
     return Configuration;
 }
 
@@ -44,10 +48,15 @@ export function setup(setupString?: string) {
  */
 export function start() {
     try {
-        defaultClient.initialize();
+        if (!defaultClient) {
+            diag.setLogger(new DiagConsoleLogger());
+            diag.warn("Start cannot be called before setup. Please call setup() first.");
+        } else {
+            defaultClient.initialize();
+        }
         return Configuration;
     } catch (error) {
-        diag.warn("The default client has not been initialized. Please make sure to call setup() before start().");
+        diag.warn(`Failed to start default client: ${Util.getInstance().dumpObj(error)}`);
     }
 }
 
@@ -139,6 +148,7 @@ export class Configuration {
      * @param collectExtendedMetrics if true, extended metrics counters will be collected every minute and sent to Application Insights
      * @returns {Configuration} this class
      */
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars 
     public static setAutoCollectPerformance(value: boolean, collectExtendedMetrics: any) {
         if (defaultClient) {
             defaultClient.config.enableAutoCollectPerformance = value;
diff --git a/src/shim/shim-config.ts b/src/shim/shim-config.ts
index 11a144d..f4de9dd 100644
--- a/src/shim/shim-config.ts
+++ b/src/shim/shim-config.ts
@@ -61,18 +61,20 @@ class Config implements IConfig {
     public noPatchModules: string;
     public noDiagnosticChannel: boolean;
 
+    private _configWarnings: string[];
 
     /**
    * Creates a new Config instance
    * @param setupString Connection String, instrumentationKey is no longer supported here
    */
-    constructor(setupString?: string) {
+    constructor(setupString?: string, configWarnings?: string[]){
         // Load config values from env variables and JSON if available
         this._mergeConfig();
         this.connectionString = setupString;
         this.webInstrumentationConfig = this.webInstrumentationConfig || null;
         this.ignoreLegacyHeaders = true;
         this.webInstrumentationConnectionString = this.webInstrumentationConnectionString || "";
+        this._configWarnings = configWarnings || [];
     }
 
     private _mergeConfig() {
@@ -335,49 +337,51 @@ class Config implements IConfig {
 
         // NOT SUPPORTED CONFIGURATION OPTIONS
         if (this.disableAppInsights) {
-            diag.warn("disableAppInsights configuration no longer supported.");
+            this._configWarnings.push("disableAppInsights configuration no longer supported.");
         }
-        if (this.enableAutoCollectHeartbeat) {
-            diag.warn("Heartbeat metris are no longer supported.");
+        if (this.enableAutoCollectHeartbeat === true) {
+            this._configWarnings.push("Heartbeat metrics are no longer supported.");
         }
         if (this.enableAutoDependencyCorrelation === false) {
-            diag.warn("Auto dependency correlation cannot be turned off anymore.");
+            this._configWarnings.push("Auto dependency correlation cannot be turned off anymore.");
         }
         if (typeof (this.enableAutoCollectIncomingRequestAzureFunctions) === "boolean") {
-            diag.warn("Auto request generation in Azure Functions is no longer supported.");
+            this._configWarnings.push("Auto request generation in Azure Functions is no longer supported.");
         }
         if (this.enableUseAsyncHooks === false) {
-            diag.warn("The use of non async hooks is no longer supported.");
+            this._configWarnings.push("The use of non async hooks is no longer supported.");
         }
         if (this.distributedTracingMode === DistributedTracingModes.AI) {
-            diag.warn("AI only distributed tracing mode is no longer supported.");
+            this._configWarnings.push("AI only distributed tracing mode is no longer supported.");
         }
         if (this.enableResendInterval) {
-            diag.warn("The resendInterval configuration option is not supported by the shim.");
+            this._configWarnings.push("The resendInterval configuration option is not supported by the shim.");
         }
         if (this.enableMaxBytesOnDisk) {
-            diag.warn("The maxBytesOnDisk configuration option is not supported by the shim.");
+            this._configWarnings.push("The maxBytesOnDisk configuration option is not supported by the shim.");
         }
         if (this.ignoreLegacyHeaders === false) {
-            diag.warn("LegacyHeaders are not supported by the shim.");
+            this._configWarnings.push("LegacyHeaders are not supported by the shim.");
         }
-
         if (this.maxBatchSize) {
-            diag.warn("The maxBatchSize configuration option is not supported by the shim.");
+            this._configWarnings.push("The maxBatchSize configuration option is not supported by the shim.");
         }
         if (this.enableLoggerErrorToTrace) {
-            diag.warn("The enableLoggerErrorToTrace configuration option is not supported by the shim.");
+            this._configWarnings.push("The enableLoggerErrorToTrace configuration option is not supported by the shim.");
         }
         if (this.httpAgent || this.httpsAgent) {
-            diag.warn("The httpAgent and httpsAgent configuration options are not supported by the shim.");
+            this._configWarnings.push("The httpAgent and httpsAgent configuration options are not supported by the shim.");
         }
         if (
             this.webInstrumentationConfig || this.webInstrumentationSrc
         ) {
-            diag.warn("The webInstrumentation config and src options are not supported by the shim.");
+            this._configWarnings.push("The webInstrumentation config and src options are not supported by the shim.");
         }
         if (this.quickPulseHost) {
-            diag.warn("The quickPulseHost configuration option is not suppored by the shim.");
+            this._configWarnings.push("The quickPulseHost configuration option is not supported by the shim.");
+        }
+        if (this.correlationHeaderExcludedDomains) {
+            this._configWarnings.push("The correlationHeaderExcludedDomains configuration option is not supported by the shim.");
         }
         return options;
     }
diff --git a/src/shim/telemetryClient.ts b/src/shim/telemetryClient.ts
index ef0c174..a33a465 100644
--- a/src/shim/telemetryClient.ts
+++ b/src/shim/telemetryClient.ts
@@ -22,21 +22,22 @@ import { AzureMonitorOpenTelemetryOptions } from "../types";
  * and manually trigger immediate sending (flushing)
  */
 export class TelemetryClient {
+    public context: Context;
+    public commonProperties: { [key: string]: string };
+    public config: Config;
     private _attributeSpanProcessor: AttributeSpanProcessor;
     private _attributeLogProcessor: AttributeLogProcessor;
     private _logApi: LogApi;
     private _isInitialized: boolean;
-    public context: Context;
-    public commonProperties: { [key: string]: string };
-    public config: Config;
     private _options: AzureMonitorOpenTelemetryOptions;
+    private _configWarnings: string[] = [];
 
     /**
      * Constructs a new instance of TelemetryClient
      * @param setupString the Connection String or Instrumentation Key to use (read from environment variable if not specified)
      */
     constructor(input?: string) {
-        const config = new Config(input);
+        const config = new Config(input, this._configWarnings);
         this.config = config;
         this.commonProperties = {};
         this.context = new Context();
@@ -57,7 +58,15 @@ export class TelemetryClient {
 
             this._attributeLogProcessor = new AttributeLogProcessor({ ...this.context.tags, ...this.commonProperties });
             (logs.getLoggerProvider() as LoggerProvider).addLogRecordProcessor(this._attributeLogProcessor);
-        } catch (error) {
+
+            // Warn if any config warnings were generated during parsing
+            for (let i = 0; i < this._configWarnings.length; i++) {
+                diag.warn(this._configWarnings[i]);
+                this._attributeLogProcessor = new AttributeLogProcessor({ ...this.context.tags, ...this.commonProperties });
+                (logs.getLoggerProvider() as LoggerProvider).addLogRecordProcessor(this._attributeLogProcessor);
+            }
+        } 
+        catch (error) {
             diag.error(`Failed to initialize TelemetryClient ${error}`);
         }
     }
@@ -315,4 +324,8 @@ export class TelemetryClient {
     public async shutdown(): Promise<void> {
         return shutdownAzureMonitor();
     }
+
+    public pushWarningToLog(warning: string) {
+        this._configWarnings.push(warning);
+    }
 }


Test patch:
diff --git a/test/unitTests/shim/applicationInsights.tests.ts b/test/unitTests/shim/applicationInsights.tests.ts
index 12cc389..0305249 100644
--- a/test/unitTests/shim/applicationInsights.tests.ts
+++ b/test/unitTests/shim/applicationInsights.tests.ts
@@ -4,8 +4,9 @@ import * as assert from "assert";
 import * as sinon from "sinon";
 
 import * as appInsights from "../../../src/index";
-import { diag, DiagLogLevel } from "@opentelemetry/api";
+import { diag } from "@opentelemetry/api";
 import { InstrumentationOptions } from "../../../src/types";
+import { checkWarnings } from "./testUtils";
 
 describe("ApplicationInsights", () => {
     let sandbox: sinon.SinonSandbox;
@@ -21,21 +22,25 @@ describe("ApplicationInsights", () => {
 
     describe("#setup()", () => {
         it("should not warn if setup is called once", (done) => {
-            const warnStub = sandbox.stub(console, "warn");
             appInsights.setup(connString);
-            assert.ok(warnStub.notCalled, "warning was raised");
+            const warnings = appInsights.defaultClient["_configWarnings"];
+            assert.ok(!checkWarnings("Setup has already been called once", warnings), "setup warning was incorrectly raised");
             done();
         });
         it("should warn if setup is called twice", (done) => {
-            const warnStub = sandbox.stub(console, "warn");
+            const warnStub = sandbox.stub(diag, "warn");
             appInsights.setup(connString);
             appInsights.setup(connString);
-            assert.ok(warnStub.calledOn, "warning was not raised");
+            warnStub.args.forEach((arg) => {
+                if (arg.includes("Setup has already been called once")) {
+                    assert.ok(true, "setup warning was not raised");
+                }
+            });
             done();
         });
         it("should not overwrite default client if called more than once", (done) => {
             appInsights.setup(connString);
-            var client = appInsights.defaultClient;
+            const client = appInsights.defaultClient;
             appInsights.setup(connString);
             appInsights.setup(connString);
             appInsights.setup(connString);
@@ -46,16 +51,20 @@ describe("ApplicationInsights", () => {
 
     describe("#start()", () => {
         it("should warn if start is called before setup", (done) => {
-            const warnStub = sandbox.stub(console, "warn");
+            const warnStub = sandbox.stub(diag, "warn");
             appInsights.start();
-            assert.ok(warnStub.calledOn, "warning was not raised");
+            warnStub.args.forEach((arg) => {
+                if (arg.includes("Start cannot be called before setup. Please call setup() first.")) {
+                    assert.ok(true, "setup warning was not raised");
+                }
+            });
             done();
         });
 
         it("should not warn if start is called after setup", () => {
-            var warnStub = sandbox.stub(console, "warn");
             appInsights.setup(connString).start();
-            assert.ok(warnStub.notCalled, "warning was raised");
+            const warnings = appInsights.defaultClient["_configWarnings"];
+            assert.ok(!checkWarnings("Start cannot be called before setup. Please call setup() first.", warnings), "warning was thrown when start was called correctly");
         });
     });
 
@@ -93,44 +102,28 @@ describe("ApplicationInsights", () => {
         });
 
         describe("#Configuration", () => {
-            it("should throw warning if attempting to set AI distributed tracing mode", () => {
-                const warnStub = sandbox.stub(console, "warn");
+            it("should throw warning if attempting to set AI distributed tracing mode to AI", () => {
                 appInsights.setup(connString);
-                appInsights.Configuration.setDistributedTracingMode(appInsights.DistributedTracingModes.AI_AND_W3C);
+                const warnings = appInsights.defaultClient["_configWarnings"];
+                appInsights.Configuration.setDistributedTracingMode(appInsights.DistributedTracingModes.AI);
                 appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
-            });
-
-            it("should warn if attempting to set auto collection of pre-aggregated metrics", () => {
-                const warnStub = sandbox.stub(console, "warn");
-                appInsights.setup(connString);
-                appInsights.Configuration.setAutoCollectPreAggregatedMetrics(true);
-                appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("AI only distributed tracing mode is no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if attempting to set auto collection of heartbeat", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 appInsights.setup(connString);
+                const warnings = appInsights.defaultClient["_configWarnings"];
                 appInsights.Configuration.setAutoCollectHeartbeat(true);
                 appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
-            });
-
-            it("should warn if attempting to enable web instrumentation", () => {
-                const warnStub = sandbox.stub(console, "warn");
-                appInsights.setup(connString);
-                appInsights.Configuration.enableWebInstrumentation(true);
-                appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("Heartbeat metrics are no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if attempting to set maxBytesOnDisk", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 appInsights.setup(connString);
+                const warnings = appInsights.defaultClient["_configWarnings"];
                 appInsights.Configuration.setUseDiskRetryCaching(true, 1000, 10);
                 appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The maxBytesOnDisk configuration option is not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should set internal loggers", () => {
@@ -141,19 +134,11 @@ describe("ApplicationInsights", () => {
             });
 
             it("should warn if attempting to auto collect incoming azure functions requests", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 appInsights.setup(connString);
+                const warnings = appInsights.defaultClient["_configWarnings"];
                 appInsights.Configuration.setAutoCollectIncomingRequestAzureFunctions(true);
                 appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
-            });
-
-            it("should warn if attempting to send live metrics", () => {
-                const warnStub = sandbox.stub(console, "warn");
-                appInsights.setup(connString);
-                appInsights.Configuration.setSendLiveMetrics(true);
-                appInsights.start();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("Auto request generation in Azure Functions is no longer supported.", warnings), "warning was not raised");
             });
         });
     });
diff --git a/test/unitTests/shim/config.tests.ts b/test/unitTests/shim/config.tests.ts
index 31cec76..f26a487 100644
--- a/test/unitTests/shim/config.tests.ts
+++ b/test/unitTests/shim/config.tests.ts
@@ -9,7 +9,7 @@ import { TelemetryClient } from "../../../src/shim/telemetryClient";
 import http = require("http");
 import https = require("https");
 import { DistributedTracingModes } from '../../../applicationinsights';
-
+import { checkWarnings } from './testUtils';
 
 class TestTokenCredential implements azureCoreAuth.TokenCredential {
     private _expiresOn: Date;
@@ -224,127 +224,117 @@ describe("shim/configuration/config", () => {
 
         describe("#Shim unsupported messages", () => {
             it("should warn if disableAppInsights is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.disableAppInsights = true;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("disableAppInsights configuration no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if collect heartbeat is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableAutoCollectHeartbeat = true;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("Heartbeat metrics are no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if auto dependency correlation is set to false", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableAutoDependencyCorrelation = false;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("Auto dependency correlation cannot be turned off anymore.", warnings), "warning was not raised");
             });
 
             it("should warn if auto request generation is azure functions is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableAutoCollectIncomingRequestAzureFunctions = true;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
-            });
-
-            it("should warn if live metrics is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
-                const config = new Config(connectionString);
-                config.enableSendLiveMetrics = true;
-                config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("Auto request generation in Azure Functions is no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if using async hooks is set to false", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableUseAsyncHooks = false;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The use of non async hooks is no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if distributed tracing mode is set to AI", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.distributedTracingMode = DistributedTracingModes.AI;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("AI only distributed tracing mode is no longer supported.", warnings), "warning was not raised");
             });
 
             it("should warn if resend interval is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableResendInterval = 1;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The resendInterval configuration option is not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if max bytes on disk is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableMaxBytesOnDisk = 1;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The maxBytesOnDisk configuration option is not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if ignore legacy headers is false", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.ignoreLegacyHeaders = false;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("LegacyHeaders are not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if max batch size is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.maxBatchSize = 1;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The maxBatchSize configuration option is not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if logger errors are set to traces", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.enableLoggerErrorToTrace = true;
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The enableLoggerErrorToTrace configuration option is not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if httpAgent or httpsAgent are set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.httpAgent = new http.Agent();
                 config.httpsAgent = new https.Agent();
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The httpAgent and httpsAgent configuration options are not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if web instrumentations are set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
-                config.enableWebInstrumentation = true;
+                const warnings = config["_configWarnings"];
                 config.webInstrumentationConfig = [{name: "test", value: true}];
                 config.webInstrumentationSrc = "test";
-                config.webInstrumentationConnectionString = "test";
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The webInstrumentation config and src options are not supported by the shim.", warnings), "warning was not raised");
             });
 
             it("should warn if correlationHeaderExcludedDomains is set", () => {
-                const warnStub = sandbox.stub(console, "warn");
                 const config = new Config(connectionString);
+                const warnings = config["_configWarnings"];
                 config.correlationHeaderExcludedDomains = ["test.com"];
                 config.parseConfig();
-                assert.ok(warnStub.calledOn, "warning was not raised");
+                assert.ok(checkWarnings("The correlationHeaderExcludedDomains configuration option is not supported by the shim.", warnings), "warning was not raised");
             });
         });
     });
diff --git a/test/unitTests/shim/testUtils.ts b/test/unitTests/shim/testUtils.ts
new file mode 100644
index 0000000..f2f6009
--- /dev/null
+++ b/test/unitTests/shim/testUtils.ts
@@ -0,0 +1,9 @@
+export function checkWarnings(warning: string, warnings: string[]) {
+    let expectedWarning: any;
+    for (let i = 0; i < warnings.length; i++) {
+        if (warnings[i].toString().includes(warning)) {
+            expectedWarning = warnings[i];
+        }
+    }
+    return expectedWarning;
+}