Skip to content

Commit

Permalink
[Beta] Update Warning Tests and Add Console Logging by Default (#1292)
Browse files Browse the repository at this point in the history 
* Update testing and default enable logger.

* Update telemetryClient.ts

* Update to use the warnings array.

* Refactor how we handle logging errors in start/setup scenarios.

* Fix warnings.

* Update warning logger approach and setting for auto collect performance.

* Ignore warning.
  • Loading branch information
@JacksonWeber
JacksonWeber authored Mar 8, 2024
1 parent b99d62d commit a90e354
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 110 deletions.
20 changes: 15 additions & 5 deletions src/shim/applicationinsights.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand All @@ -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)}`);
}
}

Expand Down Expand Up @@ -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;
Expand Down
38 changes: 21 additions & 17 deletions src/shim/shim-config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}
Expand Down
23 changes: 18 additions & 5 deletions src/shim/telemetryClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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}`);
}
}
Expand Down Expand Up @@ -315,4 +324,8 @@ export class TelemetryClient {
public async shutdown(): Promise<void> {
return shutdownAzureMonitor();
}

public pushWarningToLog(warning: string) {
this._configWarnings.push(warning);
}
}
73 changes: 29 additions & 44 deletions test/unitTests/shim/applicationInsights.tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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");
});
});

Expand Down Expand Up @@ -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", () => {
Expand All @@ -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");
});
});
});
Expand Down
Loading

0 comments on commit a90e354

Please sign in to comment.