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.

Sign up for GitHub

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

Jump to bottom

[Beta] Update Warning Tests and Add Console Logging by Default #1292

Merged
merged 8 commits into from
Mar 8, 2024
Merged

[Beta] Update Warning Tests and Add Console Logging by Default #1292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
029b850
Update testing and default enable logger.
JacksonWeber Feb 29, 2024
b0ba05d
Update telemetryClient.ts
JacksonWeber Feb 29, 2024
33ddaf6
Update to use the warnings array.
JacksonWeber Mar 1, 2024
3d42858
Merge branch 'beta' into jacksonweber/update-logging-tests
JacksonWeber Mar 5, 2024
5294c24
Refactor how we handle logging errors in start/setup scenarios.
JacksonWeber Mar 5, 2024
5fbba54
Fix warnings.
JacksonWeber Mar 6, 2024
1751788
Update warning logger approach and setting for auto collect performance.
JacksonWeber Mar 8, 2024
6077a9a
Ignore warning.
JacksonWeber Mar 8, 2024
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
20 changes: 15 additions & 5 deletions src/shim/applicationinsights.ts
View file
Open in desktop
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) {
JacksonWeber marked this conversation as resolved.
Show resolved Hide resolved
defaultClient.config.enableAutoCollectPerformance = value;
Expand Down
38 changes: 21 additions & 17 deletions src/shim/shim-config.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading