fix(undici): avoid possible duplicate 'traceparent' header on instrumented HTTP requests (#3965)

... because Elasticsearch borks on these requests. This case should only be
possible in a weird case like having two APM agents active.

Fixes: #3964

Author: trentm

CHANGELOG.asciidoc

@@ -50,6 +50,9 @@ See the <<upgrade-to-v4>> guide.
 * Fix undici instrumentation to cope with a bug in [email protected] where
   `request.addHeader()` was accidentally removed. (It was re-added in
   [email protected].)
+* Update undici instrumentation to avoid possibly adding a *second*
+  'traceparent' header to outgoing HTTP requests, because this can break
+  Elasticsearch requests. ({issues}3964[#3964])
 
 [float]
 ===== Chores

lib/instrumentation/modules/undici.js

@@ -44,6 +44,9 @@ try {
 
 const semver = require('semver');
 
+// Search an undici@5 request.headers string for a 'traceparent' header.
+const headersStrHasTraceparentRe = /^traceparent:/im;
+
 let isInstrumented = false;
 let spanFromReq = null;
 let chans = null;
@@ -130,17 +133,37 @@ function instrumentUndici(agent) {
     const propSpan =
       span || parentRunContext.currSpan() || parentRunContext.currTransaction();
     if (propSpan) {
-      propSpan.propagateTraceContextHeaders(
-        request,
-        function (req, name, value) {
-          if (typeof request.addHeader === 'function') {
-            req.addHeader(name, value);
-          } else if (Array.isArray(request.headers)) {
-            // [email protected] accidentally, briefly removed `request.addHeader()`.
-            req.headers.push(name, value);
+      // Guard against adding a duplicate 'traceparent' header, because that
+      // breaks ES. https://github.com/elastic/apm-agent-nodejs/issues/3964
+      // Dev Note: This cheats a little and assumes the header names to add
+      // will include 'traceparent'.
+      let alreadyHasTp = false;
+      if (Array.isArray(request.headers)) {
+        // undici@6
+        for (let i = 0; i < request.headers.length; i += 2) {
+          if (request.headers[i].toLowerCase() === 'traceparent') {
+            alreadyHasTp = true;
+            break;
           }
-        },
-      );
+        }
+      } else if (typeof request.headers === 'string') {
+        // undici@5
+        alreadyHasTp = headersStrHasTraceparentRe.test(request.headers);
+      }
+
+      if (!alreadyHasTp) {
+        propSpan.propagateTraceContextHeaders(
+          request,
+          function (req, name, value) {
+            if (typeof request.addHeader === 'function') {
+              req.addHeader(name, value);
+            } else if (Array.isArray(request.headers)) {
+              // [email protected] accidentally, briefly removed `request.addHeader()`.
+              req.headers.push(name, value);
+            }
+          },
+        );
+      }
     }
 
     if (span) {

test/instrumentation/modules/undici/undici.test.js

@@ -274,6 +274,45 @@ if (global.AbortController) {
   });
 }
 
+test('undici no duplicate headers', async (t) => {
+  apm._apmClient.clear();
+  const aTrans = apm.startTransaction('aTransName');
+
+  const url = origin + '/ping';
+  const { statusCode, body } = await undici.request(url, {
+    headers: {
+      traceparent: 'this-value-is-not-from-elastic-apm-node',
+    },
+  });
+  t.equal(statusCode, 200, 'statusCode');
+  await consumeResponseBody(body);
+
+  aTrans.end();
+  t.error(await promisyApmFlush(), 'no apm.flush() error');
+
+  t.equal(apm._apmClient.spans.length, 1);
+  const span = apm._apmClient.spans[0];
+  assertUndiciSpan(t, span, url);
+
+  // If there is already a 'traceparent' header in the way, the APM agent
+  // should at least *not* result in duplicate headers, even if that means
+  // breaking distributed tracing.
+  let numTraceparentHeaders = 0;
+  let numTracestateHeaders = 0;
+  for (let i = 0; i < lastServerReq.rawHeaders.length; i += 2) {
+    const k = lastServerReq.rawHeaders[i].toLowerCase();
+    if (k === 'traceparent') {
+      numTraceparentHeaders++;
+    } else if (k === 'tracestate') {
+      numTracestateHeaders++;
+    }
+  }
+  t.equal(numTraceparentHeaders, 1, 'just one traceparent header');
+  t.equal(numTracestateHeaders, 0, 'tracestate header was skipped');
+
+  t.end();
+});
+
 test('teardown', (t) => {
   undici.getGlobalDispatcher().close(); // Close kept-alive sockets.
   server.close();