fix(node): Avoid creating breadcrumbs for suppressed requests (#16285)

It was noticed that we were creating breadcrumbs for sentry-internal
requests, which is not desired. This was introduced by us moving
breadcrumb generation out of the base http/fetch instrumentation into
our own, where we did not look at `suppressTracing`. Now, when tracing
is suppressed, no fetch/http breadcrumbs will be created.

Author: mydea

dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario.mjs

@@ -8,6 +8,8 @@ async function run() {
   await fetch(`${process.env.SERVER_URL}/api/v2`).then(res => res.text());
   await fetch(`${process.env.SERVER_URL}/api/v3`).then(res => res.text());
 
+  await Sentry.suppressTracing(() => fetch(`${process.env.SERVER_URL}/api/v4`).then(res => res.text()));
+
   Sentry.captureException(new Error('foo'));
 }
 

dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts

@@ -4,7 +4,7 @@ import { createTestServer } from '../../../../utils/server';
 
 describe('outgoing fetch', () => {
   createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
-    test('outgoing fetch requests create breadcrumbs xxx', async () => {
+    test('outgoing fetch requests create breadcrumbs', async () => {
       const [SERVER_URL, closeTestServer] = await createTestServer().start();
 
       await createRunner()

dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/scenario.mjs

@@ -9,6 +9,8 @@ async function run() {
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v2`);
   await makeHttpRequest(`${process.env.SERVER_URL}/api/v3`);
 
+  await Sentry.suppressTracing(() => makeHttpRequest(`${process.env.SERVER_URL}/api/v4`));
+
   Sentry.captureException(new Error('foo'));
 }
 

packages/node/src/integrations/http/SentryHttpInstrumentation.ts

@@ -5,7 +5,7 @@ import type * as http from 'node:http';
 import type * as https from 'node:https';
 import type { EventEmitter } from 'node:stream';
 import { context, propagation } from '@opentelemetry/api';
-import { VERSION } from '@opentelemetry/core';
+import { isTracingSuppressed, VERSION } from '@opentelemetry/core';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import type { AggregationCounts, Client, SanitizedRequestData, Scope } from '@sentry/core';
@@ -132,11 +132,13 @@ const MAX_BODY_BYTE_LENGTH = 1024 * 1024;
  */
 export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpInstrumentationOptions> {
   private _propagationDecisionMap: LRUMap<string, boolean>;
+  private _ignoreOutgoingRequestsMap: WeakMap<http.ClientRequest, boolean>;
 
   public constructor(config: SentryHttpInstrumentationOptions = {}) {
     super(INSTRUMENTATION_NAME, VERSION, config);
 
     this._propagationDecisionMap = new LRUMap<string, boolean>(100);
+    this._ignoreOutgoingRequestsMap = new WeakMap<http.ClientRequest, boolean>();
   }
 
   /** @inheritdoc */
@@ -165,6 +167,37 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
       this._onOutgoingRequestCreated(data.request);
     }) satisfies ChannelListener;
 
+    const wrap = <T extends Http | Https>(moduleExports: T): T => {
+      if (hasRegisteredHandlers) {
+        return moduleExports;
+      }
+
+      hasRegisteredHandlers = true;
+
+      subscribe('http.server.request.start', onHttpServerRequestStart);
+      subscribe('http.client.response.finish', onHttpClientResponseFinish);
+
+      // When an error happens, we still want to have a breadcrumb
+      // In this case, `http.client.response.finish` is not triggered
+      subscribe('http.client.request.error', onHttpClientRequestError);
+
+      // NOTE: This channel only exist since Node 22
+      // 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;
+    };
+
+    const unwrap = (): void => {
+      unsubscribe('http.server.request.start', onHttpServerRequestStart);
+      unsubscribe('http.client.response.finish', onHttpClientResponseFinish);
+      unsubscribe('http.client.request.error', onHttpClientRequestError);
+      unsubscribe('http.client.request.created', onHttpClientRequestCreated);
+    };
+
     /**
      * You may be wondering why we register these diagnostics-channel listeners
      * in such a convoluted way (as InstrumentationNodeModuleDefinition...)˝,
@@ -174,64 +207,8 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
      * especially the "import-on-top" pattern of setting up ESM applications.
      */
     return [
-      new InstrumentationNodeModuleDefinition(
-        'http',
-        ['*'],
-        (moduleExports: Http): Http => {
-          if (hasRegisteredHandlers) {
-            return moduleExports;
-          }
-
-          hasRegisteredHandlers = true;
-
-          subscribe('http.server.request.start', onHttpServerRequestStart);
-          subscribe('http.client.response.finish', onHttpClientResponseFinish);
-
-          // When an error happens, we still want to have a breadcrumb
-          // 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(
-        'https',
-        ['*'],
-        (moduleExports: Https): Https => {
-          if (hasRegisteredHandlers) {
-            return moduleExports;
-          }
-
-          hasRegisteredHandlers = true;
-
-          subscribe('http.server.request.start', onHttpServerRequestStart);
-          subscribe('http.client.response.finish', onHttpClientResponseFinish);
-
-          // When an error happens, we still want to have a breadcrumb
-          // In this case, `http.client.response.finish` is not triggered
-          subscribe('http.client.request.error', onHttpClientRequestError);
-
-          return moduleExports;
-        },
-        () => {
-          unsubscribe('http.server.request.start', onHttpServerRequestStart);
-          unsubscribe('http.client.response.finish', onHttpClientResponseFinish);
-          unsubscribe('http.client.request.error', onHttpClientRequestError);
-        },
-      ),
+      new InstrumentationNodeModuleDefinition('http', ['*'], wrap, unwrap),
+      new InstrumentationNodeModuleDefinition('https', ['*'], wrap, unwrap),
     ];
   }
 
@@ -244,13 +221,12 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
 
     const _breadcrumbs = this.getConfig().breadcrumbs;
     const breadCrumbsEnabled = typeof _breadcrumbs === 'undefined' ? true : _breadcrumbs;
-    const options = getRequestOptions(request);
 
-    const _ignoreOutgoingRequests = this.getConfig().ignoreOutgoingRequests;
-    const shouldCreateBreadcrumb =
-      typeof _ignoreOutgoingRequests === 'function' ? !_ignoreOutgoingRequests(getRequestUrl(request), options) : true;
+    // Note: We cannot rely on the map being set by `_onOutgoingRequestCreated`, because that is not run in Node <22
+    const shouldIgnore = this._ignoreOutgoingRequestsMap.get(request) ?? this._shouldIgnoreOutgoingRequest(request);
+    this._ignoreOutgoingRequestsMap.set(request, shouldIgnore);
 
-    if (breadCrumbsEnabled && shouldCreateBreadcrumb) {
+    if (breadCrumbsEnabled && !shouldIgnore) {
       addRequestBreadcrumb(request, response);
     }
   }
@@ -260,15 +236,16 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
    * 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;
+    const shouldIgnore = this._ignoreOutgoingRequestsMap.get(request) ?? this._shouldIgnoreOutgoingRequest(request);
+    this._ignoreOutgoingRequestsMap.set(request, shouldIgnore);
 
-    if (!shouldPropagate) {
+    if (shouldIgnore) {
       return;
     }
 
+    // Add trace propagation headers
+    const url = getRequestUrl(request);
+
     // 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
@@ -384,6 +361,25 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
 
     server.emit = newEmit;
   }
+
+  /**
+   * Check if the given outgoing request should be ignored.
+   */
+  private _shouldIgnoreOutgoingRequest(request: http.ClientRequest): boolean {
+    if (isTracingSuppressed(context.active())) {
+      return true;
+    }
+
+    const ignoreOutgoingRequests = this.getConfig().ignoreOutgoingRequests;
+
+    if (!ignoreOutgoingRequests) {
+      return false;
+    }
+
+    const options = getRequestOptions(request);
+    const url = getRequestUrl(request);
+    return ignoreOutgoingRequests(url, options);
+  }
 }
 
 /** Add a breadcrumb for outgoing requests. */

packages/node/src/integrations/node-fetch/SentryNodeFetchInstrumentation.ts

@@ -1,4 +1,5 @@
-import { VERSION } from '@opentelemetry/core';
+import { context } from '@opentelemetry/api';
+import { isTracingSuppressed, VERSION } from '@opentelemetry/core';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase } from '@opentelemetry/instrumentation';
 import type { SanitizedRequestData } from '@sentry/core';
@@ -61,11 +62,13 @@ export class SentryNodeFetchInstrumentation extends InstrumentationBase<SentryNo
   // unsubscribing.
   private _channelSubs: Array<ListenerRecord>;
   private _propagationDecisionMap: LRUMap<string, boolean>;
+  private _ignoreOutgoingRequestsMap: WeakMap<UndiciRequest, boolean>;
 
   public constructor(config: SentryNodeFetchInstrumentationOptions = {}) {
     super('@sentry/instrumentation-node-fetch', VERSION, config);
     this._channelSubs = [];
     this._propagationDecisionMap = new LRUMap<string, boolean>(100);
+    this._ignoreOutgoingRequestsMap = new WeakMap<UndiciRequest, boolean>();
   }
 
   /** No need to instrument files/modules. */
@@ -118,15 +121,17 @@ export class SentryNodeFetchInstrumentation extends InstrumentationBase<SentryNo
       return;
     }
 
-    // Add trace propagation headers
-    const url = getAbsoluteUrl(request.origin, request.path);
-    const _ignoreOutgoingRequests = config.ignoreOutgoingRequests;
-    const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url);
+    const shouldIgnore = this._shouldIgnoreOutgoingRequest(request);
+    // We store this decisision for later so we do not need to re-evaluate it
+    // Additionally, the active context is not correct in _onResponseHeaders, so we need to make sure it is evaluated here
+    this._ignoreOutgoingRequestsMap.set(request, shouldIgnore);
 
     if (shouldIgnore) {
       return;
     }
 
+    const url = getAbsoluteUrl(request.origin, request.path);
+
     // 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
@@ -197,13 +202,9 @@ export class SentryNodeFetchInstrumentation extends InstrumentationBase<SentryNo
     const _breadcrumbs = config.breadcrumbs;
     const breadCrumbsEnabled = typeof _breadcrumbs === 'undefined' ? true : _breadcrumbs;
 
-    const _ignoreOutgoingRequests = config.ignoreOutgoingRequests;
-    const shouldCreateBreadcrumb =
-      typeof _ignoreOutgoingRequests === 'function'
-        ? !_ignoreOutgoingRequests(getAbsoluteUrl(request.origin, request.path))
-        : true;
+    const shouldIgnore = this._ignoreOutgoingRequestsMap.get(request);
 
-    if (breadCrumbsEnabled && shouldCreateBreadcrumb) {
+    if (breadCrumbsEnabled && !shouldIgnore) {
       addRequestBreadcrumb(request, response);
     }
   }
@@ -232,6 +233,25 @@ export class SentryNodeFetchInstrumentation extends InstrumentationBase<SentryNo
       unsubscribe,
     });
   }
+
+  /**
+   * Check if the given outgoing request should be ignored.
+   */
+  private _shouldIgnoreOutgoingRequest(request: UndiciRequest): boolean {
+    if (isTracingSuppressed(context.active())) {
+      return true;
+    }
+
+    // Add trace propagation headers
+    const url = getAbsoluteUrl(request.origin, request.path);
+    const ignoreOutgoingRequests = this.getConfig().ignoreOutgoingRequests;
+
+    if (typeof ignoreOutgoingRequests !== 'function' || !url) {
+      return false;
+    }
+
+    return ignoreOutgoingRequests(url);
+  }
 }
 
 /** Add a breadcrumb for outgoing requests. */