fix(node): Ensure traces are propagated without spans in Node 22+ (#16221)

mydea · AbhiPrasad · andreiborza · web-flow · commit 9d659a565788 · 2025-05-12T14:34:48.000+02:00
Today, if the `httpIntegration` is used without spans (e.g. in a custom OTEL setup or when setting `httpIntegration({ spans: false })` manually), outgoing requests will not have traces propagated. This PR fixes this by using the `http.client.request.created` diagnostics channel to add the trace headers in this scenario. However, sadly this channel was only added in Node 22, so it does not work on versions before that. I suppose this is still worth adding because it is better than what we have today (which is that it does not work at all). We may think about making this work for Node <22, but this would require us monkey patching http again, which we really do not want to do... Also note that as of now this should not really affect the vast majority of cases, as unless you specifically opt out of spans today this will always work as we always add the otel http instrumentation by default. And in custom otel setups, users will usually have this set up anyhow. (Also, fetch works in all environments as expected). If we want to, in a follow up, avoid adding the otel spans instrumentation if performance is disabled, we need to think about this... an option could be to always adding the instrumentation on node <22, and only skip it on Node 22+. But this can be looked at separately, this PR at least makes things better than before on Node 22+... This supersedes #15735 --------- Co-authored-by: Abhijeet Prasad <aprasad@sentry.io> Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/instrument.mjs
@@ -0,0 +1,17 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  integrations: [Sentry.httpIntegration({ spans: false })],
+  transport: loggingTransport,
+  // Ensure this gets a correct hint
+  beforeBreadcrumb(breadcrumb, hint) {
+    breadcrumb.data = breadcrumb.data || {};
+    const req = hint?.request;
+    breadcrumb.data.ADDED_PATH = req?.path;
+    return breadcrumb;
+  },
+});
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/scenario.mjs
@@ -0,0 +1,43 @@
+import * as Sentry from '@sentry/node';
+import * as http from 'http';
+
+async function run() {
+  Sentry.addBreadcrumb({ message: 'manual breadcrumb' });
+
+  await makeHttpRequest(`${process.env.SERVER_URL}/api/v0`);
+  await makeHttpGet(`${process.env.SERVER_URL}/api/v1`);
+  await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`);
+  await makeHttpRequest(`${process.env.SERVER_URL}/api/v3`);
+
+  Sentry.captureException(new Error('foo'));
+}
+
+run();
+
+function makeHttpRequest(url) {
+  return new Promise(resolve => {
+    http
+      .request(url, httpRes => {
+        httpRes.on('data', () => {
+          // we don't care about data
+        });
+        httpRes.on('end', () => {
+          resolve();
+        });
+      })
+      .end();
+  });
+}
+
+function makeHttpGet(url) {
+  return new Promise(resolve => {
+    http.get(url, httpRes => {
+      httpRes.on('data', () => {
+        // we don't care about data
+      });
+      httpRes.on('end', () => {
+        resolve();
+      });
+    });
+  });
+}
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/test.ts
@@ -0,0 +1,204 @@
+import { describe, expect } from 'vitest';
+import { conditionalTest } from '../../../../utils';
+import { createEsmAndCjsTests } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
+
+describe('outgoing http requests with tracing & spans disabled', () => {
+  createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
+    conditionalTest({ min: 22 })('node >=22', () => {
+      test('outgoing http requests are correctly instrumented with tracing & spans disabled', async () => {
+        expect.assertions(11);
+
+        const [SERVER_URL, closeTestServer] = await createTestServer()
+          .get('/api/v0', headers => {
+            expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
+            expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
+            expect(headers['baggage']).toEqual(expect.any(String));
+          })
+          .get('/api/v1', headers => {
+            expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
+            expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
+            expect(headers['baggage']).toEqual(expect.any(String));
+          })
+          .get('/api/v2', headers => {
+            expect(headers['baggage']).toBeUndefined();
+            expect(headers['sentry-trace']).toBeUndefined();
+          })
+          .get('/api/v3', headers => {
+            expect(headers['baggage']).toBeUndefined();
+            expect(headers['sentry-trace']).toBeUndefined();
+          })
+          .start();
+
+        await createRunner()
+          .withEnv({ SERVER_URL })
+          .ensureNoErrorOutput()
+          .expect({
+            event: {
+              exception: {
+                values: [
+                  {
+                    type: 'Error',
+                    value: 'foo',
+                  },
+                ],
+              },
+              breadcrumbs: [
+                {
+                  message: 'manual breadcrumb',
+                  timestamp: expect.any(Number),
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v0`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v0',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v1`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v1',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v2`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v2',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v3`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v3',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+              ],
+            },
+          })
+          .start()
+          .completed();
+
+        closeTestServer();
+      });
+    });
+
+    // On older node versions, outgoing requests do not get trace-headers injected, sadly
+    // This is because the necessary diagnostics channel hook is not available yet
+    conditionalTest({ max: 21 })('node <22', () => {
+      test('outgoing http requests generate breadcrumbs correctly with tracing & spans disabled', async () => {
+        expect.assertions(9);
+
+        const [SERVER_URL, closeTestServer] = await createTestServer()
+          .get('/api/v0', headers => {
+            // This is not instrumented, sadly
+            expect(headers['baggage']).toBeUndefined();
+            expect(headers['sentry-trace']).toBeUndefined();
+          })
+          .get('/api/v1', headers => {
+            // This is not instrumented, sadly
+            expect(headers['baggage']).toBeUndefined();
+            expect(headers['sentry-trace']).toBeUndefined();
+          })
+          .get('/api/v2', headers => {
+            expect(headers['baggage']).toBeUndefined();
+            expect(headers['sentry-trace']).toBeUndefined();
+          })
+          .get('/api/v3', headers => {
+            expect(headers['baggage']).toBeUndefined();
+            expect(headers['sentry-trace']).toBeUndefined();
+          })
+          .start();
+
+        await createRunner()
+          .withEnv({ SERVER_URL })
+          .ensureNoErrorOutput()
+          .expect({
+            event: {
+              exception: {
+                values: [
+                  {
+                    type: 'Error',
+                    value: 'foo',
+                  },
+                ],
+              },
+              breadcrumbs: [
+                {
+                  message: 'manual breadcrumb',
+                  timestamp: expect.any(Number),
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v0`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v0',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v1`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v1',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v2`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v2',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+                {
+                  category: 'http',
+                  data: {
+                    'http.method': 'GET',
+                    url: `${SERVER_URL}/api/v3`,
+                    status_code: 200,
+                    ADDED_PATH: '/api/v3',
+                  },
+                  timestamp: expect.any(Number),
+                  type: 'http',
+                },
+              ],
+            },
+          })
+          .start()
+          .completed();
+
+        closeTestServer();
+      });
+    });
+  });
+});
diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts
@@ -18,13 +18,17 @@ import {
   getCurrentScope,
   getIsolationScope,
   getSanitizedUrlString,
+  getTraceData,
   httpRequestToRequestData,
   logger,
+  LRUMap,
   parseUrl,
   stripUrlQueryAndFragment,
   withIsolationScope,
 } from '@sentry/core';
