From cbd15ece1e6c49dd7df428853979d83b0cf5a325 Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Thu, 9 May 2024 22:11:50 -0700 Subject: [PATCH] Fix Default Logger Settings and Add Console Log Level Support (#1326) * Fix default logger settings and add console log level support. * Clean up. * Update to disable bunyan and winston in non-AKS auto-attach. --- src/agent/agentLoader.ts | 15 +++++++-------- src/agent/aksLoader.ts | 7 +++++++ src/logs/diagnostic-channel/console.sub.ts | 2 +- src/shared/configuration/config.ts | 18 ++++++++++++++++-- src/shared/util/logLevelParser.ts | 17 +++++++++++++++++ src/shim/shim-config.ts | 2 +- test/unitTests/agent/agentLoader.tests.ts | 2 ++ test/unitTests/agent/aksLoader.tests.ts | 4 ++++ test/unitTests/logs/console.tests.ts | 2 +- test/unitTests/shim/config.tests.ts | 4 ++-- 10 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 src/shared/util/logLevelParser.ts diff --git a/src/agent/agentLoader.ts b/src/agent/agentLoader.ts index 9b7ded98..54bf2b87 100644 --- a/src/agent/agentLoader.ts +++ b/src/agent/agentLoader.ts @@ -259,10 +259,10 @@ export class AgentLoader { distroInstance = require.resolve(`${process.cwd()}/node_modules/@azure/monitor-opentelemetry`); } /** - * If loaded instance is in Azure machine home path do not attach the SDK, this means customer already instrumented their app. - * Linux App Service doesn't append the full cwd to the require.resolve, so we need to check for the relative path we expect - * if application insights is being imported in the user app code. - */ + * If loaded instance is in Azure machine home path do not attach the SDK, this means customer already instrumented their app. + * Linux App Service doesn't append the full cwd to the require.resolve, so we need to check for the relative path we expect + * if application insights is being imported in the user app code. + */ if ( shimInstance.indexOf("home") > -1 || distroInstance.indexOf("home") > -1 || (shimInstance === LINUX_USER_APPLICATION_INSIGHTS_PATH && this._isLinux) @@ -274,10 +274,9 @@ export class AgentLoader { this._diagnosticLogger.logMessage(diagnosticLog); return true; } - else { - // ApplicationInsights or Azure Monitor Distro could be loaded outside of customer application, attach in this case - return false; - } + // ApplicationInsights or Azure Monitor Distro could be loaded outside of customer application, attach in this case + return false; + } catch (e) { // crashed while trying to resolve "applicationinsights", so SDK does not exist. Attach appinsights diff --git a/src/agent/aksLoader.ts b/src/agent/aksLoader.ts index 248cab92..c2021ce5 100644 --- a/src/agent/aksLoader.ts +++ b/src/agent/aksLoader.ts @@ -7,6 +7,7 @@ import { DiagnosticLogger } from './diagnostics/diagnosticLogger'; import { FileWriter } from "./diagnostics/writers/fileWriter"; import { StatusLogger } from "./diagnostics/statusLogger"; import { AgentLoader } from "./agentLoader"; +import { InstrumentationOptions } from '../types'; export class AKSLoader extends AgentLoader { @@ -18,6 +19,12 @@ export class AKSLoader extends AgentLoader { // Add OTLP if env variable is present enabled: process.env["OTEL_EXPORTER_OTLP_METRICS_ENDPOINT"] ? true : false }; + (this._options.instrumentationOptions as InstrumentationOptions) = { + ...this._options.instrumentationOptions, + console: { enabled: true }, + bunyan: { enabled: true }, + winston: { enabled: true }, + } let statusLogDir = '/var/log/applicationinsights/'; if (this._isWindows) { diff --git a/src/logs/diagnostic-channel/console.sub.ts b/src/logs/diagnostic-channel/console.sub.ts index 0e23a4a0..2cf6a539 100644 --- a/src/logs/diagnostic-channel/console.sub.ts +++ b/src/logs/diagnostic-channel/console.sub.ts @@ -11,7 +11,7 @@ let logger: Logger; let logSendingLevel: SeverityNumber; const subscriber = (event: IStandardEvent) => { - const severity = (event.data.message as string | Error) instanceof Error ? SeverityNumber.ERROR : (event.data.stderr + const severity = event.data.message.indexOf("Error:") > -1 ? SeverityNumber.ERROR : (event.data.stderr ? SeverityNumber.WARN : SeverityNumber.INFO); if (logSendingLevel <= severity) { diff --git a/src/shared/configuration/config.ts b/src/shared/configuration/config.ts index b911fab1..440b1e58 100644 --- a/src/shared/configuration/config.ts +++ b/src/shared/configuration/config.ts @@ -11,6 +11,9 @@ import { } from "@opentelemetry/resources"; import { JsonConfig } from "./jsonConfig"; import { AzureMonitorOpenTelemetryOptions, OTLPExporterConfig, InstrumentationOptions } from "../../types"; +import { logLevelParser } from "../util/logLevelParser"; + +const loggingLevel = "APPLICATIONINSIGHTS_INSTRUMENTATION_LOGGING_LEVEL"; export class ApplicationInsightsConfig { @@ -75,9 +78,7 @@ export class ApplicationInsightsConfig { this._resource = this._getDefaultResource(); // Merge JSON configuration file if available - this._mergeConfig(); // Check for explicitly passed options when instantiating client - // This will take precedence over other settings if (options) { if (typeof(options.enableAutoCollectExceptions) === "boolean") { this.enableAutoCollectExceptions = options.enableAutoCollectExceptions; @@ -109,6 +110,19 @@ export class ApplicationInsightsConfig { ); this.resource = Object.assign(this.resource, options.resource); this.samplingRatio = options.samplingRatio || this.samplingRatio; + + // Set console logging level from env var + if (process.env[loggingLevel]) { + this.instrumentationOptions = { + ...this.instrumentationOptions, + console: { + ...this.instrumentationOptions.console, + logSendingLevel: logLevelParser(process.env[loggingLevel]), + }, + } + } + + this._mergeConfig(); } } diff --git a/src/shared/util/logLevelParser.ts b/src/shared/util/logLevelParser.ts new file mode 100644 index 00000000..3986578d --- /dev/null +++ b/src/shared/util/logLevelParser.ts @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { SeverityNumber } from "@opentelemetry/api-logs"; + +export function logLevelParser(level: string): number { + switch (level) { + case "ERROR": + return SeverityNumber.ERROR; + case "WARN": + return SeverityNumber.WARN; + case "INFO": + return SeverityNumber.INFO; + default: + return SeverityNumber.UNSPECIFIED; + } +} \ No newline at end of file diff --git a/src/shim/shim-config.ts b/src/shim/shim-config.ts index cc971ed2..1591fc70 100644 --- a/src/shim/shim-config.ts +++ b/src/shim/shim-config.ts @@ -148,7 +148,7 @@ class Config implements IConfig { }; (options.instrumentationOptions as InstrumentationOptions) = { ...options.instrumentationOptions, - console: { enabled: true }, + console: { enabled: false }, winston: { enabled: true }, }; if (this.samplingPercentage) { diff --git a/test/unitTests/agent/agentLoader.tests.ts b/test/unitTests/agent/agentLoader.tests.ts index 08c83efc..0fdbd5eb 100644 --- a/test/unitTests/agent/agentLoader.tests.ts +++ b/test/unitTests/agent/agentLoader.tests.ts @@ -7,6 +7,7 @@ import * as sinon from "sinon"; import { AgentLoader } from "../../../src/agent/agentLoader"; import * as azureMonitor from "@azure/monitor-opentelemetry"; import { DiagnosticMessageId } from "../../../src/agent/types"; +import { dispose } from "../../../src/logs/diagnostic-channel/winston.sub"; describe("agent/agentLoader", () => { let originalEnv: NodeJS.ProcessEnv; @@ -53,6 +54,7 @@ describe("agent/agentLoader", () => { }); afterEach(() => { + dispose(); process.env = originalEnv; sandbox.restore(); }); diff --git a/test/unitTests/agent/aksLoader.tests.ts b/test/unitTests/agent/aksLoader.tests.ts index 266943cb..46e9d0d0 100644 --- a/test/unitTests/agent/aksLoader.tests.ts +++ b/test/unitTests/agent/aksLoader.tests.ts @@ -6,6 +6,8 @@ import { logs } from "@opentelemetry/api-logs"; import { AKSLoader } from "../../../src/agent/aksLoader"; import { DiagnosticLogger } from "../../../src/agent/diagnostics/diagnosticLogger"; import { FileWriter } from "../../../src/agent/diagnostics/writers/fileWriter"; +import { dispose as disposeConsole } from "../../../src/logs/diagnostic-channel/console.sub"; +import { dispose as disposeWinston } from "../../../src/logs/diagnostic-channel/winston.sub"; describe("agent/AKSLoader", () => { let originalEnv: NodeJS.ProcessEnv; @@ -20,6 +22,8 @@ describe("agent/AKSLoader", () => { }); afterEach(() => { + disposeConsole(); + disposeWinston(); process.env = originalEnv; sandbox.restore(); }); diff --git a/test/unitTests/logs/console.tests.ts b/test/unitTests/logs/console.tests.ts index 8801524d..e22f9bb7 100644 --- a/test/unitTests/logs/console.tests.ts +++ b/test/unitTests/logs/console.tests.ts @@ -41,7 +41,7 @@ describe("AutoCollection/Console", () => { }); const dummyError = new Error("test error"); const errorEvent: console.IConsoleData = { - message: dummyError as any, + message: dummyError.toString(), stderr: false, }; channel.publish("console", errorEvent); diff --git a/test/unitTests/shim/config.tests.ts b/test/unitTests/shim/config.tests.ts index 53d4cd7f..cfcc860b 100644 --- a/test/unitTests/shim/config.tests.ts +++ b/test/unitTests/shim/config.tests.ts @@ -159,7 +159,7 @@ describe("shim/configuration/config", () => { redis4: { enabled: false }, postgreSql: { enabled: false }, bunyan: { enabled: true }, - console: { enabled: true }, + console: { enabled: false }, winston: { enabled: true }, })); }); @@ -189,7 +189,7 @@ describe("shim/configuration/config", () => { "redis4": { "enabled": true }, "postgreSql": { "enabled": true }, "bunyan": { "enabled": false }, - "console": { "enabled": true }, + "console": { "enabled": false }, "winston": { "enabled": false } })); });