From 2b3dbd0d3a4309ecaa41895792a20de4f933e057 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Thu, 18 Jun 2026 20:43:33 -0400 Subject: [PATCH 1/3] ref(node): Streamline undici (node-fetch) instrumentation Refactor the vendored `UndiciInstrumentation` (the span-emitting half of the NodeFetch integration) to use Sentry's span APIs instead of OpenTelemetry's tracing/propagation APIs. - Span creation: `tracer.startSpan` -> `startInactiveSpan({ kind: CLIENT })` (the span is created on `undici:request:create` and ended later in `onDone`/`onError`, so it must be inactive). - Status: `SpanStatusCode` -> `SPAN_STATUS_ERROR`; drop the no-op `recordException`. - Trace propagation: replace `propagation.inject(trace.setSpan(...))` with `withActiveSpan(span, () => getTraceData(...))` gated by `shouldPropagateTraceForUrl`. This keeps the propagated `sentry-trace` referencing the http.client span (not the parent), matching prior output. Passing `{ span }` to `getTraceData` alone is insufficient: for an inactive span it resolves to the span's captured scope, whose active span is the parent. - Drop the OTel metrics (no MeterProvider is wired up) and the dead `requireParentforSpans` code path (the SDK always passes `false`). - Replace the diag logger with `debug` + `DEBUG_BUILD`, swap OTel type imports for `@sentry/core`, inline the semantic-convention constants into a local `semconv.ts`, and drop the blanket `eslint-disable`s. The only remaining `@opentelemetry/api` usage is `SpanKind`; the `@opentelemetry/instrumentation` patching base is kept. Adds integration coverage for the error path (`internal_error` span) and for `headersToSpanAttributes` (request + response header capture). --- .../http-client-spans/fetch-error/scenario.ts | 35 +++ .../http-client-spans/fetch-error/test.ts | 21 ++ .../scenario.ts | 22 ++ .../fetch-headers-to-span-attributes/test.ts | 35 +++ .../node/src/integrations/node-fetch/index.ts | 1 - .../node-fetch/vendored/internal-types.ts | 1 - .../node-fetch/vendored/semconv.ts | 23 ++ .../integrations/node-fetch/vendored/types.ts | 7 +- .../node-fetch/vendored/undici.ts | 241 +++++++----------- 9 files changed, 236 insertions(+), 150 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts create mode 100644 packages/node/src/integrations/node-fetch/vendored/semconv.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts new file mode 100644 index 000000000000..f324c3258bea --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/scenario.ts @@ -0,0 +1,35 @@ +import { createServer } from 'http'; +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', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// Bind and immediately release a port so we have an address that reliably refuses the connection. +// A refused outgoing request fires the `undici:request:error` channel, exercising the error path. +function getRefusedPort(): Promise { + return new Promise(resolve => { + const server = createServer(); + server.listen(0, () => { + const { port } = server.address() as { port: number }; + server.close(() => resolve(port)); + }); + }); +} + +async function run(): Promise { + const port = await getRefusedPort(); + + await Sentry.startSpan({ name: 'test_transaction' }, async () => { + await fetch(`http://localhost:${port}/api/v0`).catch(() => { + // Ignore the expected connection error + }); + }); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts new file mode 100644 index 000000000000..9afe7a471591 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-error/test.ts @@ -0,0 +1,21 @@ +import { expect, test } from 'vitest'; +import { createRunner } from '../../../../utils/runner'; + +test('captures an errored span for a failed outgoing fetch request', async () => { + await createRunner(__dirname, 'scenario.ts') + .expect({ + transaction: { + transaction: 'test_transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/GET http:\/\/localhost:\d+\//), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + status: 'internal_error', + }), + ]), + }, + }) + .start() + .completed(); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts new file mode 100644 index 000000000000..ee95ba49aa1b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/scenario.ts @@ -0,0 +1,22 @@ +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', + tracesSampleRate: 1.0, + transport: loggingTransport, + integrations: [ + Sentry.nativeNodeFetchIntegration({ + headersToSpanAttributes: { + requestHeaders: ['x-test-header'], + responseHeaders: ['x-powered-by'], + }, + }), + ], +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'test_transaction' }, async () => { + await fetch(`${process.env.SERVER_URL}/api/v0`, { headers: { 'x-test-header': 'test-value' } }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts new file mode 100644 index 000000000000..eb0ee30352ef --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http-client-spans/fetch-headers-to-span-attributes/test.ts @@ -0,0 +1,35 @@ +import { createTestServer } from '@sentry-internal/test-utils'; +import { expect, test } from 'vitest'; +import { createRunner } from '../../../../utils/runner'; + +test('maps configured request & response headers to span attributes', async () => { + expect.assertions(2); + + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', headers => { + expect(headers['x-test-header']).toBe('test-value'); + }) + .start(); + + await createRunner(__dirname, 'scenario.ts') + .withEnv({ SERVER_URL }) + .expect({ + transaction: { + transaction: 'test_transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: expect.stringMatching(/GET .*\/api\/v0/), + op: 'http.client', + origin: 'auto.http.otel.node_fetch', + data: expect.objectContaining({ + 'http.request.header.x-test-header': ['test-value'], + 'http.response.header.x-powered-by': ['Express'], + }), + }), + ]), + }, + }) + .start() + .completed(); + closeTestServer(); +}); diff --git a/packages/node/src/integrations/node-fetch/index.ts b/packages/node/src/integrations/node-fetch/index.ts index 2aa277b211c4..0dc610c0ba0a 100644 --- a/packages/node/src/integrations/node-fetch/index.ts +++ b/packages/node/src/integrations/node-fetch/index.ts @@ -116,7 +116,6 @@ function _shouldInstrumentSpans(options: NodeFetchOptions, clientOptions: Partia /** Exported only for tests. */ export function _getConfigWithDefaults(options: Partial = {}): UndiciInstrumentationConfig { const instrumentationConfig = { - requireParentforSpans: false, ignoreRequestHook: request => { const url = getAbsoluteUrl(request.origin, request.path); const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; diff --git a/packages/node/src/integrations/node-fetch/vendored/internal-types.ts b/packages/node/src/integrations/node-fetch/vendored/internal-types.ts index 99fde69bb60b..687ee9c43cfe 100644 --- a/packages/node/src/integrations/node-fetch/vendored/internal-types.ts +++ b/packages/node/src/integrations/node-fetch/vendored/internal-types.ts @@ -7,7 +7,6 @@ * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0 * - Tracking issue: https://github.com/getsentry/sentry-javascript/issues/20165 */ -/* eslint-disable -- vendored @opentelemetry/instrumentation-undici (#20165) */ import type { UndiciRequest, UndiciResponse } from './types'; diff --git a/packages/node/src/integrations/node-fetch/vendored/semconv.ts b/packages/node/src/integrations/node-fetch/vendored/semconv.ts new file mode 100644 index 000000000000..882cf2281487 --- /dev/null +++ b/packages/node/src/integrations/node-fetch/vendored/semconv.ts @@ -0,0 +1,23 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + * + * NOTICE from the Sentry authors: + * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/ed97091c9890dd18e52759f2ea98e9d7593b3ae4/packages/instrumentation-undici + * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0 + * - The semantic-convention constants this package emits, inlined from + * `@opentelemetry/semantic-conventions` (matching the sibling vendored dirs). + */ + +export const ATTR_HTTP_REQUEST_METHOD = 'http.request.method' as const; +export const ATTR_HTTP_REQUEST_METHOD_ORIGINAL = 'http.request.method_original' as const; +export const ATTR_HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code' as const; +export const ATTR_NETWORK_PEER_ADDRESS = 'network.peer.address' as const; +export const ATTR_NETWORK_PEER_PORT = 'network.peer.port' as const; +export const ATTR_SERVER_ADDRESS = 'server.address' as const; +export const ATTR_SERVER_PORT = 'server.port' as const; +export const ATTR_URL_FULL = 'url.full' as const; +export const ATTR_URL_PATH = 'url.path' as const; +export const ATTR_URL_QUERY = 'url.query' as const; +export const ATTR_URL_SCHEME = 'url.scheme' as const; +export const ATTR_USER_AGENT_ORIGINAL = 'user_agent.original' as const; diff --git a/packages/node/src/integrations/node-fetch/vendored/types.ts b/packages/node/src/integrations/node-fetch/vendored/types.ts index c1e8433f21a3..9bcb90f08c8e 100644 --- a/packages/node/src/integrations/node-fetch/vendored/types.ts +++ b/packages/node/src/integrations/node-fetch/vendored/types.ts @@ -7,10 +7,9 @@ * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0 * - Tracking issue: https://github.com/getsentry/sentry-javascript/issues/20165 */ -/* eslint-disable -- vendored @opentelemetry/instrumentation-undici (#20165) */ import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; -import type { Attributes, Span } from '@opentelemetry/api'; +import type { Span, SpanAttributes } from '@sentry/core'; export interface UndiciRequest { origin: string; @@ -54,7 +53,7 @@ export interface ResponseHookFunction { - (request: T): Attributes; + (request: T): SpanAttributes; } // This package will instrument HTTP requests made through `undici` or `fetch` global API @@ -71,8 +70,6 @@ export interface UndiciInstrumentationConfig< responseHook?: ResponseHookFunction; /** Function for adding custom attributes before a span is started */ startSpanHook?: StartSpanHookFunction; - /** Require parent to create span for outgoing requests */ - requireParentforSpans?: boolean; /** Map the following HTTP headers to span attributes. */ headersToSpanAttributes?: { requestHeaders?: string[]; diff --git a/packages/node/src/integrations/node-fetch/vendored/undici.ts b/packages/node/src/integrations/node-fetch/vendored/undici.ts index 0ab187ab4007..f348e87d5b0d 100644 --- a/packages/node/src/integrations/node-fetch/vendored/undici.ts +++ b/packages/node/src/integrations/node-fetch/vendored/undici.ts @@ -6,26 +6,30 @@ * - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/ed97091c9890dd18e52759f2ea98e9d7593b3ae4/packages/instrumentation-undici * - Upstream version: @opentelemetry/instrumentation-undici@0.24.0 * - Tracking issue: https://github.com/getsentry/sentry-javascript/issues/20165 - * - Minor TypeScript strictness adjustments for this repository's compiler settings + * - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs + * - Dropped the OTel metrics (no MeterProvider is wired up) and the dead + * `requireParentforSpans` code path (the SDK always passes `false`) */ -/* eslint-disable -- vendored @opentelemetry/instrumentation-undici (#20165) */ import * as diagch from 'diagnostics_channel'; import { URL } from 'url'; +import { SpanKind } from '@opentelemetry/api'; import { InstrumentationBase, safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; -import type { Attributes, Histogram, Span } from '@opentelemetry/api'; +import type { Span, SpanAttributes } from '@sentry/core'; import { - context, - INVALID_SPAN_CONTEXT, - propagation, - SpanKind, - SpanStatusCode, - trace, - ValueType, -} from '@opentelemetry/api'; + debug, + getClient, + getTraceData, + LRUMap, + SDK_VERSION, + shouldPropagateTraceForUrl, + SPAN_STATUS_ERROR, + startInactiveSpan, + withActiveSpan, +} from '@sentry/core'; +import { DEBUG_BUILD } from '../../../debug-build'; import { - ATTR_ERROR_TYPE, ATTR_HTTP_REQUEST_METHOD, ATTR_HTTP_REQUEST_METHOD_ORIGINAL, ATTR_HTTP_RESPONSE_STATUS_CODE, @@ -38,11 +42,11 @@ import { ATTR_URL_QUERY, ATTR_URL_SCHEME, ATTR_USER_AGENT_ORIGINAL, - METRIC_HTTP_CLIENT_REQUEST_DURATION, -} from '@opentelemetry/semantic-conventions'; +} from './semconv'; import type { ListenerRecord, + RequestErrorMessage, RequestHeadersMessage, RequestMessage, RequestTrailersMessage, @@ -50,14 +54,6 @@ import type { } from './internal-types'; import type { UndiciInstrumentationConfig, UndiciRequest } from './types'; -import { SDK_VERSION, timestampInSeconds } from '@sentry/core'; - -interface InstrumentationRecord { - span: Span; - attributes: Attributes; - startTime: number; -} - const PACKAGE_NAME = '@sentry/instrumentation-undici'; // A combination of https://github.com/elastic/apm-agent-nodejs and @@ -66,16 +62,16 @@ export class UndiciInstrumentation extends InstrumentationBase; - private _recordFromReq = new WeakMap(); - - declare private _httpClientDurationHistogram: Histogram; + private _spanFromReq = new WeakMap(); + // Caches trace-propagation decisions per URL so we don't recompute the `tracePropagationTargets` regexes per request. + private _propagationDecisionMap = new LRUMap(100); constructor(config: UndiciInstrumentationConfig = {}) { super(PACKAGE_NAME, SDK_VERSION, config); } // No need to instrument files/modules - protected override init() { + protected override init(): void { return undefined; } @@ -113,18 +109,10 @@ export class UndiciInstrumentation extends InstrumentationBase void) { + private subscribeToChannel( + diagnosticChannel: string, + onMessage: (message: any, name: string | symbol) => void, + ): void { // `diagnostics_channel` had a ref counting bug until v18.19.0. // https://github.com/nodejs/node/pull/47520 const [major = 0, minor = 0] = process.version @@ -149,7 +137,7 @@ export class UndiciInstrumentation extends InstrumentationBase { const result = new Map(); if (Array.isArray(request.headers)) { @@ -205,7 +193,7 @@ export class UndiciInstrumentation extends InstrumentationBase !enabled || request.method === 'CONNECT' || config.ignoreRequestHook?.(request), - e => e && this._diag.error('caught ignoreRequestHook error: ', e), + e => e && DEBUG_BUILD && debug.error('caught ignoreRequestHook error: ', e), true, ); @@ -213,18 +201,17 @@ export class UndiciInstrumentation extends InstrumentationBase config.startSpanHook?.(request), - e => e && this._diag.error('caught startSpanHook error: ', e), + e => e && DEBUG_BUILD && debug.error('caught startSpanHook error: ', e), true, ); if (hookAttributes) { @@ -266,75 +253,40 @@ export class UndiciInstrumentation extends InstrumentationBase config.requestHook?.(span, request), - e => e && this._diag.error('caught requestHook error: ', e), + e => e && DEBUG_BUILD && debug.error('caught requestHook error: ', e), true, ); - // Context propagation goes last so no hook can tamper - // the propagation headers - const requestContext = trace.setSpan(context.active(), span); - const addedHeaders: Record = {}; - propagation.inject(requestContext, addedHeaders); - - const headerEntries = Object.entries(addedHeaders); + // 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. + this.injectTracePropagationHeaders(span, request, requestUrl.toString()); - for (let i = 0; i < headerEntries.length; i++) { - const pair = headerEntries[i]; - if (!pair) { - continue; - } - const [k, v] = pair; - - 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); - } - } - this._recordFromReq.set(request, { span, attributes, startTime }); + this._spanFromReq.set(request, span); } // 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 private onRequestHeaders({ request, socket }: RequestHeadersMessage): void { - const record = this._recordFromReq.get(request); + const span = this._spanFromReq.get(request); - if (!record) { + if (!span) { return; } const config = this.getConfig(); - const { span } = record; const { remoteAddress, remotePort } = socket; - const spanAttributes: Attributes = { + const spanAttributes: SpanAttributes = { [ATTR_NETWORK_PEER_ADDRESS]: remoteAddress, [ATTR_NETWORK_PEER_PORT]: remotePort, }; @@ -360,14 +312,13 @@ export class UndiciInstrumentation extends InstrumentationBase config.responseHook?.(span, { request, response }), - e => e && this._diag.error('caught responseHook error: ', e), + e => e && DEBUG_BUILD && debug.error('caught responseHook error: ', e), true, ); if (config.headersToSpanAttributes?.responseHeaders) { - const headersToAttribs = new Set(); + const headersToAttribs = new Set(); config.headersToSpanAttributes?.responseHeaders.forEach(name => headersToAttribs.add(name.toLowerCase())); for (let idx = 0; idx < response.headers.length; idx = idx + 2) { @@ -405,28 +356,24 @@ export class UndiciInstrumentation extends InstrumentationBase= 400 ? SpanStatusCode.ERROR : SpanStatusCode.UNSET, - }); - record.attributes = Object.assign(attributes, spanAttributes); + + // The Sentry pipeline infers `ok` / `not_found` / etc. from `http.response.status_code` when the + // status is left unset, so we only need to flag erroneous responses explicitly. + if (response.statusCode >= 400) { + span.setStatus({ code: SPAN_STATUS_ERROR }); + } } // This is the last event we receive if the request went without any errors private onDone({ request }: RequestTrailersMessage): void { - const record = this._recordFromReq.get(request); + const span = this._spanFromReq.get(request); - if (!record) { + if (!span) { return; } - const { span, attributes, startTime } = record; - - // End the span span.end(); - this._recordFromReq.delete(request); - - // Record metrics - this.recordRequestDuration(attributes, startTime); + this._spanFromReq.delete(request); } // This is the event we get when something is wrong in the request like @@ -435,55 +382,63 @@ export class UndiciInstrumentation extends InstrumentationBase { - if (key in attributes) { - metricsAttributes[key] = attributes[key]; + // 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. + private injectTracePropagationHeaders(span: Span, request: UndiciRequest, url: string): void { + const { tracePropagationTargets, propagateTraceparent } = getClient()?.getOptions() ?? {}; + + if (!shouldPropagateTraceForUrl(url, tracePropagationTargets, this._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 })); + + const headerEntries = Object.entries(addedHeaders); + + for (let i = 0; i < headerEntries.length; i++) { + const pair = headerEntries[i]; + if (!pair) { + continue; + } + const [k, v] = pair; + if (!v) { + continue; } - }); - // Take the duration and record it - const durationSeconds = timestampInSeconds() - startTime; - this._httpClientDurationHistogram.record(durationSeconds, metricsAttributes); + 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); + } + } } private getRequestMethod(original: string): string { From efa80eceb8957f3ad18e88450fe5ed4675daffa2 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 19 Jun 2026 14:35:51 -0400 Subject: [PATCH 2/3] test(node): Assert outgoing fetch traceparent references the http.client span --- .../suites/tracing/requests/traceparent/test.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/traceparent/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/traceparent/test.ts index 517ea314dfc5..b8c3f8946f94 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/traceparent/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/traceparent/test.ts @@ -5,10 +5,13 @@ import { createEsmAndCjsTests } from '../../../../utils/runner'; describe('outgoing traceparent', () => { createEsmAndCjsTests(__dirname, 'scenario-fetch.mjs', 'instrument.mjs', (createRunner, test) => { test('outgoing fetch requests should get traceparent headers', async () => { - expect.assertions(5); + expect.assertions(7); + + let outgoingSentryTrace: string | undefined; const [SERVER_URL, closeTestServer] = await createTestServer() .get('/api/v1', headers => { + outgoingSentryTrace = headers['sentry-trace'] as string; expect(headers['baggage']).toEqual(expect.any(String)); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})-1$/)); expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0'); @@ -19,8 +22,14 @@ describe('outgoing traceparent', () => { await createRunner() .withEnv({ SERVER_URL }) .expect({ - transaction: { - // we're not too concerned with the actual transaction here since this is tested elsewhere + // The propagated `sentry-trace` must reference the `http.client` span, not the surrounding transaction span. + transaction: event => { + const propagatedSpanId = outgoingSentryTrace?.split('-')[1]; + const httpClientSpan = event.spans?.find(span => span.op === 'http.client'); + + expect(httpClientSpan).toBeDefined(); + expect(propagatedSpanId).toBe(httpClientSpan?.span_id); + expect(propagatedSpanId).not.toBe(event.contexts?.trace?.span_id); }, }) .start() From 3962e7f4a162768b47e4f2389373ebd03372edaa Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 19 Jun 2026 15:10:11 -0400 Subject: [PATCH 3/3] ref(node): De-otelify undici instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the @opentelemetry/instrumentation base from the vendored undici instrumentation. Since undici reports via diagnostics_channel rather than module patching, InstrumentationBase provided nothing here — it's now a plain class that the integration subscribes/unsubscribes directly, off OTel's registerInstrumentations path. Also drops the remaining @opentelemetry/api usage (SpanKind, inlined) and safeExecuteInTheMiddle (local helper). Pure refactor: span emission, trace propagation, and breadcrumbs are unchanged. --- .../node/src/integrations/node-fetch/index.ts | 27 ++++--- .../node-fetch/vendored/undici.ts | 79 ++++++++----------- 2 files changed, 49 insertions(+), 57 deletions(-) diff --git a/packages/node/src/integrations/node-fetch/index.ts b/packages/node/src/integrations/node-fetch/index.ts index 0dc610c0ba0a..e3471ead1834 100644 --- a/packages/node/src/integrations/node-fetch/index.ts +++ b/packages/node/src/integrations/node-fetch/index.ts @@ -53,13 +53,16 @@ interface NodeFetchOptions extends Pick< ignoreOutgoingRequests?: (url: string) => boolean; } -const instrumentOtelNodeFetch = generateInstrumentOnce( - INTEGRATION_NAME, - UndiciInstrumentation, - (options: NodeFetchOptions) => { - return _getConfigWithDefaults(options); - }, -); +let _undiciInstrumentation: UndiciInstrumentation | undefined; + +// Sets up the vendored undici instrumentation (emits `http.client` spans & propagates traces). +// The module-level singleton mirrors `generateInstrumentOnce`'s "instrument once per process" behavior. +function instrumentNodeFetchSpans(options: NodeFetchOptions): void { + if (!_undiciInstrumentation) { + _undiciInstrumentation = new UndiciInstrumentation(_getConfigWithDefaults(options)); + } + _undiciInstrumentation.enable(); +} const instrumentSentryNodeFetch = generateInstrumentOnce( `${INTEGRATION_NAME}.sentry`, @@ -75,14 +78,14 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { setupOnce() { const instrumentSpans = _shouldInstrumentSpans(options, getClient()?.getOptions()); - // This is the "regular" OTEL instrumentation that emits spans + // This is the instrumentation that emits spans & propagates traces for outgoing fetch requests if (instrumentSpans) { - instrumentOtelNodeFetch(options); + instrumentNodeFetchSpans(options); } - // This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces - // This must be registered after the OTEL one, to ensure that the core trace propagation logic takes presedence - // Otherwise, the sentry-trace header may be set multiple times + // 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); }, }; diff --git a/packages/node/src/integrations/node-fetch/vendored/undici.ts b/packages/node/src/integrations/node-fetch/vendored/undici.ts index f348e87d5b0d..10d6c568cb71 100644 --- a/packages/node/src/integrations/node-fetch/vendored/undici.ts +++ b/packages/node/src/integrations/node-fetch/vendored/undici.ts @@ -9,20 +9,19 @@ * - Refactored to use Sentry's span APIs instead of OpenTelemetry tracing APIs * - Dropped the OTel metrics (no MeterProvider is wired up) and the dead * `requireParentforSpans` code path (the SDK always passes `false`) + * - Dropped the `@opentelemetry/instrumentation` base (undici reports via `diagnostics_channel`, + * so no module patching was needed) — now a plain class wired up directly by the integration */ import * as diagch from 'diagnostics_channel'; import { URL } from 'url'; -import { SpanKind } from '@opentelemetry/api'; -import { InstrumentationBase, safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; import type { Span, SpanAttributes } from '@sentry/core'; import { debug, getClient, getTraceData, LRUMap, - SDK_VERSION, shouldPropagateTraceForUrl, SPAN_STATUS_ERROR, startInactiveSpan, @@ -54,50 +53,44 @@ import type { } from './internal-types'; import type { UndiciInstrumentationConfig, UndiciRequest } from './types'; -const PACKAGE_NAME = '@sentry/instrumentation-undici'; +// `SpanKind.CLIENT`, inlined to avoid importing from `@opentelemetry/api`. +const SPAN_KIND_CLIENT = 2; + +/** Replaces OTel's `safeExecuteInTheMiddle`: run `fn`, route any error to `onError`, and swallow it. */ +function safeExecute(fn: () => T, onError: (error: unknown) => void): T | undefined { + try { + return fn(); + } catch (error) { + onError(error); + return undefined; + } +} // 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 class UndiciInstrumentation extends InstrumentationBase { - // Keep ref to avoid https://github.com/nodejs/node/issues/42170 bug and for - // unsubscribing. - declare private _channelSubs: Array; +// +// Not an OTel `InstrumentationBase` (undici reports via `diagnostics_channel`, not module patching); +// the integration wires this up directly via `enable()` / `disable()`. +export class UndiciInstrumentation { + // Keep ref to avoid https://github.com/nodejs/node/issues/42170 bug and for unsubscribing. + private _channelSubs: Array = []; private _spanFromReq = new WeakMap(); // Caches trace-propagation decisions per URL so we don't recompute the `tracePropagationTargets` regexes per request. private _propagationDecisionMap = new LRUMap(100); + private _config: UndiciInstrumentationConfig; constructor(config: UndiciInstrumentationConfig = {}) { - super(PACKAGE_NAME, SDK_VERSION, config); - } - - // No need to instrument files/modules - protected override init(): void { - return undefined; + this._config = config; } - override disable(): void { - super.disable(); + public disable(): void { this._channelSubs.forEach(sub => sub.unsubscribe()); this._channelSubs.length = 0; } - override enable(): void { - // "enabled" handling is currently a bit messy with InstrumentationBase. - // If constructed with `{enabled: false}`, this `.enable()` is still called, - // and `this.getConfig().enabled !== this.isEnabled()`, creating confusion. - // - // For now, this class will setup for instrumenting if `.enable()` is - // called, but use `this.getConfig().enabled` to determine if - // instrumentation should be generated. This covers the more likely common - // case of config being given a construction time, rather than later via - // `instance.enable()`, `.disable()`, or `.setConfig()` calls. - super.enable(); - - // This method is called by the super-class constructor before ours is - // called. So we need to ensure the property is initalized. - this._channelSubs = this._channelSubs || []; - - // Avoid to duplicate subscriptions + /** Subscribe to the undici diagnostics channels (idempotent). */ + public enable(): void { + // Avoid duplicate subscriptions if (this._channelSubs.length > 0) { return; } @@ -189,12 +182,11 @@ export class UndiciInstrumentation extends InstrumentationBase !enabled || request.method === 'CONNECT' || config.ignoreRequestHook?.(request), e => e && DEBUG_BUILD && debug.error('caught ignoreRequestHook error: ', e), - true, ); if (shouldIgnoreReq) { @@ -242,10 +234,9 @@ export class UndiciInstrumentation extends InstrumentationBase config.startSpanHook?.(request), e => e && DEBUG_BUILD && debug.error('caught startSpanHook error: ', e), - true, ); if (hookAttributes) { Object.entries(hookAttributes).forEach(([key, val]) => { @@ -255,15 +246,14 @@ export class UndiciInstrumentation extends InstrumentationBase config.requestHook?.(span, request), e => e && DEBUG_BUILD && debug.error('caught requestHook error: ', e), - true, ); // Context propagation goes last so no hook can tamper the propagation headers. @@ -284,7 +274,7 @@ export class UndiciInstrumentation extends InstrumentationBase config.responseHook?.(span, { request, response }), e => e && DEBUG_BUILD && debug.error('caught responseHook error: ', e), - true, ); if (config.headersToSpanAttributes?.responseHeaders) {