+import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry';
 import { DEBUG_BUILD } from '../../debug-build';
+import { mergeBaggageHeaders } from '../../utils/baggage';
 import { getRequestUrl } from '../../utils/getRequestUrl';
 
 const INSTRUMENTATION_NAME = '@sentry/instrumentation-http';
@@ -49,6 +53,15 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   extractIncomingTraceFromHeader?: boolean;
 
+  /**
+   * Whether to propagate Sentry trace headers in outgoing requests.
+   * By default this is done by the HttpInstrumentation, but if that is not added (e.g. because tracing is disabled)
+   * then this instrumentation can take over.
+   *
+   * @default `false`
+   */
+  propagateTraceInOutgoingRequests?: boolean;
+
   /**
    * Do not capture breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`.
    * For the scope of this instrumentation, this callback only controls breadcrumb creation.
@@ -102,8 +115,12 @@ const MAX_BODY_BYTE_LENGTH = 1024 * 1024;
  * https://github.com/open-telemetry/opentelemetry-js/blob/f8ab5592ddea5cba0a3b33bf8d74f27872c0367f/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
  */
 export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpInstrumentationOptions> {
+  private _propagationDecisionMap: LRUMap<string, boolean>;
+
   public constructor(config: SentryHttpInstrumentationOptions = {}) {
     super(INSTRUMENTATION_NAME, VERSION, config);
+
+    this._propagationDecisionMap = new LRUMap<string, boolean>(100);
   }
 
   /** @inheritdoc */
@@ -127,6 +144,11 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
       this._onOutgoingRequestFinish(data.request, undefined);
     }) satisfies ChannelListener;
 
