fix: Fixed undici/fetch instrumentation to properly assign the parent-id portion of the traceparent header on outgoing requests to the active http external span id (#2951)

bizob2828 · web-flow · commit bc714cfa8fe6 · 2025-02-13T16:04:58.000-05:00
diff --git a/lib/instrumentation/undici.js b/lib/instrumentation/undici.js
@@ -136,8 +136,8 @@ function requestCreateHook(shim, { request }) {
   }
 
   try {
-    addDTHeaders({ transaction, config, request })
     createExternalSegment({ shim, request, segment })
+    addDTHeaders({ transaction, config, request })
   } catch (err) {
     logger.warn(err, 'Unable to create external segment')
   }
diff --git a/test/integration/instrumentation/fetch.test.js b/test/integration/instrumentation/fetch.test.js
@@ -27,6 +27,13 @@ function createServer() {
       const status = parts[parts.length - 1]
       res.writeHead(status)
       res.end()
+    } else if (req.url.includes('/headers')) {
+      const data = JSON.stringify(req.headers)
+      res.writeHead(200, {
+        'Content-Length': data.length,
+        'Content-Type': 'application/json'
+      })
+      res.end(data)
     } else {
       res.writeHead(200)
       res.end('ok')
@@ -102,6 +109,21 @@ test('fetch', async function (t) {
     })
   })
 
