fix: make programmatic config override env vars

Author: YangJonghun

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));

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;