+    const onHttpClientRequestCreated = ((_data: unknown) => {
+      const data = _data as { request: http.ClientRequest };
+      this._onOutgoingRequestCreated(data.request);
+    }) satisfies ChannelListener;
+
     /**
      * You may be wondering why we register these diagnostics-channel listeners
      * in such a convoluted way (as InstrumentationNodeModuleDefinition...)˝,
@@ -153,12 +175,20 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
           // In this case, `http.client.response.finish` is not triggered
           subscribe('http.client.request.error', onHttpClientRequestError);
 
+          // NOTE: This channel only exist since Node 23
+          // Before that, outgoing requests are not patched
+          // and trace headers are not propagated, sadly.
+          if (this.getConfig().propagateTraceInOutgoingRequests) {
+            subscribe('http.client.request.created', onHttpClientRequestCreated);
+          }
+
           return moduleExports;
         },
         () => {
           unsubscribe('http.server.request.start', onHttpServerRequestStart);
           unsubscribe('http.client.response.finish', onHttpClientResponseFinish);
           unsubscribe('http.client.request.error', onHttpClientRequestError);
+          unsubscribe('http.client.request.created', onHttpClientRequestCreated);
         },
       ),
       new InstrumentationNodeModuleDefinition(
@@ -209,6 +239,49 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
     }
   }
 
+  /**
+   * This is triggered when an outgoing request is created.
+   * It has access to the request object, and can mutate it before the request is sent.
+   */
+  private _onOutgoingRequestCreated(request: http.ClientRequest): void {
+    const url = getRequestUrl(request);
+    const ignoreOutgoingRequests = this.getConfig().ignoreOutgoingRequests;
+    const shouldPropagate =
+      typeof ignoreOutgoingRequests === 'function' ? !ignoreOutgoingRequests(url, getRequestOptions(request)) : true;
+
+    if (!shouldPropagate) {
+      return;
+    }
+
+    // Manually add the trace headers, if it applies
+    // Note: We do not use `propagation.inject()` here, because our propagator relies on an active span
+    // Which we do not have in this case
+    const tracePropagationTargets = getClient()?.getOptions().tracePropagationTargets;
+    const addedHeaders = shouldPropagateTraceForUrl(url, tracePropagationTargets, this._propagationDecisionMap)
+      ? getTraceData()
+      : undefined;
+
+    if (!addedHeaders) {
+      return;
+    }
+
+    const { 'sentry-trace': sentryTrace, baggage } = addedHeaders;
+
+    // We do not want to overwrite existing header here, if it was already set
+    if (sentryTrace && !request.getHeader('sentry-trace')) {
+      request.setHeader('sentry-trace', sentryTrace);
+      logger.log(INSTRUMENTATION_NAME, 'Added sentry-trace header to outgoing request');
+    }
+
+    if (baggage) {
+      // For baggage, we make sure to merge this into a possibly existing header
+      const newBaggage = mergeBaggageHeaders(request.getHeader('baggage'), baggage);
+      if (newBaggage) {
+        request.setHeader('baggage', newBaggage);
+      }
+    }
+  }
+
   /**
    * Patch a server.emit function to handle process isolation for incoming requests.
    * This will only patch the emit function if it was not already patched.
diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts
