fix(node): Ensure httpIntegration propagates traces

mydea · chargome · commit ce13e76c673a · 2025-03-19T14:48:13.000+01:00
This is another stab at #15233, which was reverted here: #15354, with a different implementation. It is required to make #15730 work.
diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing-no-spans/scenario.ts
@@ -0,0 +1,61 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+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 as { path?: string };
+    breadcrumb.data.ADDED_PATH = req?.path;
+    return breadcrumb;
+  },
+});
+
+import * as http from 'http';
+
+async function run(): Promise<void> {
+  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'));
+}
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+run();
+
+function makeHttpRequest(url: string): Promise<void> {
+  return new Promise<void>(resolve => {
+    http
+      .request(url, httpRes => {
+        httpRes.on('data', () => {
+          // we don't care about data
+        });
+        httpRes.on('end', () => {
+          resolve();
+        });
+      })
+      .end();
+  });
+}
+
+function makeHttpGet(url: string): Promise<void> {
+  return new Promise<void>(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,95 @@
+import { createRunner } from '../../../../utils/runner';
+import { createTestServer } from '../../../../utils/server';
+
+test('outgoing http requests are correctly instrumented with tracing & spans disabled', done => {
+  expect.assertions(11);
+
+  createTestServer(done)
+    .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()
+    .then(([SERVER_URL, closeTestServer]) => {
+      createRunner(__dirname, 'scenario.ts')
+        .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(closeTestServer);
+    });
+});
diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts
@@ -5,19 +5,24 @@ import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import type { AggregationCounts, Client, RequestEventData, SanitizedRequestData, Scope } from '@sentry/core';
 import {
+  LRUMap,
   addBreadcrumb,
   generateSpanId,
   getBreadcrumbLogLevelFromHttpStatusCode,
   getClient,
   getCurrentScope,
   getIsolationScope,
   getSanitizedUrlString,
+  getTraceData,
   httpRequestToRequestData,
   logger,
+  objectToBaggageHeader,
+  parseBaggageHeader,
   parseUrl,
   stripUrlQueryAndFragment,
   withIsolationScope,
 } from '@sentry/core';
+import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry';
 import type * as http from 'node:http';
 import type { IncomingMessage, RequestOptions } from 'node:http';
 import type * as https from 'node:https';
@@ -30,6 +35,12 @@ import { getRequestInfo } from './vendor/getRequestInfo';
 type Http = typeof http;
 type Https = typeof https;
 
+type RequestArgs =
+  // eslint-disable-next-line @typescript-eslint/ban-types
+  | [url: string | URL, options?: RequestOptions, callback?: Function]
+  // eslint-disable-next-line @typescript-eslint/ban-types
+  | [options: RequestOptions, callback?: Function];
+
 export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
   /**
    * Whether breadcrumbs should be recorded for requests.
@@ -47,6 +58,15 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
    */
   extractIncomingTraceFromHeader?: boolean;
 
+  /**
+   * Whether to propagate Sentry trace headers in ougoing 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.
@@ -91,8 +111,11 @@ 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('@sentry/instrumentation-http', VERSION, config);
+    this._propagationDecisionMap = new LRUMap<string, boolean>(100);
   }
 
   /** @inheritdoc */
@@ -232,18 +255,25 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
         // We need to access and reconstruct the request options object passed to `ignoreOutgoingRequests`
         // so that it matches what Otel instrumentation passes to `ignoreOutgoingRequestHook`.
         // @see https://github.com/open-telemetry/opentelemetry-js/blob/7293e69c1e55ca62e15d0724d22605e61bd58952/experimental/packages/opentelemetry-instrumentation-http/src/http.ts#L756-L789
-        const argsCopy = [...args];
+        const argsCopy = [...args] as RequestArgs;
 
-        const options = argsCopy.shift() as URL | http.RequestOptions | string;
+        const options = argsCopy[0];
+        const extraOptions = typeof argsCopy[1] === 'object' ? argsCopy[1] : undefined;
 
-        const extraOptions =
-          typeof argsCopy[0] === 'object' && (typeof options === 'string' || options instanceof URL)
-            ? (argsCopy.shift() as http.RequestOptions)
-            : undefined;
+        const { optionsParsed, origin, pathname } = getRequestInfo(instrumentation._diag, options, extraOptions);
+        const url = getAbsoluteUrl(origin, pathname);
 
-        const { optionsParsed } = getRequestInfo(instrumentation._diag, options, extraOptions);
+        // This will be undefined if there are no changed headers
+        const mergedHeaders = instrumentation.getConfig().propagateTraceInOutgoingRequests
+          ? getMergedHeadersForRequestOptions(url, optionsParsed, instrumentation._propagationDecisionMap)
+          : undefined;
 
-        const request = original.apply(this, args) as ReturnType<typeof http.request>;
+          // If we are not proapgating traces, we skip touching the args for the request at all
+        const request = mergedHeaders
+          ? (original.apply(this, getOutgoingRequestArgsWithHeaders(argsCopy, mergedHeaders)) as ReturnType<
+              typeof http.request
+            >)
+          : (original.apply(this, args) as ReturnType<typeof http.request>);
 
         request.prependListener('response', (response: http.IncomingMessage) => {
           const _breadcrumbs = instrumentation.getConfig().breadcrumbs;
@@ -515,3 +545,118 @@ const clientToRequestSessionAggregatesMap = new Map<
   Client,
   { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } }
 >();
+
+/**
+ * If there are any headers to be added for this request, this will return the full merged headers object.
+ * Else, it will return void.
+ */
+function getMergedHeadersForRequestOptions(
+  url: string,
+  options: RequestOptions,
+  propagationDecisionMap: LRUMap<string, boolean>,
+): void | http.OutgoingHttpHeaders {
+  // 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, propagationDecisionMap)
+    ? getTraceData()
+    : undefined;
+
+  if (!addedHeaders) {
+    return;
+  }
+
+  const headers = options.headers || {};
+
+  const { 'sentry-trace': sentryTrace, baggage } = addedHeaders;
+
+  // We do not want to overwrite existing header here, if it was already set
+  if (sentryTrace && !headers['sentry-trace']) {
+    headers['sentry-trace'] = sentryTrace;
+  }
+
+  // For baggage, we make sure to merge this into a possibly existing header
+  if (baggage) {
+    headers['baggage'] = mergeBaggageHeaders(headers['baggage'], baggage);
+  }
+
+  return headers;
+}
+
+function getAbsoluteUrl(origin: string, path: string = '/'): string {
+  try {
+    const url = new URL(path, origin);
+    return url.toString();
+  } catch {
+    // fallback: Construct it on our own
+    const url = `${origin}`;
+
+    if (url.endsWith('/') && path.startsWith('/')) {
+      return `${url}${path.slice(1)}`;
+    }
+
+    if (!url.endsWith('/') && !path.startsWith('/')) {
+      return `${url}/${path.slice(1)}`;
+    }
+
+    return `${url}${path}`;
+  }
+}
+
+function mergeBaggageHeaders(
+  existing: string | string[] | number | undefined,
+  baggage: string,
+): string | string[] | number | undefined {
+  if (!existing) {
+    return baggage;
+  }
+
+  const existingBaggageEntries = parseBaggageHeader(existing);
+  const newBaggageEntries = parseBaggageHeader(baggage);
+
+  if (!newBaggageEntries) {
+    return existing;
+  }
+
+  // Existing entries take precedence, ensuring order remains stable for minimal changes
+  const mergedBaggageEntries = { ...existingBaggageEntries };
+  Object.entries(newBaggageEntries).forEach(([key, value]) => {
+    if (!mergedBaggageEntries[key]) {
+      mergedBaggageEntries[key] = value;
+    }
+  });
+
+  return objectToBaggageHeader(mergedBaggageEntries);
+}
+
+function getOutgoingRequestArgsWithHeaders(originalArgs: RequestArgs, headers: http.OutgoingHttpHeaders): RequestArgs {
+  const argsCopy = [...originalArgs] as RequestArgs;
+
+  const arg1 = argsCopy[0];
+
+  // If the first argument is a string or URL, we need to merge the headers into the options object, which is optional
+  if (typeof arg1 === 'string' || arg1 instanceof URL) {
+    const arg2 = argsCopy[1];
+
+    // If the second argument is an object, we just overwrite the headers there
+    if (typeof arg2 === 'object') {
+      argsCopy[1] = {
+        ...arg2,
+        headers,
+      };
+      return argsCopy;
+    }
+
+    // Else, we need to insert a new object as second argument and insert the headers there
+    argsCopy.splice(1, 0, { headers });
+    return argsCopy;
+  }
+
+  // If the first argument is an object, we just overwrite the headers there
+  argsCopy[0] = {
+    ...arg1,
+    headers,
+  };
+  return argsCopy;
+}
diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts
@@ -160,6 +160,9 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
         // If spans are not instrumented, it means the HttpInstrumentation has not been added
         // In that case, we want to handle incoming trace extraction ourselves
         extractIncomingTraceFromHeader: !instrumentSpans,
+        // If spans are not instrumented, it means the HttpInstrumentation has not been added
+        // In that case, we want to handle trace propagation ourselves
+        propagateTraceInOutgoingRequests: !instrumentSpans,
       });
     },
   };
