Make DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED consistent (#5223)

* Make DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED consistent
DataDog · Feb 24, 2025 · e26ea08 · e26ea08
1 parent af9d28d
commit e26ea08
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 4 deletions.
diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js
@@ -46,6 +46,7 @@ const tracestateTagKeyFilter = /[^\x21-\x2b\x2d-\x3c\x3e-\x7e]/g
 const tracestateTagValueFilter = /[^\x20-\x2b\x2d-\x3a\x3c-\x7d]/g
 const invalidSegment = /^0+$/
 const zeroTraceId = '0000000000000000'
+const hex16 = /^[0-9A-Fa-f]{16}$/
 
 class TextMapPropagator {
   constructor (config) {
@@ -482,10 +483,20 @@ class TextMapPropagator {
               }
               break
             }
-            default:
+            default: {
               if (!key.startsWith('t.')) continue
-              spanContext._trace.tags[`_dd.p.${key.slice(2)}`] = value
-                .replace(/[\x7e]/gm, '=')
+              const subKey = key.slice(2) // e.g. t.tid -> tid
+              const transformedValue = value.replace(/[\x7e]/gm, '=')
+
+              // If subkey is tid  then do nothing because trace header tid should always be preserved
+              if (subKey === 'tid') {
+                if (!hex16.test(value) || spanContext._trace.tags['_dd.p.tid'] !== transformedValue) {
+                  log.error(`Invalid trace id ${value} in tracestate, skipping`)
+                }
+                continue
+              }
+              spanContext._trace.tags[`_dd.p.${subKey}`] = transformedValue
+            }
           }
         }
       })
@@ -645,7 +656,11 @@ class TextMapPropagator {
           log.error('Trace tags from carrier are invalid, skipping extraction.')
           return
         }
-
+        // Check if value is a valid 16 character lower-case hexadecimal encoded number as per spec
+        if (key === '_dd.p.tid' && !(hex16.test(value))) {
+          log.error(`Invalid _dd.p.tid tag ${value}, skipping`)
+          continue
+        }
         tags[key] = value
       }
 

diff --git a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js
@@ -451,6 +451,41 @@ describe('TextMapPropagator', () => {
       expect(spanContextD._baggageItems).to.deep.equal({})
     })
 
+    it('should discard malformed tids', () => {
+      // tid with malformed characters
+      let carrier = {
+        'x-datadog-trace-id': '1234567890123456789',
+        'x-datadog-parent-id': '987654321',
+        'x-datadog-tags': '_dd.p.tid=1234567890abcdeX'
+      }
+      let spanContext = propagator.extract(carrier)
+      expect(spanContext.toTraceId()).to.equal(carrier['x-datadog-trace-id'])
+      expect(spanContext.toSpanId()).to.equal(carrier['x-datadog-parent-id'])
+      expect(spanContext._trace.tags).to.not.have.property('_dd.p.tid')
+
+      // tid too long
+      carrier = {
+        'x-datadog-trace-id': '234567890123456789',
+        'x-datadog-parent-id': '987654321',
+        'x-datadog-tags': '_dd.p.tid=1234567890abcdef1'
+      }
+      spanContext = propagator.extract(carrier)
+      expect(spanContext.toTraceId()).to.equal(carrier['x-datadog-trace-id'])
+      expect(spanContext.toSpanId()).to.equal(carrier['x-datadog-parent-id'])
+      expect(spanContext._trace.tags).to.not.have.property('_dd.p.tid')
+
+      // tid too short
+      carrier = {
+        'x-datadog-trace-id': '1234567890123456789',
+        'x-datadog-parent-id': '987654321',
+        'x-datadog-tags': '_dd.p.tid=1234567890abcde'
+      }
+      spanContext = propagator.extract(carrier)
+      expect(spanContext.toTraceId()).to.equal(carrier['x-datadog-trace-id'])
+      expect(spanContext.toSpanId()).to.equal(carrier['x-datadog-parent-id'])
+      expect(spanContext._trace.tags).to.not.have.property('_dd.p.tid')
+    })
+
     // temporary test. On the contrary, it SHOULD extract baggage
     it('should not extract baggage when it is the only propagation style', () => {
       config = new Config({
@@ -630,6 +665,30 @@ describe('TextMapPropagator', () => {
       expect(spanContext._trace.tags).to.have.property('_dd.parent_id', '2244eeee6666aaaa')
     })
 
+    it('should preserve trace header tid when tracestate contains an inconsistent tid', () => {
+      textMap.traceparent = '00-640cfd8d00000000abcdefab12345678-000000003ade68b1-01'
+      textMap.tracestate = 'dd=t.tid:640cfd8d0000ffff'
+      config.tracePropagationStyle.extract = ['tracecontext']
+
+      const carrier = textMap
+      const spanContext = propagator.extract(carrier)
+
+      expect(spanContext._traceId.toString(16)).to.equal('640cfd8d00000000abcdefab12345678')
+      expect(spanContext._trace.tags).to.have.property('_dd.p.tid', '640cfd8d00000000')
+    })
+
+    it('should preserve trace header tid when tracestate contains a malformed tid', () => {
+      textMap.traceparent = '00-640cfd8d00000000abcdefab12345678-000000003ade68b1-01'
+      textMap.tracestate = 'dd=t.tid:XXXX'
+      config.tracePropagationStyle.extract = ['tracecontext']
+
+      const carrier = textMap
+      const spanContext = propagator.extract(carrier)
+
+      expect(spanContext._traceId.toString(16)).to.equal('640cfd8d00000000abcdefab12345678')
+      expect(spanContext._trace.tags).to.have.property('_dd.p.tid', '640cfd8d00000000')
+    })
+
     it('should set the last datadog parent id to zero when p: is NOT in the tracestate', () => {
       textMap.traceparent = '00-0000000000000000000000000000007B-0000000000000456-01'
       textMap.tracestate = 'other=gg,dd=s:-1;'