+  await t.test('should add proper traceparent to outgoing headers', async () => {
+    await helper.runInTransaction(agent, async (tx) => {
+      const body = await fetch(`${REQUEST_URL}/headers`)
+      assert.equal(body.status, 200)
+      const segment = metrics.findSegment(tx.trace, tx.trace.root, `External/${HOST}/headers`)
+      const { traceparent } = await body.json()
+      const [version, traceId, parentSpan, sampledFlag] = traceparent.split('-')
+      assert.equal(version, '00')
+      assert.equal(traceId, tx.traceId)
+      assert.equal(parentSpan, segment.id)
+      assert.equal(sampledFlag, '01')
+      tx.end()
+    })
+  })
+
   await t.test('should add unscoped metrics for an external request', async () => {
     await helper.runInTransaction(agent, async (tx) => {
       const { status } = await fetch(`${REQUEST_URL}/get?a=b&c=d`)
diff --git a/test/unit/instrumentation/http/outbound.test.js b/test/unit/instrumentation/http/outbound.test.js
@@ -565,9 +565,15 @@ test('when working with http.request', async (t) => {
     nock(host)
       .get(path)
       .reply(200, function () {
+        const { transaction, segment } = agent.tracer.getContext()
+        assert.equal(segment.name, 'External/www.google.com')
         headers = this.req.headers
         assert.ok(headers.traceparent, 'traceparent header')
-        assert.equal(headers.traceparent.split('-').length, 4)
+        const [version, traceId, parentSpanId, sampledFlag] = headers.traceparent.split('-')
+        assert.equal(version, '00')
+        assert.equal(traceId, transaction.traceId)
+        assert.equal(parentSpanId, segment.id)
+        assert.equal(sampledFlag, '01')
         assert.ok(headers.tracestate, 'tracestate header')
         assert.ok(!headers.tracestate.includes('null'))
         assert.ok(!headers.tracestate.includes('true'))
diff --git a/test/unit/shim/message-shim.test.js b/test/unit/shim/message-shim.test.js
@@ -11,6 +11,7 @@ const API = require('../../../api')
 const DESTINATIONS = require('../../../lib/config/attribute-filter').DESTINATIONS
 const hashes = require('../../../lib/util/hashes')
 const helper = require('../../lib/agent_helper')
+const { findSegment } = require('#testlib/metrics_helper.js')
 const MessageShim = require('../../../lib/shim/message-shim')
 const { MessageSpec, MessageSubscribeSpec } = require('../../../lib/shim/specs')
 const {
@@ -369,6 +370,27 @@ test('MessageShim', async function (t) {
       })
     })
 
+    await t.test('should insert DT request headers', function (t, end) {
+      const { agent, shim, wrappable } = t.nr
+      agent.config.cross_application_tracer.enabled = false
+      agent.config.distributed_tracing.enabled = true
+      const headers = {}
+      shim.recordProduce(wrappable, 'getActiveSegment', function () {
+        return new MessageSpec({ headers })
+      })
+
+      helper.runInTransaction(agent, function (tx) {
+        wrappable.getActiveSegment()
+        const segment = findSegment(tx.trace, tx.trace.root, 'MessageBroker/RabbitMQ/Exchange/Produce/Temp')
+        const [version, traceId, parentSpanId, sampledFlag] = headers.traceparent.split('-')
+        assert.equal(version, '00')
+        assert.equal(traceId, tx.traceId)
+        assert.equal(parentSpanId, segment.id)
+        assert.equal(sampledFlag, '01')
+        end()
+      })
+    })
+
     await t.test('should insert distributed trace headers in all messages', async function (t) {
       const plan = tspl(t, { plan: 1 })
       const { agent, shim, wrappable } = t.nr
@@ -393,26 +415,27 @@ test('MessageShim', async function (t) {
       )
 
       let called = 0
-      agent.on('transactionFinished', () => {
+      agent.on('transactionFinished', (tx) => {
         called++
+        const segment = findSegment(tx.trace, tx.trace.root, 'MessageBroker/RabbitMQ/Exchange/Produce/Temp')
         match(messages, [
           {
             headers: {
               newrelic: '',
-              traceparent: /^00-/
+              traceparent: `00-${tx.traceId}-${segment.id}-01`
             }
           },
           {
             headers: {
               newrelic: '',
-              traceparent: /^00-/,
+              traceparent: `00-${tx.traceId}-${segment.id}-01`,
               foo: 'foo'
             }
           },
           {
             headers: {
               newrelic: '',
-              traceparent: /^00-/
+              traceparent: `00-${tx.traceId}-${segment.id}-01`
             }
           }
         ])
diff --git a/test/unit/shim/transaction-shim.test.js b/test/unit/shim/transaction-shim.test.js
@@ -662,7 +662,7 @@ test('TransactionShim', async function (t) {
           const outboundHeaders = {}
           tx.insertDistributedTraceHeaders(outboundHeaders)
 
-          assert.ok(outboundHeaders.traceparent.startsWith('00-4bf92f3577b3'))
+          assert.equal(outboundHeaders.traceparent, `00-${tx.traceId}-${segment.id}-01`)
           assert.ok(outboundHeaders.tracestate.endsWith(tracestate))
           end()
         })
diff --git a/test/versioned/undici/requests.test.js b/test/versioned/undici/requests.test.js
@@ -33,6 +33,13 @@ function createServer() {
       const statusCode = parts[parts.length - 1]
       res.writeHead(statusCode)
       res.end()
+    } else if (req.url.includes('/headers')) {
+      const data = JSON.stringify(req.headers)
+      res.writeHead(200, {
+        'Content-Length': data.length,
+        'Content-Type': 'application/json'
+      })
+      res.end(data)
     } else {
       res.writeHead(200)
       res.end('ok')
@@ -145,6 +152,24 @@ test('Undici request tests', async (t) => {
     })
   })
 
+  await t.test('should add proper traceparent to outgoing headers', async () => {
+    await helper.runInTransaction(agent, async (tx) => {
+      const { statusCode, body } = await undici.request(REQUEST_URL, {
+        path: '/headers',
+        method: 'GET'
+      })
+      assert.equal(statusCode, 200)
+      const segment = metrics.findSegment(tx.trace, tx.trace.root, `External/${HOST}/headers`)
+      const { traceparent } = await body.json()
+      const [version, traceId, parentSpan, sampledFlag] = traceparent.split('-')
+      assert.equal(version, '00')
+      assert.equal(traceId, tx.traceId)
+      assert.equal(parentSpan, segment.id)
+      assert.equal(sampledFlag, '01')
+      tx.end()
+    })
+  })
+
   await t.test('should add unscoped metrics for an external request', async () => {
     // make sure metric aggregator is empty before asserting metrics
     agent.metrics.clear()

Original file line number	Diff line number	Diff line change
`@@ -136,8 +136,8 @@ function requestCreateHook(shim, { request }) {`
`136`	`136`	`}`
`137`	`137`
`138`	`138`	`try {`
`139`		`- addDTHeaders({ transaction, config, request })`
`140`	`139`	`createExternalSegment({ shim, request, segment })`
	`140`	`+ addDTHeaders({ transaction, config, request })`
`141`	`141`	`} catch (err) {`
`142`	`142`	`logger.warn(err, 'Unable to create external segment')`
`143`	`143`	`}`