fix dd-otel baggage handling

keep span-specific baggage APIs exclusively for legacy, and use global baggage for dd-otel baggage translation

Author: ida613

packages/dd-trace/src/opentelemetry/context_manager.js

@@ -2,6 +2,7 @@
 
 const { trace, ROOT_CONTEXT, propagation } = require('@opentelemetry/api')
 const { storage } = require('../../../datadog-core')
+const { getAllBaggageItems, setBaggageItem, removeAllBaggageItems } = require('../baggage')
 
 const tracer = require('../../')
 const SpanContext = require('./span_context')
@@ -19,31 +20,26 @@ class ContextManager {
 
     const storedSpan = store ? trace.getSpan(store) : null
 
+    // Convert DD baggage to OTel format
+    const baggages = getAllBaggageItems()
+    const hasBaggage = Object.keys(baggages).length > 0
+    let otelBaggages
+    if (hasBaggage) {
+      const entries = {}
+      for (const [key, value] of Object.entries(baggages)) {
+        entries[key] = { value }
+      }
+      otelBaggages = propagation.createBaggage(entries)
+    }
+
     // If stored span wraps the active DD span, prefer the stored context
     if (storedSpan && storedSpan._ddSpan === activeSpan) {
-      const baggages = JSON.parse(activeSpan.getAllBaggageItems())
-      if (Object.keys(baggages).length > 0) {
-        const entries = {}
-        for (const [key, value] of Object.entries(baggages)) {
-          entries[key] = { value }
-        }
-        const otelBaggages = propagation.createBaggage(entries)
-        return propagation.setBaggage(store, otelBaggages)
-      }
+      if (otelBaggages) return propagation.setBaggage(store, otelBaggages)
       return store
     }
 
     if (!activeSpan) {
-      const storedBaggageItems = storedSpan?._spanContext?._ddContext?._baggageItems
-      if (storedBaggageItems) {
-        const baggages = storedBaggageItems
-        const entries = {}
-        for (const [key, value] of Object.entries(baggages)) {
-          entries[key] = { value }
-        }
-        const otelBaggages = propagation.createBaggage(entries)
-        return propagation.setBaggage(baseContext, otelBaggages)
-      }
+      if (otelBaggages) return propagation.setBaggage(baseContext, otelBaggages)
       return baseContext
     }
 
@@ -53,18 +49,6 @@ class ContextManager {
       ddContext._otelSpanContext = new SpanContext(ddContext)
     }
 
-    // Convert DD baggage to OTel format
-    const baggages = JSON.parse(activeSpan.getAllBaggageItems())
-    const hasBaggage = Object.keys(baggages).length > 0
-    let otelBaggages
-    if (hasBaggage) {
-      const entries = {}
-      for (const [key, value] of Object.entries(baggages)) {
-        entries[key] = { value }
-      }
-      otelBaggages = propagation.createBaggage(entries)
-    }
-
     if (store && trace.getSpanContext(store) === ddContext._otelSpanContext) {
       return otelBaggages ? propagation.setBaggage(store, otelBaggages) : store
     }
@@ -86,22 +70,11 @@ class ContextManager {
     if (baggages) {
       baggageItems = baggages.getAllEntries()
     }
-    if (span && span._ddSpan) {
-      // does otel always override datadog?
-      span._ddSpan.removeAllBaggageItems()
-      for (const baggage of baggageItems) {
-        span._ddSpan.setBaggageItem(baggage[0], baggage[1].value)
-      }
-      return ddScope.activate(span._ddSpan, run)
-    }
-    // span instanceof NonRecordingSpan
-    const ddContext = span?._spanContext?._ddContext
-    if (ddContext && ddContext._baggageItems) {
-      ddContext._baggageItems = {}
-      for (const baggage of baggageItems) {
-        ddContext._baggageItems[baggage[0]] = baggage[1].value
-      }
+    removeAllBaggageItems()
+    for (const baggage of baggageItems) {
+      setBaggageItem(baggage[0], baggage[1].value)
     }
+    if (span && span._ddSpan) return ddScope.activate(span._ddSpan, run)
     return run()
   }
 

packages/dd-trace/test/opentelemetry/context_manager.spec.js

@@ -5,18 +5,12 @@ const assert = require('node:assert/strict')
 const { describe, it, beforeEach } = require('mocha')
 const { context, propagation, trace, ROOT_CONTEXT } = require('@opentelemetry/api')
 const api = require('@opentelemetry/api')
+const { getAllBaggageItems, setBaggageItem, removeBaggageItem } = require('../../src/baggage')
 
 require('../setup/core')
 const ContextManager = require('../../src/opentelemetry/context_manager')
 const TracerProvider = require('../../src/opentelemetry/tracer_provider')
-const tracer = require('../../').init()
-
-function makeSpan (...args) {
-  const tracerProvider = new TracerProvider()
-  tracerProvider.register()
-  const tracer = tracerProvider.getTracer()
-  return tracer.startSpan(...args)
-}
+require('../../').init()
 
 function getTracer () {
   const tracerProvider = new TracerProvider()
@@ -138,63 +132,46 @@ describe('OTel Context Manager', () => {
     }
     const baggage = propagation.createBaggage(entries)
     const contextWithBaggage = propagation.setBaggage(context.active(), baggage)
-    const span = makeSpan('otel-to-dd')
-    const contextWithSpan = trace.setSpan(contextWithBaggage, span)
-    api.context.with(contextWithSpan, () => {
-      assert.strictEqual(tracer.scope().active().getBaggageItem('foo'), 'bar')
+    api.context.with(contextWithBaggage, () => {
+      assert.deepStrictEqual(getAllBaggageItems(), { foo: 'bar' })
     })
   })
 
   it('should propagate baggage from a datadog span to an otel span', () => {
-    const baggageKey = 'raccoon'
-    const baggageVal = 'chunky'
-    const ddSpan = tracer.startSpan('dd-to-otel')
-    ddSpan.setBaggageItem(baggageKey, baggageVal)
-    tracer.scope().activate(ddSpan, () => {
-      const baggages = propagation.getActiveBaggage().getAllEntries()
-      assert.strictEqual(baggages.length, 1)
-      const baggage = baggages[0]
-      assert.strictEqual(baggage[0], baggageKey)
-      assert.strictEqual(baggage[1].value, baggageVal)
-    })
+    setBaggageItem('raccoon', 'chunky')
+    assert.deepStrictEqual(propagation.getActiveBaggage().getAllEntries(),
+      [['raccoon', { value: 'chunky' }]]
+    )
   })
 
   it('should handle dd-otel baggage conflict', () => {
-    const ddSpan = tracer.startSpan('dd')
-    ddSpan.setBaggageItem('key1', 'dd1')
-    let contextWithUpdatedBaggages
-    tracer.scope().activate(ddSpan, () => {
-      let baggages = propagation.getBaggage(api.context.active())
-      baggages = baggages.setEntry('key1', { value: 'otel1' })
-      baggages = baggages.setEntry('key2', { value: 'otel2' })
-      contextWithUpdatedBaggages = propagation.setBaggage(api.context.active(), baggages)
-    })
-    assert.deepStrictEqual(JSON.parse(ddSpan.getAllBaggageItems()), { key1: 'dd1' })
+    setBaggageItem('key1', 'dd1')
+    let baggages = propagation.getActiveBaggage()
+    baggages = baggages.setEntry('key1', { value: 'otel1' })
+    baggages = baggages.setEntry('key2', { value: 'otel2' })
+    const contextWithUpdatedBaggages = propagation.setBaggage(context.active(), baggages)
+    assert.deepStrictEqual(getAllBaggageItems(), { key1: 'dd1' })
     api.context.with(contextWithUpdatedBaggages, () => {
-      assert.deepStrictEqual(JSON.parse(ddSpan.getAllBaggageItems()), { key1: 'otel1', key2: 'otel2' })
-      ddSpan.setBaggageItem('key2', 'dd2')
-      assert.deepStrictEqual(propagation.getActiveBaggage().getAllEntries(),
-        [['key1', { value: 'otel1' }], ['key2', { value: 'dd2' }]]
-      )
+      assert.deepStrictEqual(getAllBaggageItems(), { key1: 'otel1', key2: 'otel2' })
     })
+    setBaggageItem('key2', 'dd2')
+    assert.deepStrictEqual(propagation.getActiveBaggage().getAllEntries(),
+      [['key1', { value: 'otel1' }], ['key2', { value: 'dd2' }]]
+    )
   })
 
   it('should handle dd-otel baggage removal', () => {
-    const ddSpan = tracer.startSpan('dd')
-    ddSpan.setBaggageItem('key1', 'dd1')
-    ddSpan.setBaggageItem('key2', 'dd2')
-    let contextWithUpdatedBaggages
-    tracer.scope().activate(ddSpan, () => {
-      let baggages = propagation.getBaggage(api.context.active())
-      baggages = baggages.removeEntry('key1')
-      contextWithUpdatedBaggages = propagation.setBaggage(api.context.active(), baggages)
-    })
-    assert.deepStrictEqual(JSON.parse(ddSpan.getAllBaggageItems()), { key1: 'dd1', key2: 'dd2' })
+    setBaggageItem('key1', 'dd1')
+    setBaggageItem('key2', 'dd2')
+    let baggages = propagation.getActiveBaggage()
+    baggages = baggages.removeEntry('key1')
+    const contextWithUpdatedBaggages = propagation.setBaggage(context.active(), baggages)
+    assert.deepStrictEqual(getAllBaggageItems(), { key1: 'dd1', key2: 'dd2' })
     api.context.with(contextWithUpdatedBaggages, () => {
-      assert.deepStrictEqual(JSON.parse(ddSpan.getAllBaggageItems()), { key2: 'dd2' })
-      ddSpan.removeBaggageItem('key2')
-      assert.deepStrictEqual(propagation.getActiveBaggage().getAllEntries(), [])
+      assert.deepStrictEqual(getAllBaggageItems(), { key2: 'dd2' })
     })
+    removeBaggageItem('key2')
+    assert.deepStrictEqual(propagation.getActiveBaggage(), undefined)
   })
 
   it('should return active span', () => {