refactor(config): parse and resolve defaults in getValueFromEnvSources (#8925)

getValueFromEnvSources returned the raw environment string, so every caller
re-implemented the boolean/int/array parsing Config already does and those copies
drifted. It now maps aliases to the canonical option, runs Config's own parser and
transformer, and returns the typed value, falling back to the registered default
when unset. Callers that must distinguish an unset variable from an explicitly
configured one pass skipDefault and receive undefined instead of the default.

1. Resolving a value pulls in config/defaults, which transitively requires the
   instrumented dns module -> the dns plugin -> log -> back into config/defaults
   before it finished exporting. config/defaults now defers its dns require until
   after module.exports and log requires config/defaults eagerly again, breaking the
   cycle at its source. ritm reads DD_LAMBDA_HANDLER through the new
   getConfiguredEnvName, a presence check that never parses and so cannot re-enter
   the require hook.
2. skipDefault keeps the unset case meaningful where the registered default would
   otherwise mask it: the OTEL_TRACES_EXPORTER fallback in index.js, experimental
   plugin enablement in plugin_manager, the LLMObs programmatic opt-in, and the OTel
   SDK gate.
3. The env helpers no longer tolerate a missing table entry. Every production caller
   passes a registered name, so an unregistered DD_/OTEL_ name is a programming error
   that throws rather than silently returning a raw string.
4. log.configure() recomputes enabled and logLevel from options, env, and defaults on
   every call instead of retaining the previous call's values.

Author: BridgeAR

ci/init.js

@@ -2,7 +2,6 @@
 
 /* eslint-disable no-console */
 const tracer = require('../packages/dd-trace')
-const { isTrue, isFalse } = require('../packages/dd-trace/src/util')
 const log = require('../packages/dd-trace/src/log')
 const { getEnvironmentVariable, getValueFromEnvSources } = require('../packages/dd-trace/src/config/helper')
 
@@ -43,8 +42,10 @@ const baseOptions = {
   flushInterval: isJestWorker ? JEST_FLUSH_INTERVAL : DEFAULT_FLUSH_INTERVAL,
 }
 
-let shouldInit = !isFalse(getValueFromEnvSources('DD_CIVISIBILITY_ENABLED'))
-const isAgentlessEnabled = isTrue(getValueFromEnvSources('DD_CIVISIBILITY_AGENTLESS_ENABLED'))
+// skipDefault: CI visibility stays on unless DD_CIVISIBILITY_ENABLED is explicitly false; the
+// registered default (false) would otherwise turn it off whenever the variable is unset.
+let shouldInit = getValueFromEnvSources('DD_CIVISIBILITY_ENABLED', true) !== false
+const isAgentlessEnabled = getValueFromEnvSources('DD_CIVISIBILITY_AGENTLESS_ENABLED')
 
 if (!isTestWorker && isPackageManager()) {
   log.debug('dd-trace is not initialized in a package manager.')

packages/datadog-instrumentations/src/helpers/register.js

@@ -13,8 +13,9 @@ const Hook = require('./hook')
 const { isRelativeRequire } = require('./shared-utils')
 const rewriter = require('./rewriter')
 
-const DD_TRACE_DISABLED_INSTRUMENTATIONS = getValueFromEnvSources('DD_TRACE_DISABLED_INSTRUMENTATIONS') || ''
-const DD_TRACE_DEBUG = getValueFromEnvSources('DD_TRACE_DEBUG') || ''
+const DD_TRACE_DISABLED_INSTRUMENTATIONS =
+  getValueFromEnvSources('DD_TRACE_DISABLED_INSTRUMENTATIONS')
+const DD_TRACE_DEBUG = getValueFromEnvSources('DD_TRACE_DEBUG')
 
 const hooks = require('./hooks')
 const instrumentations = require('./instrumentations')
@@ -36,7 +37,7 @@ if (!disabledInstrumentations.has('process')) {
   require('../process')
 }
 
-const debugEnabled = DD_TRACE_DEBUG && DD_TRACE_DEBUG.toLowerCase() !== 'false'
+const debugEnabled = DD_TRACE_DEBUG
 checkRequireCache.checkForRequiredModules(debugEnabled)
 if (debugEnabled) {
   setImmediate(checkRequireCache.checkForPotentialConflicts)

packages/datadog-instrumentations/src/otel-sdk-trace.js

@@ -3,7 +3,6 @@
 const shimmer = require('../../datadog-shimmer')
 const tracer = require('../../dd-trace')
 const { getValueFromEnvSources } = require('../../dd-trace/src/config/helper')
-const { isFalse, isTrue } = require('../../dd-trace/src/util')
 const { addHook } = require('./helpers/instrument')
 
 if (isOtelSdkEnabled()) {
@@ -21,9 +20,11 @@ if (isOtelSdkEnabled()) {
 
 function isOtelSdkEnabled () {
   // Datadog explicit opt-out wins over every OTel signal; check it first.
-  const ddTraceOtelEnabled = getValueFromEnvSources('DD_TRACE_OTEL_ENABLED')
-  if (isFalse(ddTraceOtelEnabled)) return false
-  const otelSdkDisabled = getValueFromEnvSources('OTEL_SDK_DISABLED')
-  if (isTrue(otelSdkDisabled)) return false
-  return isTrue(ddTraceOtelEnabled) || isFalse(otelSdkDisabled)
+  // skipDefault: an unset option must stay undefined so the OTel signal still decides — the
+  // registered defaults (false / true) would otherwise force-disable before that check.
+  const ddTraceOtelEnabled = getValueFromEnvSources('DD_TRACE_OTEL_ENABLED', true)
+  if (ddTraceOtelEnabled === false) return false
+  const otelSdkDisabled = getValueFromEnvSources('OTEL_SDK_DISABLED', true)
+  if (otelSdkDisabled) return false
+  return ddTraceOtelEnabled || otelSdkDisabled === false
 }

packages/datadog-instrumentations/src/playwright.js

@@ -59,7 +59,7 @@ const testSuiteToTestStatuses = new Map()
 const testSuiteToErrors = new Map()
 const testsToTestStatuses = new Map()
 
-const RUM_FLUSH_WAIT_TIME = Number(getValueFromEnvSources('DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS')) || 500
+const RUM_FLUSH_WAIT_TIME = getValueFromEnvSources('DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS')
 const DD_PROPERTIES_TIMEOUT = 5000
 
 let applyRepeatEachIndex = null

packages/datadog-instrumentations/src/selenium.js

@@ -19,8 +19,7 @@ if (window.DD_RUM && window.DD_RUM.stopSession) {
 `
 const IS_RUM_ACTIVE_SCRIPT = 'return !!window.DD_RUM'
 
-const DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS =
-Number(getValueFromEnvSources('DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS')) || 500
+const DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS = getValueFromEnvSources('DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS')
 const DD_CIVISIBILITY_TEST_EXECUTION_ID_COOKIE_NAME = 'datadog-ci-visibility-test-execution-id'
 
 // TODO: can we increase the supported version range?

packages/datadog-instrumentations/test/otel-sdk-trace.spec.js

@@ -8,10 +8,13 @@ const sinon = require('sinon')
 
 describe('otel-sdk-trace', () => {
   /**
-   * Re-load `otel-sdk-trace.js` with the supplied env values stubbed in via
-   * `getValueFromEnvSources` and report what `addHook` saw.
+   * Re-load `otel-sdk-trace.js` with the supplied config values stubbed in via
+   * `getValueFromEnvSources` and report what `addHook` saw. `getValueFromEnvSources`
+   * already parses these registered boolean options, so the gate sees `true`,
+   * `false`, or `undefined` — never a raw string. Parsing tolerance ('1', 'maybe',
+   * '') is exercised in the config helper's own spec.
    *
-   * @param {{ ddTraceOtelEnabled?: string, otelSdkDisabled?: string }} env
+   * @param {{ ddTraceOtelEnabled?: boolean, otelSdkDisabled?: boolean }} env
    * @param {{ TracerProvider?: unknown }} [tracerStub]
    * @returns {{ addHook: sinon.SinonSpy, wrap: sinon.SinonSpy }}
    */
@@ -35,48 +38,43 @@ describe('otel-sdk-trace', () => {
 
   describe('gate precedence', () => {
     it('disables when DD_TRACE_OTEL_ENABLED is the explicit opt-out', () => {
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: 'false' }).addHook.called, false)
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: '0' }).addHook.called, false)
+      assert.equal(loadWithEnv({ ddTraceOtelEnabled: false }).addHook.called, false)
     })
 
     it('keeps DD opt-out winning even when OTEL_SDK_DISABLED=false opts in', () => {
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: 'false', otelSdkDisabled: 'false' }).addHook.called, false)
+      assert.equal(loadWithEnv({ ddTraceOtelEnabled: false, otelSdkDisabled: false }).addHook.called, false)
     })
 
     it('disables when OTEL_SDK_DISABLED is the explicit opt-out', () => {
-      assert.equal(loadWithEnv({ otelSdkDisabled: 'true' }).addHook.called, false)
-      assert.equal(loadWithEnv({ otelSdkDisabled: '1' }).addHook.called, false)
+      assert.equal(loadWithEnv({ otelSdkDisabled: true }).addHook.called, false)
     })
 
     it('keeps OTel opt-out winning even when DD_TRACE_OTEL_ENABLED=true opts in', () => {
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: 'true', otelSdkDisabled: 'true' }).addHook.called, false)
+      assert.equal(loadWithEnv({ ddTraceOtelEnabled: true, otelSdkDisabled: true }).addHook.called, false)
     })
 
     it('enables when DD_TRACE_OTEL_ENABLED is the explicit opt-in', () => {
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: 'true' }).addHook.called, true)
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: '1' }).addHook.called, true)
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: 'true', otelSdkDisabled: 'false' }).addHook.called, true)
+      assert.equal(loadWithEnv({ ddTraceOtelEnabled: true }).addHook.called, true)
+      assert.equal(loadWithEnv({ ddTraceOtelEnabled: true, otelSdkDisabled: false }).addHook.called, true)
     })
 
     it('enables when OTEL_SDK_DISABLED=false is the OTel positive opt-in', () => {
-      assert.equal(loadWithEnv({ otelSdkDisabled: 'false' }).addHook.called, true)
-      assert.equal(loadWithEnv({ otelSdkDisabled: '0' }).addHook.called, true)
+      assert.equal(loadWithEnv({ otelSdkDisabled: false }).addHook.called, true)
     })
 
-    it('stays disabled by default when neither env var is set', () => {
+    it('stays disabled by default when neither option is set', () => {
       assert.equal(loadWithEnv({}).addHook.called, false)
     })
 
-    it('stays disabled for unrecognized values on either side', () => {
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: 'maybe', otelSdkDisabled: 'sure' }).addHook.called, false)
-      assert.equal(loadWithEnv({ ddTraceOtelEnabled: '' }).addHook.called, false)
+    it('treats a value the helper rejected (undefined) as unset', () => {
+      assert.equal(loadWithEnv({ ddTraceOtelEnabled: undefined, otelSdkDisabled: undefined }).addHook.called, false)
     })
   })
 
   describe('hook registration', () => {
     it('wraps NodeTracerProvider with the dd-trace TracerProvider', () => {
       const tracerProvider = function FakeTracerProvider () {}
-      const { addHook, wrap } = loadWithEnv({ ddTraceOtelEnabled: 'true' }, { TracerProvider: tracerProvider })
+      const { addHook, wrap } = loadWithEnv({ ddTraceOtelEnabled: true }, { TracerProvider: tracerProvider })
 
       sinon.assert.calledOnce(addHook)
       const [hookOptions, transform] = addHook.firstCall.args

packages/datadog-plugin-dd-trace-api/test/index.spec.js

@@ -12,22 +12,35 @@ const {
 } = require('../../dd-trace/src/config/supported-configurations.json')
 
 const SELF = Symbol('self')
-const supportedConfigurationsWithDdTraceApi = {
-  ...supportedConfigurations,
-  DD_TRACE_DD_TRACE_API_ENABLED: [
-    {
-      implementation: 'A',
-      type: 'boolean',
-      default: 'true',
-    },
-  ],
+
+// The dd-trace-api plugin is not released yet (the `before` hook fires its load event
+// manually below), so its DD_TRACE_DD_TRACE_API_ENABLED flag is intentionally absent from
+// the shipped supported-configurations.json and must never be added there. Register it in an
+// in-memory copy and reload both consumers of that file — config/helper and the config/defaults
+// table it reads — against the copy, so plugin_manager's getEnabled() resolves the flag through
+// real production code instead of throwing on an unknown DD_-prefixed name. Reloading helper
+// alone would leave the defaults table without the entry and only pass while getEnabled keeps
+// short-circuiting before the default lookup.
+const stubbedConfigurations = {
+  supportedConfigurations: {
+    ...supportedConfigurations,
+    DD_TRACE_DD_TRACE_API_ENABLED: [
+      {
+        implementation: 'A',
+        type: 'boolean',
+        default: 'true',
+      },
+    ],
+  },
 }
 
 const configHelperPath = require.resolve('../../dd-trace/src/config/helper')
+const reloadedDefaults = proxyquire.noPreserveCache()(require.resolve('../../dd-trace/src/config/defaults'), {
+  './supported-configurations.json': stubbedConfigurations,
+})
 const reloadedConfigHelper = proxyquire.noPreserveCache()(configHelperPath, {
-  './supported-configurations.json': {
-    supportedConfigurations: supportedConfigurationsWithDdTraceApi,
-  },
+  './supported-configurations.json': stubbedConfigurations,
+  './defaults': reloadedDefaults,
 })
 Object.assign(require(configHelperPath), reloadedConfigHelper)
 

packages/dd-trace/src/config/defaults.js

@@ -1,6 +1,5 @@
 'use strict'
 
-const dns = require('dns')
 const util = require('util')
 
 const { DD_MAJOR } = require('../../../../version')
@@ -51,7 +50,6 @@ const defaults = {
   isServiceUserProvided: false,
   plugins: true,
   isCiVisibility: false,
-  lookup: dns.lookup,
   logger: undefined,
 }
 
@@ -347,3 +345,16 @@ module.exports = {
 
   generateTelemetry,
 }
+
+// `dns` is instrumented, so requiring it pulls in the dns plugin, which loads
+// `log`, whose bootstrap calls back into `require('./defaults')`. Resolve the
+// `lookup` default last — after the exports above are fully built — so that the
+// re-entrant require during that cascade sees the complete module instead of a
+// half-initialized one.
+defaults.lookup = require('dns').lookup
+configWithOrigin.set('lookupdefault', {
+  name: 'lookup',
+  value: defaults.lookup,
+  origin: 'default',
+  seq_id: seqId++,
+})