make sure config origin values are tracked properly for telemetry

DataDog · Sep 18, 2024 · 3bd6bb5 · 3bd6bb5
1 parent 6e8f039
commit 3bd6bb5
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 10 deletions.
diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js
@@ -425,13 +425,6 @@ class Config {
     return inAWSLambda || isGCPFunction || isAzureFunction
   }
 
-  _isLLMObsEnabled () {
-    return coalesce(
-      process.env.DD_LLMOBS_ENABLED,
-      !!this._optionsArg.llmobs
-    )
-  }
-
   // for _merge to work, every config value must have a default value
   _applyDefaults () {
     const {
@@ -606,6 +599,7 @@ class Config {
       DD_INSTRUMENTATION_CONFIG_ID,
       DD_LOGS_INJECTION,
       DD_LLMOBS_AGENTLESS_ENABLED,
+      DD_LLMOBS_ENABLED,
       DD_LLMOBS_ML_APP,
       DD_OPENAI_LOGS_ENABLED,
       DD_OPENAI_SPAN_CHAR_LIMIT,
@@ -748,6 +742,7 @@ class Config {
     this._setBoolean(env, 'isAzureFunction', getIsAzureFunction())
     this._setBoolean(env, 'isGCPFunction', getIsGCPFunction())
     this._setBoolean(env, 'llmobs.agentlessEnabled', DD_LLMOBS_AGENTLESS_ENABLED)
+    this._setBoolean(env, 'llmobs.enabled', DD_LLMOBS_ENABLED)
     this._setString(env, 'llmobs.mlApp', DD_LLMOBS_ML_APP)
     this._setBoolean(env, 'logInjection', DD_LOGS_INJECTION)
     // Requires an accompanying DD_APM_OBFUSCATION_MEMCACHED_KEEP_COMMAND=true in the agent
@@ -955,6 +950,15 @@ class Config {
     this._setBoolean(opts, 'traceId128BitGenerationEnabled', options.traceId128BitGenerationEnabled)
     this._setBoolean(opts, 'traceId128BitLoggingEnabled', options.traceId128BitLoggingEnabled)
     this._setString(opts, 'version', options.version || tags.version)
+
+    // For LLMObs, we want the environment variable to take precedence over the options.
+    // This is reliant on environment config being set before options.
+    // This is to make sure the origins of each value are tracked appropriately for telemetry.
+    // We'll only set `llmobs.enabled` on the opts when it's not set on the environment, and options.llmobs is provided.
+    const llmobsEnabledEnv = this._env['llmobs.enabled']
+    if (llmobsEnabledEnv == null && options.llmobs) {
+      this._setBoolean(opts, 'llmobs.enabled', !!options.llmobs)
+    }
   }
 
   _isCiVisibility () {
@@ -1089,8 +1093,6 @@ class Config {
       calc['tracePropagationStyle.inject'] = calc['tracePropagationStyle.inject'] || defaultPropagationStyle
       calc['tracePropagationStyle.extract'] = calc['tracePropagationStyle.extract'] || defaultPropagationStyle
     }
-
-    this._setBoolean(calc, 'llmobs.enabled', this._isLLMObsEnabled())
   }
 
   _applyRemote (options) {

diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js
@@ -2015,29 +2015,54 @@ describe('Config', () => {
     it('should disable llmobs by default', () => {
       const config = new Config()
       expect(config.llmobs.enabled).to.be.false
+
+      // check origin computation
+      expect(updateConfig.getCall(0).args[0]).to.deep.include({
+        name: 'llmobs.enabled', value: false, origin: 'default'
+      })
     })
 
     it('should enable llmobs if DD_LLMOBS_ENABLED is set to true', () => {
       process.env.DD_LLMOBS_ENABLED = 'true'
       const config = new Config()
       expect(config.llmobs.enabled).to.be.true
+
+      // check origin computation
+      expect(updateConfig.getCall(0).args[0]).to.deep.include({
+        name: 'llmobs.enabled', value: true, origin: 'env_var'
+      })
     })
 
     it('should disable llmobs if DD_LLMOBS_ENABLED is set to false', () => {
       process.env.DD_LLMOBS_ENABLED = 'false'
       const config = new Config()
       expect(config.llmobs.enabled).to.be.false
+
+      // check origin computation
+      expect(updateConfig.getCall(0).args[0]).to.deep.include({
+        name: 'llmobs.enabled', value: false, origin: 'env_var'
+      })
     })
 
     it('should enable llmobs with options and DD_LLMOBS_ENABLED is not set', () => {
       const config = new Config({ llmobs: {} })
       expect(config.llmobs.enabled).to.be.true
+
+      // check origin computation
+      expect(updateConfig.getCall(0).args[0]).to.deep.include({
+        name: 'llmobs.enabled', value: true, origin: 'code'
+      })
     })
 
     it('should have DD_LLMOBS_ENABLED take priority over options', () => {
       process.env.DD_LLMOBS_ENABLED = 'false'
-      const config = new Config({ llmobs: { enabled: true } })
+      const config = new Config({ llmobs: {} })
       expect(config.llmobs.enabled).to.be.false
+
+      // check origin computation
+      expect(updateConfig.getCall(0).args[0]).to.deep.include({
+        name: 'llmobs.enabled', value: false, origin: 'env_var'
+      })
     })
   })