Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"@opentelemetry/instrumentation": "^0.214.0",
"@opentelemetry/sdk-trace-base": "^2.6.1",
"@opentelemetry/semantic-conventions": "^1.40.0",
"@sentry/conventions": "^0.12.0",
"@sentry/core": "10.61.0",
"@sentry/node-core": "10.61.0",
"@sentry/opentelemetry": "10.61.0",
Expand Down
120 changes: 9 additions & 111 deletions packages/node/src/integrations/node-fetch/index.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,13 @@
import type { UndiciInstrumentationConfig } from './vendored/types';
import { UndiciInstrumentation } from './vendored/undici';
import { instrumentUndici } from './undici-instrumentation';
import type { NodeFetchOptions } from './types';
import type { IntegrationFn } from '@sentry/core';
import {
defineIntegration,
getClient,
hasSpansEnabled,
SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_URL_FULL,
stripDataUrlContent,
} 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';

interface NodeFetchOptions extends Pick<
UndiciInstrumentationConfig,
'requestHook' | 'responseHook' | 'headersToSpanAttributes'
> {
/**
* Whether breadcrumbs should be recorded for requests.
* Defaults to true
*/
breadcrumbs?: boolean;

/**
* 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.
Comment on lines -27 to -32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The tracePropagation: false option is ignored by the new instrumentUndici function, causing trace headers to be injected unconditionally for span instrumentation.
Severity: MEDIUM

Suggested Fix

Update the UndiciInstrumentationConfig type to include the tracePropagation option. Pass this option from the nativeNodeFetchIntegration options into instrumentUndici(). Finally, modify instrumentUndici() and its helper injectTracePropagationHeaders() to respect the tracePropagation flag and only inject headers when it is not false.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/node/src/integrations/node-fetch/index.ts#L27-L32

Potential issue: The new undici span instrumentation does not honor the
`tracePropagation: false` option. The `instrumentUndici()` function, which handles span
instrumentation, is called without the `tracePropagation` option and has no mechanism to
receive it. As a result, its internal `injectTracePropagationHeaders()` function is
always called, leading to trace headers being unconditionally injected for outgoing
requests when spans are enabled. This is a regression from previous behavior where the
option was respected, and it contradicts the user's explicit configuration.

Also affects:

  • packages/node/src/integrations/node-fetch/undici-instrumentation.ts:385~411

Did we get this right? 👍 / 👎 to inform future reviews.

*
* If `skipOpenTelemetrySetup: true` is configured, this defaults to `false`, otherwise it defaults to `true`.
*/
spans?: boolean;

/**
* Whether to inject trace propagation headers (sentry-trace, baggage, traceparent) into outgoing fetch requests.
*
* When set to `false`, Sentry will not inject any trace propagation headers, but will still create breadcrumbs
* (if `breadcrumbs` is enabled). This is useful when `skipOpenTelemetrySetup: true` is configured and you want
* to avoid duplicate trace headers being injected by both Sentry and OpenTelemetry's UndiciInstrumentation.
*
* @default `true`
*/
tracePropagation?: boolean;

/**
* Do not capture spans or breadcrumbs for outgoing fetch requests to URLs where the given callback returns `true`.
* This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled.
*/
ignoreOutgoingRequests?: (url: string) => boolean;
}

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`,
SentryNodeFetchInstrumentation,
Expand All @@ -80,7 +24,12 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {

// This is the instrumentation that emits spans & propagates traces for outgoing fetch requests
if (instrumentSpans) {
instrumentNodeFetchSpans(options);
instrumentUndici({
ignoreOutgoingRequests: options.ignoreOutgoingRequests,
requestHook: options.requestHook,
responseHook: options.responseHook,
headersToSpanAttributes: options.headersToSpanAttributes,
});
}

// This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces.
Expand All @@ -93,61 +42,10 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {

export const nativeNodeFetchIntegration = defineIntegration(_nativeNodeFetchIntegration);

// Matching the behavior of the base instrumentation
function getAbsoluteUrl(origin: string, path: string = '/'): string {
const url = `${origin}`;

if (url.endsWith('/') && path.startsWith('/')) {
return `${url}${path.slice(1)}`;
}

if (!url.endsWith('/') && !path.startsWith('/')) {
return `${url}/${path}`;
}

return `${url}${path}`;
}

function _shouldInstrumentSpans(options: NodeFetchOptions, clientOptions: Partial<NodeClientOptions> = {}): boolean {
// If `spans` is passed in, it takes precedence
// Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true` or spans are not enabled
return typeof options.spans === 'boolean'
? options.spans
: !clientOptions.skipOpenTelemetrySetup && hasSpansEnabled(clientOptions);
}

