From df8451f994cace0931b2a4c519bac21f077b4fa8 Mon Sep 17 00:00:00 2001 From: YangJH Date: Sat, 19 Jul 2025 15:56:26 +0900 Subject: [PATCH 1/5] 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/5] 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/5] 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`); From e1e4e4b199c66a5840f0b459ff239c281eb5260a Mon Sep 17 00:00:00 2001 From: YangJH Date: Sun, 23 Nov 2025 20:42:32 +0900 Subject: [PATCH 4/5] fix: make programmatic config override env vars --- .../auto-instrumentations-node/src/utils.ts | 72 ++++++++++++------- .../test/utils.test.ts | 26 +++---- 2 files changed, 59 insertions(+), 39 deletions(-) diff --git a/packages/auto-instrumentations-node/src/utils.ts b/packages/auto-instrumentations-node/src/utils.ts index 8dbe604926..74206d40ed 100644 --- a/packages/auto-instrumentations-node/src/utils.ts +++ b/packages/auto-instrumentations-node/src/utils.ts @@ -150,6 +150,44 @@ export type InstrumentationConfigMap = { >; }; +function shouldDisableInstrumentation( + name: string, + userConfig: any, + enabledInstrumentationsFromEnv: string[], + disabledInstrumentationsFromEnv: string[] +): boolean { + // Priority 1: Programmatic config + if (userConfig.enabled === false) { + diag.debug(`Disabling instrumentation for ${name} - disabled by user config`); + return true; + } + + if (userConfig.enabled === true) { + diag.debug(`Enabling instrumentation for ${name} - explicitly enabled by user config`); + return false; + } + + // Priority 2: Environment variables + if (disabledInstrumentationsFromEnv.includes(name)) { + diag.debug(`Disabling instrumentation for ${name} - disabled by env var`); + return true; + } + + const isEnabledEnvSet = !!process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; + if (isEnabledEnvSet && !enabledInstrumentationsFromEnv.includes(name)) { + diag.debug(`Disabling instrumentation for ${name} - not in enabled env var list`); + return true; + } + + // Priority 3: Default exclusions + if (!isEnabledEnvSet && defaultExcludedInstrumentations.includes(name)) { + diag.debug(`Disabling instrumentation for ${name} - excluded by default`); + return true; + } + + return false; +} + export function getNodeAutoInstrumentations( inputConfigs: InstrumentationConfigMap = {} ): Instrumentation[] { @@ -165,36 +203,18 @@ export function getNodeAutoInstrumentations( const Instance = InstrumentationMap[name]; // Defaults are defined by the instrumentation itself const userConfig: any = inputConfigs[name] ?? {}; +ㄷ + const shouldDisable = shouldDisableInstrumentation( + name, + userConfig, + enabledInstrumentationsFromEnv, + disabledInstrumentationsFromEnv + ); - // 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. - // 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`); + if (shouldDisable) { continue; } - 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 { - 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}`); instrumentations.push(new Instance(userConfig)); diff --git a/packages/auto-instrumentations-node/test/utils.test.ts b/packages/auto-instrumentations-node/test/utils.test.ts index d65678273c..b0806aef3e 100644 --- a/packages/auto-instrumentations-node/test/utils.test.ts +++ b/packages/auto-instrumentations-node/test/utils.test.ts @@ -190,18 +190,18 @@ describe('utils', () => { ); }); - it('should respect OTEL_NODE_ENABLED_INSTRUMENTATIONS - env var overrides user config', () => { + it('should respect programmatic config over OTEL_NODE_ENABLED_INSTRUMENTATIONS - user config overrides env var', () => { 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 + enabled: false, // User explicitly disables - should override env var }, '@opentelemetry/instrumentation-express': { - enabled: true, // User tries to enable but not in env var list + enabled: true, // User explicitly enables - should override env var }, }); - + const fsInstrumentation = instrumentations.find( instr => instr.instrumentationName === '@opentelemetry/instrumentation-fs' ); @@ -211,32 +211,32 @@ describe('utils', () => { 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'); + + assert.strictEqual(fsInstrumentation, undefined, 'fs should be disabled by user config despite env var'); + assert.notStrictEqual(httpInstrumentation, undefined, 'http should be enabled by env var (no user override)'); + assert.notStrictEqual(expressInstrumentation, undefined, 'express should be enabled by user config despite not being in env var list'); } finally { delete process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; } }); - it('should respect OTEL_NODE_DISABLED_INSTRUMENTATIONS with absolute priority', () => { + it('should respect programmatic config over OTEL_NODE_DISABLED_INSTRUMENTATIONS - user config overrides env var', () => { process.env.OTEL_NODE_DISABLED_INSTRUMENTATIONS = 'http'; try { const instrumentations = getNodeAutoInstrumentations({ '@opentelemetry/instrumentation-http': { - enabled: true, // User tries to enable but env var disables + enabled: true, // User explicitly enables - should override env var }, }); const httpInstrumentation = instrumentations.find( instr => instr.instrumentationName === '@opentelemetry/instrumentation-http' ); - - assert.strictEqual( + + assert.notStrictEqual( httpInstrumentation, undefined, - 'http instrumentation should be disabled by OTEL_NODE_DISABLED_INSTRUMENTATIONS even when user enables it' + 'http instrumentation should be enabled by user config despite OTEL_NODE_DISABLED_INSTRUMENTATIONS' ); } finally { delete process.env.OTEL_NODE_DISABLED_INSTRUMENTATIONS; From d6aabd1f7c87899e3b0a6b4bcd9d5c13a65259cc Mon Sep 17 00:00:00 2001 From: YangJH Date: Sun, 23 Nov 2025 20:43:51 +0900 Subject: [PATCH 5/5] chore: fix lint error --- .../auto-instrumentations-node/src/utils.ts | 14 +++++++--- .../test/utils.test.ts | 28 +++++++++++++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/packages/auto-instrumentations-node/src/utils.ts b/packages/auto-instrumentations-node/src/utils.ts index 74206d40ed..13c416a117 100644 --- a/packages/auto-instrumentations-node/src/utils.ts +++ b/packages/auto-instrumentations-node/src/utils.ts @@ -158,12 +158,16 @@ function shouldDisableInstrumentation( ): boolean { // Priority 1: Programmatic config if (userConfig.enabled === false) { - diag.debug(`Disabling instrumentation for ${name} - disabled by user config`); + diag.debug( + `Disabling instrumentation for ${name} - disabled by user config` + ); return true; } if (userConfig.enabled === true) { - diag.debug(`Enabling instrumentation for ${name} - explicitly enabled by user config`); + diag.debug( + `Enabling instrumentation for ${name} - explicitly enabled by user config` + ); return false; } @@ -175,7 +179,9 @@ function shouldDisableInstrumentation( const isEnabledEnvSet = !!process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; if (isEnabledEnvSet && !enabledInstrumentationsFromEnv.includes(name)) { - diag.debug(`Disabling instrumentation for ${name} - not in enabled env var list`); + diag.debug( + `Disabling instrumentation for ${name} - not in enabled env var list` + ); return true; } @@ -203,7 +209,7 @@ export function getNodeAutoInstrumentations( const Instance = InstrumentationMap[name]; // Defaults are defined by the instrumentation itself const userConfig: any = inputConfigs[name] ?? {}; -ㄷ + const shouldDisable = shouldDisableInstrumentation( name, userConfig, diff --git a/packages/auto-instrumentations-node/test/utils.test.ts b/packages/auto-instrumentations-node/test/utils.test.ts index b0806aef3e..c83fc520c0 100644 --- a/packages/auto-instrumentations-node/test/utils.test.ts +++ b/packages/auto-instrumentations-node/test/utils.test.ts @@ -203,18 +203,34 @@ describe('utils', () => { }); const fsInstrumentation = instrumentations.find( - instr => instr.instrumentationName === '@opentelemetry/instrumentation-fs' + instr => + instr.instrumentationName === '@opentelemetry/instrumentation-fs' ); const httpInstrumentation = instrumentations.find( - instr => instr.instrumentationName === '@opentelemetry/instrumentation-http' + instr => + instr.instrumentationName === '@opentelemetry/instrumentation-http' ); const expressInstrumentation = instrumentations.find( - instr => instr.instrumentationName === '@opentelemetry/instrumentation-express' + instr => + instr.instrumentationName === + '@opentelemetry/instrumentation-express' ); - assert.strictEqual(fsInstrumentation, undefined, 'fs should be disabled by user config despite env var'); - assert.notStrictEqual(httpInstrumentation, undefined, 'http should be enabled by env var (no user override)'); - assert.notStrictEqual(expressInstrumentation, undefined, 'express should be enabled by user config despite not being in env var list'); + assert.strictEqual( + fsInstrumentation, + undefined, + 'fs should be disabled by user config despite env var' + ); + assert.notStrictEqual( + httpInstrumentation, + undefined, + 'http should be enabled by env var (no user override)' + ); + assert.notStrictEqual( + expressInstrumentation, + undefined, + 'express should be enabled by user config despite not being in env var list' + ); } finally { delete process.env.OTEL_NODE_ENABLED_INSTRUMENTATIONS; }