From df8451f994cace0931b2a4c519bac21f077b4fa8 Mon Sep 17 00:00:00 2001 From: YangJH Date: Sat, 19 Jul 2025 15:56:26 +0900 Subject: [PATCH 1/3] fix: ensure environment variables take precedence over programmatic config --- .../auto-instrumentations-node/src/utils.ts | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/packages/auto-instrumentations-node/src/utils.ts b/packages/auto-instrumentations-node/src/utils.ts index eea8d06869..88f85f4196 100644 --- a/packages/auto-instrumentations-node/src/utils.ts +++ b/packages/auto-instrumentations-node/src/utils.ts @@ -166,14 +166,37 @@ export function getNodeAutoInstrumentations( // Defaults are defined by the instrumentation itself const userConfig: any = inputConfigs[name] ?? {}; - if ( - userConfig.enabled === false || - !enabledInstrumentationsFromEnv.includes(name) || - disabledInstrumentationsFromEnv.includes(name) - ) { - diag.debug(`Disabling instrumentation for ${name}`); + // Configuration priority: Environment variables > programmatic config + // If both OTEL_NODE_ENABLED_INSTRUMENTATIONS and OTEL_NODE_DISABLED_INSTRUMENTATIONS are set, + // ENABLED is applied first, then DISABLED is applied to that list. + // If the same instrumentation is in both lists, it will be disabled. + + // 1. OTEL_NODE_DISABLED_INSTRUMENTATIONS takes absolute priority + if (disabledInstrumentationsFromEnv.includes(name)) { + diag.debug(`Disabling instrumentation for ${name} - disabled by env var`); continue; } + + // 2. If OTEL_NODE_ENABLED_INSTRUMENTATIONS is set, only enable those in the list + const isEnabledEnvSet = !!process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; + if (isEnabledEnvSet) { + if (!enabledInstrumentationsFromEnv.includes(name)) { + diag.debug(`Disabling instrumentation for ${name} - not in enabled env var list`); + continue; + } + } else { + // 3. If no env vars are set, fall back to programmatic config and defaults + if (userConfig.enabled === false) { + diag.debug(`Disabling instrumentation for ${name} - disabled by user config`); + continue; + } + + const isDefaultExcluded = defaultExcludedInstrumentations.includes(name); + if (isDefaultExcluded && userConfig.enabled !== true) { + diag.debug(`Disabling instrumentation for ${name} - excluded by default and not explicitly enabled`); + continue; + } + } try { diag.debug(`Loading instrumentation for ${name}`); From 9bf97f8ff9fd9cd51bf383a1bd8cb08eed4d4004 Mon Sep 17 00:00:00 2001 From: YangJH Date: Sat, 19 Jul 2025 15:56:36 +0900 Subject: [PATCH 2/3] test: add comprehensive tests for environment variable precedence --- .../test/utils.test.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/packages/auto-instrumentations-node/test/utils.test.ts b/packages/auto-instrumentations-node/test/utils.test.ts index 1241ca6cce..d65678273c 100644 --- a/packages/auto-instrumentations-node/test/utils.test.ts +++ b/packages/auto-instrumentations-node/test/utils.test.ts @@ -172,6 +172,76 @@ describe('utils', () => { spy.restore(); }); + + it('should enable fs instrumentation when explicitly enabled and OTEL_NODE_ENABLED_INSTRUMENTATIONS is not set', () => { + const instrumentations = getNodeAutoInstrumentations({ + '@opentelemetry/instrumentation-fs': { + enabled: true, + }, + }); + const fsInstrumentation = instrumentations.find( + instr => + instr.instrumentationName === '@opentelemetry/instrumentation-fs' + ); + assert.notStrictEqual( + fsInstrumentation, + undefined, + 'fs instrumentation should be included when explicitly enabled in config and env var is not set' + ); + }); + + it('should respect OTEL_NODE_ENABLED_INSTRUMENTATIONS - env var overrides user config', () => { + process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS = 'fs,http'; + try { + const instrumentations = getNodeAutoInstrumentations({ + '@opentelemetry/instrumentation-fs': { + enabled: false, // User tries to disable but env var overrides + }, + '@opentelemetry/instrumentation-express': { + enabled: true, // User tries to enable but not in env var list + }, + }); + + const fsInstrumentation = instrumentations.find( + instr => instr.instrumentationName === '@opentelemetry/instrumentation-fs' + ); + const httpInstrumentation = instrumentations.find( + instr => instr.instrumentationName === '@opentelemetry/instrumentation-http' + ); + const expressInstrumentation = instrumentations.find( + instr => instr.instrumentationName === '@opentelemetry/instrumentation-express' + ); + + assert.notStrictEqual(fsInstrumentation, undefined, 'fs should be enabled by env var despite user config'); + assert.notStrictEqual(httpInstrumentation, undefined, 'http should be enabled by env var'); + assert.strictEqual(expressInstrumentation, undefined, 'express should be disabled - not in env var list'); + } finally { + delete process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; + } + }); + + it('should respect OTEL_NODE_DISABLED_INSTRUMENTATIONS with absolute priority', () => { + process.env.OTEL_NODE_DISABLED_INSTRUMENTATIONS = 'http'; + try { + const instrumentations = getNodeAutoInstrumentations({ + '@opentelemetry/instrumentation-http': { + enabled: true, // User tries to enable but env var disables + }, + }); + const httpInstrumentation = instrumentations.find( + instr => + instr.instrumentationName === '@opentelemetry/instrumentation-http' + ); + + assert.strictEqual( + httpInstrumentation, + undefined, + 'http instrumentation should be disabled by OTEL_NODE_DISABLED_INSTRUMENTATIONS even when user enables it' + ); + } finally { + delete process.env.OTEL_NODE_DISABLED_INSTRUMENTATIONS; + } + }); }); describe('getResourceDetectorsFromEnv', () => { From a4585dcad999b86b12725d7e71b645beb7a2aaf9 Mon Sep 17 00:00:00 2001 From: YangJH Date: Sat, 19 Jul 2025 16:07:32 +0900 Subject: [PATCH 3/3] chore: remove redundant comments --- packages/auto-instrumentations-node/src/utils.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/auto-instrumentations-node/src/utils.ts b/packages/auto-instrumentations-node/src/utils.ts index 88f85f4196..8dbe604926 100644 --- a/packages/auto-instrumentations-node/src/utils.ts +++ b/packages/auto-instrumentations-node/src/utils.ts @@ -167,17 +167,15 @@ export function getNodeAutoInstrumentations( const userConfig: any = inputConfigs[name] ?? {}; // Configuration priority: Environment variables > programmatic config - // If both OTEL_NODE_ENABLED_INSTRUMENTATIONS and OTEL_NODE_DISABLED_INSTRUMENTATIONS are set, - // ENABLED is applied first, then DISABLED is applied to that list. + // If both OTEL_NODE_ENABLED_INSTRUMENTATIONS and OTEL_NODE_DISABLED_INSTRUMENTATIONS are set, ENABLED is applied first, then DISABLED is applied to that list. // If the same instrumentation is in both lists, it will be disabled. - - // 1. OTEL_NODE_DISABLED_INSTRUMENTATIONS takes absolute priority + // When env vars are unset, allow programmatic config to override default exclusions + if (disabledInstrumentationsFromEnv.includes(name)) { diag.debug(`Disabling instrumentation for ${name} - disabled by env var`); continue; } - - // 2. If OTEL_NODE_ENABLED_INSTRUMENTATIONS is set, only enable those in the list + const isEnabledEnvSet = !!process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; if (isEnabledEnvSet) { if (!enabledInstrumentationsFromEnv.includes(name)) { @@ -185,12 +183,11 @@ export function getNodeAutoInstrumentations( continue; } } else { - // 3. If no env vars are set, fall back to programmatic config and defaults if (userConfig.enabled === false) { diag.debug(`Disabling instrumentation for ${name} - disabled by user config`); continue; } - + const isDefaultExcluded = defaultExcludedInstrumentations.includes(name); if (isDefaultExcluded && userConfig.enabled !== true) { diag.debug(`Disabling instrumentation for ${name} - excluded by default and not explicitly enabled`);