/** Exported only for tests. */
export function _getConfigWithDefaults(options: Partial<NodeFetchOptions> = {}): UndiciInstrumentationConfig {
const instrumentationConfig = {
ignoreRequestHook: request => {
const url = getAbsoluteUrl(request.origin, request.path);
const _ignoreOutgoingRequests = options.ignoreOutgoingRequests;
const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url);

return !!shouldIgnore;
},
startSpanHook: request => {
const url = getAbsoluteUrl(request.origin, request.path);

// Sanitize data URLs to prevent long base64 strings in span attributes
if (url.startsWith('data:')) {
const sanitizedUrl = stripDataUrlContent(url);
return {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch',
'http.url': sanitizedUrl,
[SEMANTIC_ATTRIBUTE_URL_FULL]: sanitizedUrl,
[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]: `${request.method || 'GET'} ${sanitizedUrl}`,
};
}

return {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch',
};
},
requestHook: options.requestHook,
responseHook: options.responseHook,
headersToSpanAttributes: options.headersToSpanAttributes,
} satisfies UndiciInstrumentationConfig;

return instrumentationConfig;
}
123 changes: 123 additions & 0 deletions packages/node/src/integrations/node-fetch/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* 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
* - Tracking issue: https://github.com/getsentry/sentry-javascript/issues/20165
* - Dropped the `@opentelemetry/instrumentation` `InstrumentationConfig` base (its only field used
* here, `enabled`, is unused by the Sentry integration)
*/

import type { Span } from '@sentry/core';

export interface UndiciRequest {
origin: string;
method: string;
path: string;
/**
* Serialized string of headers in the form `name: value\r\n` for v5
* Array of strings `[key1, value1, key2, value2]`, where values are
* `string | string[]` for v6
*/
headers: string | (string | string[])[];
/**
* Helper method to add headers (from v6)
*/
addHeader: (name: string, value: string) => void;
throwOnError: boolean;
completed: boolean;
aborted: boolean;
idempotent: boolean;
contentLength: number | null;
contentType: string | null;
// oxlint-disable-next-line typescript/no-explicit-any
body: any;
}

export interface UndiciResponse {
headers: Buffer[];
statusCode: number;
statusText: string;
}

export interface RequestHookFunction<T = UndiciRequest> {
(span: Span, request: T): void;
}

export interface ResponseHookFunction<RequestType = UndiciRequest, ResponseType = UndiciResponse> {
(span: Span, info: { request: RequestType; response: ResponseType }): void;
}

export interface RequestMessage {
request: UndiciRequest;
}

export interface RequestHeadersMessage {
request: UndiciRequest;
// oxlint-disable-next-line typescript/no-explicit-any
socket: any;
}

export interface ResponseHeadersMessage {
request: UndiciRequest;
response: UndiciResponse;
}

export interface RequestTrailersMessage {
request: UndiciRequest;
response: UndiciResponse;
}

export interface RequestErrorMessage {
request: UndiciRequest;
error: Error;
}

// This package will instrument HTTP requests made through `undici` or `fetch` global API
// so it seems logical to have similar options than the HTTP instrumentation
export interface UndiciInstrumentationConfig<RequestType = UndiciRequest, ResponseType = UndiciResponse> {
/**
* Do not capture spans or breadcrumbs for outgoing fetch requests to URLs where the given callback returns `true`.
* This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled.
*/
ignoreOutgoingRequests?: (url: string) => boolean;
/** Function for adding custom attributes before request is handled */
requestHook?: RequestHookFunction<RequestType>;
/** Function called once response headers have been received */
responseHook?: ResponseHookFunction<RequestType, ResponseType>;
/** Map the following HTTP headers to span attributes. */
headersToSpanAttributes?: {
requestHeaders?: string[];
responseHeaders?: string[];
};
}

export interface NodeFetchOptions extends UndiciInstrumentationConfig {
/**
* Whether breadcrumbs should be recorded for requests.
* Defaults to true
*/
breadcrumbs?: boolean;

/**
* 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.
*
* If `skipOpenTelemetrySetup: true` is configured, this defaults to `false`, otherwise it defaults to `true`.
*/
spans?: boolean;

/**
* Whether to inject trace propagation headers (sentry-trace, baggage, traceparent) into outgoing fetch requests.
*
* When set to `false`, Sentry will not inject any trace propagation headers, but will still create breadcrumbs
* (if `breadcrumbs` is enabled). This is useful when `skipOpenTelemetrySetup: true` is configured and you want
* to avoid duplicate trace headers being injected by both Sentry and OpenTelemetry's UndiciInstrumentation.
*
* @default `true`
*/
tracePropagation?: boolean;
}
Loading
Loading