From d43ca81e424e9cc99919861e1fff51aef63c044c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 1 Jul 2026 10:10:47 +0200 Subject: [PATCH] ref(node): Fold breadcrumb & trace propagation logic into node fetch instrumentation Enhance the node `undici-instrumentation` so it is a single instrumentation that emits `http.client` spans (gated by a `spans` option, default `true`), records breadcrumbs, and propagates traces for outgoing fetch/undici requests. This replaces the previous setup where span creation (`instrumentUndici`) and breadcrumb/trace-propagation (`SentryNodeFetchInstrumentation`) ran as two separate subscribers on the same diagnostics channels. Trace propagation now goes through the dedup-aware `addTracePropagationHeadersToFetchRequest`, which gained an optional `span` argument so the outgoing headers reference the `http.client` span while still de-duplicating against any trace headers the user set manually (e.g. via `getTraceData()`). This keeps the change contained to the `node` package to ease review; a follow-up moves the instrumentation into `node-core`. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/node-core/src/index.ts | 1 + .../src/utils/outgoingFetchRequest.ts | 24 ++++- .../node/src/integrations/node-fetch/index.ts | 34 ++----- .../node/src/integrations/node-fetch/types.ts | 3 +- .../node-fetch/undici-instrumentation.ts | 91 +++++++++---------- 5 files changed, 71 insertions(+), 82 deletions(-) diff --git a/packages/node-core/src/index.ts b/packages/node-core/src/index.ts index 456343ac4406..612b7d45be9f 100644 --- a/packages/node-core/src/index.ts +++ b/packages/node-core/src/index.ts @@ -15,6 +15,7 @@ export { SentryNodeFetchInstrumentation, type SentryNodeFetchInstrumentationOptions, } from './integrations/node-fetch/SentryNodeFetchInstrumentation'; +export { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from './utils/outgoingFetchRequest'; export { SentryContextManager } from './otel/contextManager'; export { setupOpenTelemetryLogger } from './otel/logger'; diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index ea5a67ab1fde..76156bfe0f29 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -1,4 +1,4 @@ -import type { LRUMap, SanitizedRequestData } from '@sentry/core'; +import type { LRUMap, SanitizedRequestData, Span } from '@sentry/core'; import { addBreadcrumb, getBreadcrumbLogLevelFromHttpStatusCode, @@ -8,6 +8,7 @@ import { parseUrl, shouldPropagateTraceForUrl, mergeBaggageHeaders, + withActiveSpan, } from '@sentry/core'; import type { UndiciRequest, UndiciResponse } from '../integrations/node-fetch/types'; import { debug } from '@sentry/core'; @@ -19,11 +20,18 @@ const W3C_TRACEPARENT_HEADER = 'traceparent'; * * Checks if the request URL matches trace propagation targets, * then injects sentry-trace, traceparent, and baggage headers. + * + * When a `span` is passed (the outgoing `http.client` span), its trace data is propagated so downstream + * services are parented to that span. Without a span, the active scope's trace data is used. + * + * Existing trace headers (e.g. set manually by the user via `getTraceData()`) always take precedence and + * are de-duplicated rather than overwritten, so we never emit two `sentry-trace`/`baggage` entries. */ // eslint-disable-next-line complexity export function addTracePropagationHeadersToFetchRequest( request: UndiciRequest, propagationDecisionMap: LRUMap, + span?: Span, ): void { const url = getAbsoluteUrl(request.origin, request.path); @@ -32,9 +40,17 @@ export function addTracePropagationHeadersToFetchRequest( // Which we do not have in this case // The propagator _may_ overwrite this, but this should be fine as it is the same data const { tracePropagationTargets, propagateTraceparent } = getClient()?.getOptions() || {}; - const addedHeaders = shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap) - ? getTraceData({ propagateTraceparent }) - : undefined; + + if (!shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap)) { + return; + } + + // When a span is provided, make it active so the propagated headers reference it (and not the parent + // span). Passing `{ span }` to `getTraceData()` is not enough: for an inactive span it resolves to the + // span's captured scope, whose active span is still the parent. + const addedHeaders = span + ? withActiveSpan(span, () => getTraceData({ propagateTraceparent })) + : getTraceData({ propagateTraceparent }); if (!addedHeaders) { return; diff --git a/packages/node/src/integrations/node-fetch/index.ts b/packages/node/src/integrations/node-fetch/index.ts index d4fce2f39b47..347b75e828e4 100644 --- a/packages/node/src/integrations/node-fetch/index.ts +++ b/packages/node/src/integrations/node-fetch/index.ts @@ -1,41 +1,19 @@ -import { instrumentUndici } from './undici-instrumentation'; -import type { NodeFetchOptions } from './types'; import type { IntegrationFn } from '@sentry/core'; import { defineIntegration, getClient, hasSpansEnabled } from '@sentry/core'; import type { NodeClient } from '@sentry/node-core'; -import { generateInstrumentOnce, SentryNodeFetchInstrumentation } from '@sentry/node-core'; import type { NodeClientOptions } from '../../types'; - -const INTEGRATION_NAME = 'NodeFetch'; - -const instrumentSentryNodeFetch = generateInstrumentOnce( - `${INTEGRATION_NAME}.sentry`, - SentryNodeFetchInstrumentation, - (options: NodeFetchOptions) => { - return options; - }, -); +import type { NodeFetchOptions } from './types'; +import { instrumentUndici } from './undici-instrumentation'; const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { return { name: 'NodeFetch' as const, setupOnce() { - const instrumentSpans = _shouldInstrumentSpans(options, getClient()?.getOptions()); - - // This is the instrumentation that emits spans & propagates traces for outgoing fetch requests - if (instrumentSpans) { - instrumentUndici({ - ignoreOutgoingRequests: options.ignoreOutgoingRequests, - requestHook: options.requestHook, - responseHook: options.responseHook, - headersToSpanAttributes: options.headersToSpanAttributes, - }); - } + const spans = _shouldInstrumentSpans(options, getClient()?.getOptions()); - // This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces. - // It must subscribe to the diagnostics channels after the span instrumentation above, so the core - // trace propagation logic takes precedence. Otherwise, the sentry-trace header may be set multiple times. - instrumentSentryNodeFetch(options); + // This single instrumentation emits spans (when `spans` is enabled), records breadcrumbs, and + // propagates traces for outgoing fetch/undici requests. + instrumentUndici({ ...options, spans }); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/node-fetch/types.ts b/packages/node/src/integrations/node-fetch/types.ts index 8de209e42e1f..b79e5a5fc3cf 100644 --- a/packages/node/src/integrations/node-fetch/types.ts +++ b/packages/node/src/integrations/node-fetch/types.ts @@ -104,8 +104,7 @@ export interface NodeFetchOptions extends UndiciInstrumentationConfig { /** * If set to false, do not emit any spans. - * This will ensure that the default UndiciInstrumentation from OpenTelemetry is not setup, - * only the Sentry-specific instrumentation for breadcrumbs & trace propagation is applied. + * Breadcrumbs and trace propagation for outgoing fetch requests are still applied. * * If `skipOpenTelemetrySetup: true` is configured, this defaults to `false`, otherwise it defaults to `true`. */ diff --git a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 5ca98ec17e64..e2c1126d64b5 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -22,18 +22,16 @@ import { URL } from 'url'; import type { Span, SpanAttributes } from '@sentry/core'; import { debug, - getClient, - getTraceData, + isTracingSuppressed, LRUMap, SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - shouldPropagateTraceForUrl, SPAN_KIND, SPAN_STATUS_ERROR, startInactiveSpan, stripDataUrlContent, - withActiveSpan, } from '@sentry/core'; +import { addFetchRequestBreadcrumb, addTracePropagationHeadersToFetchRequest } from '@sentry/node-core'; import { HTTP_REQUEST_METHOD, HTTP_RESPONSE_STATUS_CODE, @@ -49,7 +47,7 @@ import { } from '@sentry/conventions/attributes'; import { DEBUG_BUILD } from '../../debug-build'; import type { - UndiciInstrumentationConfig, + NodeFetchOptions, UndiciRequest, RequestErrorMessage, RequestHeadersMessage, @@ -65,12 +63,19 @@ const ATTR_HTTP_REQUEST_METHOD_ORIGINAL = 'http.request.method_original'; // We can replace this with _isInstrumented once we drop support for Node.js 18.18.0 const _channelSubs: Array = []; const spanFromReq = new WeakMap(); +// Whether breadcrumbs (and span-less trace propagation) should be skipped for a given request. +// We evaluate this at request-creation time because the active context is no longer correct by the +// time the response-headers event fires. +const ignoreRequestMap = new WeakMap(); // Caches trace-propagation decisions per URL so we don't recompute the `tracePropagationTargets` regexes per request. const propagationDecisionMap = new LRUMap(100); /** * Instrument outgoing HTTP requests made through `undici` or the global `fetch` API: emit `http.client` - * spans and propagate traces into the outgoing request headers. + * spans, record breadcrumbs, and propagate traces into the outgoing request headers. + * + * Span creation is gated by the `spans` option (defaults to `true`). When spans are disabled, breadcrumbs + * are still recorded and trace propagation headers are still injected (gated by `tracePropagation`). * * undici reports its request lifecycle via `diagnostics_channel`, so rather than patching any module we * subscribe to those channels directly. This is idempotent — subsequent calls are no-ops once the @@ -79,7 +84,7 @@ const propagationDecisionMap = new LRUMap(100); * A combination of https://github.com/elastic/apm-agent-nodejs and * https://github.com/gadget-inc/opentelemetry-instrumentations/blob/main/packages/opentelemetry-instrumentation-undici/src/index.ts */ -export function instrumentUndici(config: UndiciInstrumentationConfig = {}): void { +export function instrumentUndici(config: NodeFetchOptions = {}): void { // Avoid duplicate subscriptions if (_channelSubs.length) { return; @@ -170,18 +175,32 @@ function parseRequestHeaders(request: UndiciRequest): Map request.method === 'CONNECT' || !!config.ignoreOutgoingRequests?.(url), + const ignoredByCallback = safeExecute( + () => !!config.ignoreOutgoingRequests?.(url), e => e && DEBUG_BUILD && debug.error('caught ignoreOutgoingRequests error: ', e), ); - if (shouldIgnoreReq) { + // Breadcrumbs & span-less trace propagation are additionally skipped when tracing is suppressed. + // We store this here because the active context is no longer correct in later channel events. + const ignoreForBreadcrumbs = isTracingSuppressed() || !!ignoredByCallback; + ignoreRequestMap.set(request, ignoreForBreadcrumbs); + + // When spans are disabled we do not create a span, but we still inject trace propagation headers + // directly (unless disabled via `tracePropagation`). + if (config.spans === false) { + if (config.tracePropagation !== false && !ignoreForBreadcrumbs) { + addTracePropagationHeadersToFetchRequest(request, propagationDecisionMap); + } + return; + } + + // Ignore span creation if: + // - the outgoing request is ignored via config + // - method is 'CONNECT' + if (request.method === 'CONNECT' || ignoredByCallback) { return; } @@ -249,7 +268,8 @@ function onRequestCreated(config: UndiciInstrumentationConfig, { request }: Requ // Context propagation goes last so no hook can tamper the propagation headers. // We propagate the trace data of the freshly created client span (not the active parent span) // so downstream services are parented to the http.client span, matching the upstream behavior. - injectTracePropagationHeaders(span, request, requestUrl.toString()); + // This also de-duplicates against any trace headers the user set manually (e.g. via `getTraceData()`). + addTracePropagationHeadersToFetchRequest(request, propagationDecisionMap, span); spanFromReq.set(request, span); } @@ -257,7 +277,7 @@ function onRequestCreated(config: UndiciInstrumentationConfig, { request }: Requ // This is the 2nd message we receive for each request. It is fired when connection with // the remote is established and about to send the first byte. Here we do have info about the // remote address and port so we can populate some `network.*` attributes into the span -function onRequestHeaders(config: UndiciInstrumentationConfig, { request, socket }: RequestHeadersMessage): void { +function onRequestHeaders(config: NodeFetchOptions, { request, socket }: RequestHeadersMessage): void { const span = spanFromReq.get(request); if (!span) { @@ -290,7 +310,13 @@ function onRequestHeaders(config: UndiciInstrumentationConfig, { request, socket // This is the 3rd message we get for each request and it's fired when the server // headers are received, body may not be accessible yet. // From the response headers we can set the status and content length -function onResponseHeaders(config: UndiciInstrumentationConfig, { request, response }: ResponseHeadersMessage): void { +function onResponseHeaders(config: NodeFetchOptions, { request, response }: ResponseHeadersMessage): void { + // Breadcrumbs are recorded independently of spans, so this runs even when span creation is disabled. + const breadcrumbsEnabled = config.breadcrumbs !== false; + if (breadcrumbsEnabled && !ignoreRequestMap.get(request)) { + addFetchRequestBreadcrumb(request, response); + } + const span = spanFromReq.get(request); if (!span) { @@ -379,37 +405,6 @@ function onError({ request, error }: RequestErrorMessage): void { spanFromReq.delete(request); } -// Propagate the trace data of the given (client) span into the outgoing request headers, gated by -// `tracePropagationTargets`. Mirrors what `propagation.inject()` did with the SentryPropagator, but -// via Sentry's `getTraceData()` so we stay off OpenTelemetry's propagation API. -function injectTracePropagationHeaders(span: Span, request: UndiciRequest, url: string): void { - const { tracePropagationTargets, propagateTraceparent } = getClient()?.getOptions() ?? {}; - - if (!shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap)) { - return; - } - - // We make the freshly created client span active so the propagated headers reference it (and not - // the parent span). Passing `{ span }` to `getTraceData()` is not enough: for an inactive span it - // resolves to the span's *captured* scope, whose active span is still the parent. - const addedHeaders = withActiveSpan(span, () => getTraceData({ propagateTraceparent })); - - for (const [k, v] of Object.entries(addedHeaders)) { - if (!v) { - continue; - } - - if (typeof request.addHeader === 'function') { - request.addHeader(k, v); - } else if (typeof request.headers === 'string') { - request.headers += `${k}: ${v}\r\n`; - } else if (Array.isArray(request.headers)) { - // undici@6.11.0 accidentally, briefly removed `request.addHeader()`. - request.headers.push(k, v); - } - } -} - function getRequestMethod(original: string): string { const knownMethods = { CONNECT: true,