Remove tight coupling between logger and config

init.js

@@ -2,22 +2,13 @@
 
 const path = require('path')
 const Module = require('module')
-const telemetry = require('./packages/dd-trace/src/telemetry/init-telemetry')
 const semver = require('semver')
+const { isTrue } = require('./packages/dd-trace/src/util')
+const telemetry = require('./packages/dd-trace/src/telemetry/init-telemetry')
 
-function isTrue (envVar) {
-  return ['1', 'true', 'True'].includes(envVar)
-}
-
-// eslint-disable-next-line no-console
-let log = { info: isTrue(process.env.DD_TRACE_DEBUG) ? console.log : () => {} }
-if (semver.satisfies(process.versions.node, '>=16')) {
-  const Config = require('./packages/dd-trace/src/config')
-  log = require('./packages/dd-trace/src/log')
-
-  // eslint-disable-next-line no-new
-  new Config() // we need this to initialize the logger
-}
+const log = semver.satisfies(process.versions.node, '>=16')
+  ? require('./packages/dd-trace/src/log')
+  : { info: isTrue(process.env.DD_TRACE_DEBUG) ? console.log : () => {} } // eslint-disable-line no-console
 
 let initBailout = false
 let clobberBailout = false

packages/dd-trace/src/config.js

@@ -211,33 +211,23 @@ function propagationStyle (key, option, defaultValue) {
 }
 
 class Config {
-  constructor (options) {
-    options = options || {}
+  constructor (options = {}) {
     options = this.options = {
       ...options,
       appsec: options.appsec != null ? options.appsec : options.experimental?.appsec,
       iast: options.iast != null ? options.iast : options.experimental?.iast
     }
 
-    checkIfBothOtelAndDdEnvVarSet()
-
     // Configure the logger first so it can be used to warn about other configs
-    this.debug = isTrue(coalesce(
-      process.env.DD_TRACE_DEBUG,
-      process.env.OTEL_LOG_LEVEL && process.env.OTEL_LOG_LEVEL === 'debug',
-      false
-    ))
-    this.logger = options.logger
-
-    this.logLevel = coalesce(
-      options.logLevel,
-      process.env.DD_TRACE_LOG_LEVEL,
-      process.env.OTEL_LOG_LEVEL,
-      'debug'
-    )
+    const logConfig = log.getConfig()
+    this.debug = logConfig.enabled
+    this.logger = coalesce(options.logger, logConfig.logger)
+    this.logLevel = coalesce(options.logLevel, logConfig.logLevel)
 
     log.use(this.logger)
-    log.toggle(this.debug, this.logLevel, this)
+    log.toggle(this.debug, this.logLevel)
+
+    checkIfBothOtelAndDdEnvVarSet()
 
     const DD_TRACE_MEMCACHED_COMMAND_ENABLED = coalesce(
       process.env.DD_TRACE_MEMCACHED_COMMAND_ENABLED,

packages/dd-trace/src/log/index.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const coalesce = require('koalas')
+const { isTrue } = require('../util')
 const { debugChannel, infoChannel, warnChannel, errorChannel } = require('./channels')
 const logWriter = require('./writer')
 
@@ -20,13 +22,29 @@ function processMsg (msg) {
   return typeof msg === 'function' ? msg() : msg
 }
 
+const config = {
+  enabled: false,
+  logger: undefined,
+  logLevel: 'debug'
+}
+
 const log = {
+  /**
+   * @returns Read-only version of logging config. To modify config, call `log.use` and `log.toggle`
+   */
+  getConfig () {
+    return Object.freeze({ ...config })
+  },
+
   use (logger) {
+    config.logger = logger
     logWriter.use(logger)
     return this
   },
 
   toggle (enabled, logLevel) {
+    config.enabled = enabled
+    config.logLevel = logLevel
     logWriter.toggle(enabled, logLevel)
     return this
   },
@@ -76,4 +94,18 @@ const log = {
 
 log.reset()
 
+const enabled = isTrue(coalesce(
+  process.env.DD_TRACE_DEBUG,
+  process.env.OTEL_LOG_LEVEL === 'debug',
+  config.enabled
+))
+
+const logLevel = coalesce(
+  process.env.DD_TRACE_LOG_LEVEL,
+  process.env.OTEL_LOG_LEVEL,
+  config.logLevel
+)
+
+log.toggle(enabled, logLevel)
+
 module.exports = log

packages/dd-trace/test/config.spec.js

@@ -28,19 +28,28 @@ describe('Config', () => {
   const BLOCKED_TEMPLATE_GRAPHQL = readFileSync(BLOCKED_TEMPLATE_GRAPHQL_PATH, { encoding: 'utf8' })
   const DD_GIT_PROPERTIES_FILE = require.resolve('./fixtures/config/git.properties')
 
+  function reloadLoggerAndConfig () {
+    log = proxyquire('../src/log', {})
+    log.use = sinon.spy()
+    log.toggle = sinon.spy()
+    log.warn = sinon.spy()
+    log.error = sinon.spy()
+
+    Config = proxyquire('../src/config', {
+      './pkg': pkg,
+      './log': log,
+      './telemetry': { updateConfig },
+      fs,
+      os
+    })
+  }
+
   beforeEach(() => {
     pkg = {
       name: '',
       version: ''
     }
 
-    log = {
-      use: sinon.spy(),
-      toggle: sinon.spy(),
-      warn: sinon.spy(),
-      error: sinon.spy()
-    }
-
     updateConfig = sinon.stub()
 
     env = process.env
@@ -58,13 +67,7 @@ describe('Config', () => {
     }
     osType = 'Linux'
 
-    Config = proxyquire('../src/config', {
-      './pkg': pkg,
-      './log': log,
-      './telemetry': { updateConfig },
-      fs,
-      os
-    })
+    reloadLoggerAndConfig()
   })
 
   afterEach(() => {
@@ -73,6 +76,19 @@ describe('Config', () => {
     existsSyncParam = undefined
   })
 
+  it('should initialize its own logging config based off the loggers config', () => {
+    process.env.DD_TRACE_DEBUG = 'true'
+    process.env.DD_TRACE_LOG_LEVEL = 'error'
+
+    reloadLoggerAndConfig()
+
+    const config = new Config()
+
+    expect(config).to.have.property('debug', true)
+    expect(config).to.have.property('logger', undefined)
+    expect(config).to.have.property('logLevel', 'error')
+  })
+
   it('should initialize from environment variables with DD env vars taking precedence OTEL env vars', () => {
     process.env.DD_SERVICE = 'service'
     process.env.OTEL_SERVICE_NAME = 'otel_service'
@@ -92,6 +108,9 @@ describe('Config', () => {
     process.env.DD_TRACE_PROPAGATION_STYLE_EXTRACT = 'b3,tracecontext'
     process.env.OTEL_PROPAGATORS = 'datadog,tracecontext'
 
+    // required if we want to check updates to config.debug and config.logLevel which is fetched from logger
+    reloadLoggerAndConfig()
+
     const config = new Config()
 
     expect(config).to.have.property('debug', false)
@@ -119,6 +138,9 @@ describe('Config', () => {
     process.env.OTEL_RESOURCE_ATTRIBUTES = 'foo=bar1,baz=qux1'
     process.env.OTEL_PROPAGATORS = 'b3,datadog'
 
+    // required if we want to check updates to config.debug and config.logLevel which is fetched from logger
+    reloadLoggerAndConfig()
+
     const config = new Config()
 
     expect(config).to.have.property('debug', true)
@@ -464,6 +486,9 @@ describe('Config', () => {
     process.env.DD_INSTRUMENTATION_INSTALL_TIME = '1703188212'
     process.env.DD_INSTRUMENTATION_CONFIG_ID = 'abcdef123'
 
+    // required if we want to check updates to config.debug and config.logLevel which is fetched from logger
+    reloadLoggerAndConfig()
+
     const config = new Config()
 
     expect(config).to.have.property('tracing', false)
@@ -633,13 +658,13 @@ describe('Config', () => {
 
   it('should read case-insensitive booleans from environment variables', () => {
     process.env.DD_TRACING_ENABLED = 'False'
-    process.env.DD_TRACE_DEBUG = 'TRUE'
+    process.env.DD_TRACE_PROPAGATION_EXTRACT_FIRST = 'TRUE'
     process.env.DD_RUNTIME_METRICS_ENABLED = '0'
 
     const config = new Config()
 
     expect(config).to.have.property('tracing', false)
-    expect(config).to.have.property('debug', true)
+    expect(config).to.have.property('tracePropagationExtractFirst', true)
     expect(config).to.have.property('runtimeMetrics', false)
   })
 
@@ -649,14 +674,12 @@ describe('Config', () => {
     process.env.DD_TRACE_AGENT_HOSTNAME = 'agent'
     process.env.DD_TRACE_AGENT_PORT = '6218'
     process.env.DD_TRACING_ENABLED = 'false'
-    process.env.DD_TRACE_DEBUG = 'true'
     process.env.DD_SERVICE = 'service'
     process.env.DD_ENV = 'test'
 
     const config = new Config()
 
     expect(config).to.have.property('tracing', false)
-    expect(config).to.have.property('debug', true)
     expect(config).to.have.nested.property('dogstatsd.hostname', 'agent')
     expect(config).to.have.nested.property('url.protocol', 'https:')
     expect(config).to.have.nested.property('url.hostname', 'agent2')