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/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() diff --git a/packages/node/src/integrations/node-fetch/index.ts b/packages/node/src/integrations/node-fetch/index.ts index 2aa277b211c4..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); }, }; @@ -116,7 +119,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..10d6c568cb71 100644 --- a/packages/node/src/integrations/node-fetch/vendored/undici.ts +++ b/packages/node/src/integrations/node-fetch/vendored/undici.ts @@ -6,26 +6,29 @@ * - 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`) + * - 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 */ -/* eslint-disable -- vendored @opentelemetry/instrumentation-undici (#20165) */ import * as diagch from 'diagnostics_channel'; import { URL } from 'url'; -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, + 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 +41,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,58 +53,44 @@ import type { } from './internal-types'; import type { UndiciInstrumentationConfig, UndiciRequest } from './types'; -import { SDK_VERSION, timestampInSeconds } from '@sentry/core'; +// `SpanKind.CLIENT`, inlined to avoid importing from `@opentelemetry/api`. +const SPAN_KIND_CLIENT = 2; -interface InstrumentationRecord { - span: Span; - attributes: Attributes; - startTime: number; +/** 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; + } } -const PACKAGE_NAME = '@sentry/instrumentation-undici'; - // 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; - private _recordFromReq = new WeakMap(); - - declare private _httpClientDurationHistogram: Histogram; +// +// 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); + this._config = config; } - // No need to instrument files/modules - protected override init() { - return undefined; - } - - 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; } @@ -113,18 +102,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 +130,7 @@ export class UndiciInstrumentation extends InstrumentationBase { const result = new Map(); if (Array.isArray(request.headers)) { @@ -201,30 +182,28 @@ export class UndiciInstrumentation extends InstrumentationBase !enabled || request.method === 'CONNECT' || config.ignoreRequestHook?.(request), - e => e && this._diag.error('caught ignoreRequestHook error: ', e), - true, + e => e && DEBUG_BUILD && debug.error('caught ignoreRequestHook error: ', e), ); if (shouldIgnoreReq) { return; } - const startTime = timestampInSeconds(); let requestUrl; try { requestUrl = new URL(request.path, request.origin); } catch (err) { - this._diag.warn('could not determine url.full:', err); + DEBUG_BUILD && debug.warn('could not determine url.full:', err); // Skip instrumenting this request. return; } const urlScheme = requestUrl.protocol.replace(':', ''); const requestMethod = this.getRequestMethod(request.method); - const attributes: Attributes = { + const attributes: SpanAttributes = { [ATTR_HTTP_REQUEST_METHOD]: requestMethod, [ATTR_HTTP_REQUEST_METHOD_ORIGINAL]: request.method, [ATTR_URL_FULL]: requestUrl.toString(), @@ -255,10 +234,9 @@ export class UndiciInstrumentation extends InstrumentationBase config.startSpanHook?.(request), - e => e && this._diag.error('caught startSpanHook error: ', e), - true, + e => e && DEBUG_BUILD && debug.error('caught startSpanHook error: ', e), ); if (hookAttributes) { Object.entries(hookAttributes).forEach(([key, val]) => { @@ -266,75 +244,39 @@ export class UndiciInstrumentation extends InstrumentationBase config.requestHook?.(span, request), - e => e && this._diag.error('caught requestHook error: ', e), - true, + e => e && DEBUG_BUILD && debug.error('caught requestHook error: ', e), ); - // 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); - - for (let i = 0; i < headerEntries.length; i++) { - const pair = headerEntries[i]; - if (!pair) { - continue; - } - const [k, v] = pair; + // 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()); - 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 config = this._config; const { remoteAddress, remotePort } = socket; - const spanAttributes: Attributes = { + const spanAttributes: SpanAttributes = { [ATTR_NETWORK_PEER_ADDRESS]: remoteAddress, [ATTR_NETWORK_PEER_PORT]: remotePort, }; @@ -360,28 +302,26 @@ export class UndiciInstrumentation extends InstrumentationBase config.responseHook?.(span, { request, response }), - e => e && this._diag.error('caught responseHook error: ', e), - true, + e => e && DEBUG_BUILD && debug.error('caught responseHook error: ', e), ); 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 +345,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 +371,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 {