From 56a7636e9eb433f3f2c9a770fd0fe19222eb9871 Mon Sep 17 00:00:00 2001 From: Dan Herd Date: Wed, 2 Apr 2025 16:57:53 +0100 Subject: [PATCH 1/3] refactor(auto-instrumentations-node): use env var utility functions to setup diagnostics logging --- .../src/register.ts | 12 ++++- .../auto-instrumentations-node/src/utils.ts | 26 +---------- .../test/utils.test.ts | 44 +------------------ 3 files changed, 13 insertions(+), 69 deletions(-) diff --git a/metapackages/auto-instrumentations-node/src/register.ts b/metapackages/auto-instrumentations-node/src/register.ts index 3a4ae6cd5a..b0fef3efbb 100644 --- a/metapackages/auto-instrumentations-node/src/register.ts +++ b/metapackages/auto-instrumentations-node/src/register.ts @@ -16,12 +16,20 @@ import * as opentelemetry from '@opentelemetry/sdk-node'; import { diag, DiagConsoleLogger } from '@opentelemetry/api'; import { - getLogLevelFromEnv, + getStringFromEnv, + diagLogLevelFromString, +} from '@opentelemetry/core'; +import { getNodeAutoInstrumentations, getResourceDetectorsFromEnv, } from './utils'; -diag.setLogger(new DiagConsoleLogger(), getLogLevelFromEnv()); +const logLevel = getStringFromEnv('OTEL_LOG_LEVEL'); +if (logLevel != null) { + diag.setLogger(new DiagConsoleLogger(), { + logLevel: diagLogLevelFromString(logLevel), + }); +} const sdk = new opentelemetry.NodeSDK({ instrumentations: getNodeAutoInstrumentations(), diff --git a/metapackages/auto-instrumentations-node/src/utils.ts b/metapackages/auto-instrumentations-node/src/utils.ts index ae5349a0fc..e43ee7c32a 100644 --- a/metapackages/auto-instrumentations-node/src/utils.ts +++ b/metapackages/auto-instrumentations-node/src/utils.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { diag, DiagLogLevel } from '@opentelemetry/api'; +import { diag } from '@opentelemetry/api'; import { Instrumentation } from '@opentelemetry/instrumentation'; import { AmqplibInstrumentation } from '@opentelemetry/instrumentation-amqplib'; @@ -135,17 +135,6 @@ const InstrumentationMap = { '@opentelemetry/instrumentation-winston': WinstonInstrumentation, }; -// The support string -> DiagLogLevel mappings -const logLevelMap: { [key: string]: DiagLogLevel } = { - ALL: DiagLogLevel.ALL, - VERBOSE: DiagLogLevel.VERBOSE, - DEBUG: DiagLogLevel.DEBUG, - INFO: DiagLogLevel.INFO, - WARN: DiagLogLevel.WARN, - ERROR: DiagLogLevel.ERROR, - NONE: DiagLogLevel.NONE, -}; - const defaultExcludedInstrumentations = [ '@opentelemetry/instrumentation-fs', '@opentelemetry/instrumentation-fastify', @@ -303,16 +292,3 @@ export function getResourceDetectorsFromEnv(): Array { return resourceDetector || []; }); } - -export function getLogLevelFromEnv(): DiagLogLevel { - const rawLogLevel = process.env.OTEL_LOG_LEVEL; - - // NOTE: as per specification we should actually only register if something is set, but our previous implementation - // always registered a logger, even when nothing was set. Falling back to 'INFO' here to keep the same behavior as - // with previous implementations. - // Also: no point in warning - no logger is registered yet - return ( - logLevelMap[rawLogLevel?.trim().toUpperCase() ?? 'INFO'] ?? - DiagLogLevel.INFO - ); -} diff --git a/metapackages/auto-instrumentations-node/test/utils.test.ts b/metapackages/auto-instrumentations-node/test/utils.test.ts index 7fac6762fc..1241ca6cce 100644 --- a/metapackages/auto-instrumentations-node/test/utils.test.ts +++ b/metapackages/auto-instrumentations-node/test/utils.test.ts @@ -14,12 +14,12 @@ * limitations under the License. */ -import { diag, DiagLogLevel } from '@opentelemetry/api'; +import { diag } from '@opentelemetry/api'; import { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http'; import * as assert from 'assert'; import * as sinon from 'sinon'; import { getNodeAutoInstrumentations } from '../src'; -import { getLogLevelFromEnv, getResourceDetectorsFromEnv } from '../src/utils'; +import { getResourceDetectorsFromEnv } from '../src/utils'; describe('utils', () => { describe('getNodeAutoInstrumentations', () => { @@ -223,44 +223,4 @@ describe('utils', () => { delete process.env.OTEL_NODE_RESOURCE_DETECTORS; }); }); - - describe('getLogLevelFromEnv', function () { - afterEach(function () { - delete process.env.OTEL_LOG_LEVEL; - }); - - it('should select log level based on env var', function () { - process.env.OTEL_LOG_LEVEL = 'NONE'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.NONE); - process.env.OTEL_LOG_LEVEL = 'VERBOSE'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.VERBOSE); - process.env.OTEL_LOG_LEVEL = 'DEBUG'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.DEBUG); - process.env.OTEL_LOG_LEVEL = 'INFO'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.INFO); - process.env.OTEL_LOG_LEVEL = 'WARN'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.WARN); - process.env.OTEL_LOG_LEVEL = 'ERROR'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.ERROR); - process.env.OTEL_LOG_LEVEL = 'ALL'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.ALL); - }); - - it('should ignore casing', function () { - process.env.OTEL_LOG_LEVEL = 'warn'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.WARN); - process.env.OTEL_LOG_LEVEL = 'WaRN'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.WARN); - }); - - it('should fall back to INFO on bogus input', function () { - process.env.OTEL_LOG_LEVEL = 'bogus'; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.INFO); - }); - - it('should use INFO when unset', function () { - delete process.env.OTEL_LOG_LEVEL; - assert.strictEqual(getLogLevelFromEnv(), DiagLogLevel.INFO); - }); - }); }); From 417f37f0d16aa72d280e1d8b3706803b797c61a4 Mon Sep 17 00:00:00 2001 From: Dan Herd Date: Wed, 2 Apr 2025 17:29:56 +0100 Subject: [PATCH 2/3] fix lint --- metapackages/auto-instrumentations-node/src/register.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/metapackages/auto-instrumentations-node/src/register.ts b/metapackages/auto-instrumentations-node/src/register.ts index b0fef3efbb..5ffeb2c32e 100644 --- a/metapackages/auto-instrumentations-node/src/register.ts +++ b/metapackages/auto-instrumentations-node/src/register.ts @@ -15,10 +15,7 @@ */ import * as opentelemetry from '@opentelemetry/sdk-node'; import { diag, DiagConsoleLogger } from '@opentelemetry/api'; -import { - getStringFromEnv, - diagLogLevelFromString, -} from '@opentelemetry/core'; +import { getStringFromEnv, diagLogLevelFromString } from '@opentelemetry/core'; import { getNodeAutoInstrumentations, getResourceDetectorsFromEnv, From 6f148e9a1f0f6fade9996f7fcd4db33aea727e07 Mon Sep 17 00:00:00 2001 From: Dan Herd Date: Wed, 2 Apr 2025 17:42:13 +0100 Subject: [PATCH 3/3] explicit dependency --- metapackages/auto-instrumentations-node/package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/metapackages/auto-instrumentations-node/package.json b/metapackages/auto-instrumentations-node/package.json index 948eb68d7a..0e2720961d 100644 --- a/metapackages/auto-instrumentations-node/package.json +++ b/metapackages/auto-instrumentations-node/package.json @@ -33,10 +33,12 @@ "url": "https://github.com/open-telemetry/opentelemetry-js-contrib/issues" }, "peerDependencies": { - "@opentelemetry/api": "^1.4.1" + "@opentelemetry/api": "^1.4.1", + "@opentelemetry/core": "^2.0.0" }, "devDependencies": { "@opentelemetry/api": "^1.4.1", + "@opentelemetry/core": "^2.0.0", "@types/mocha": "10.0.10", "@types/node": "18.18.14", "@types/sinon": "17.0.4",