From 8302e68d95fda6793c86a2f2394beae667e0cb90 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 18 Oct 2024 10:47:01 +0200 Subject: [PATCH 1/7] feat(node): fetch breadcrumbs without tracing --- .../src/integrations/diagnostics_channel.d.ts | 556 ++++++++++++++++++ .../src/integrations/fetch-breadcrumbs.ts | 114 ++++ packages/node/src/integrations/node-fetch.ts | 79 +-- packages/node/src/types.ts | 7 + 4 files changed, 691 insertions(+), 65 deletions(-) create mode 100644 packages/node/src/integrations/diagnostics_channel.d.ts create mode 100644 packages/node/src/integrations/fetch-breadcrumbs.ts diff --git a/packages/node/src/integrations/diagnostics_channel.d.ts b/packages/node/src/integrations/diagnostics_channel.d.ts new file mode 100644 index 000000000000..abf3649a617f --- /dev/null +++ b/packages/node/src/integrations/diagnostics_channel.d.ts @@ -0,0 +1,556 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/ban-types */ +/* eslint-disable @typescript-eslint/explicit-member-accessibility */ + +/** + * The `node:diagnostics_channel` module provides an API to create named channels + * to report arbitrary message data for diagnostics purposes. + * + * It can be accessed using: + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * ``` + * + * It is intended that a module writer wanting to report diagnostics messages + * will create one or many top-level channels to report messages through. + * Channels may also be acquired at runtime but it is not encouraged + * due to the additional overhead of doing so. Channels may be exported for + * convenience, but as long as the name is known it can be acquired anywhere. + * + * If you intend for your module to produce diagnostics data for others to + * consume it is recommended that you include documentation of what named + * channels are used along with the shape of the message data. Channel names + * should generally include the module name to avoid collisions with data from + * other modules. + * @since v15.1.0, v14.17.0 + * @see [source](https://github.com/nodejs/node/blob/v22.x/lib/diagnostics_channel.js) + */ +declare module 'diagnostics_channel' { + import type { AsyncLocalStorage } from 'node:async_hooks'; + /** + * Check if there are active subscribers to the named channel. This is helpful if + * the message you want to send might be expensive to prepare. + * + * This API is optional but helpful when trying to publish messages from very + * performance-sensitive code. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * if (diagnostics_channel.hasSubscribers('my-channel')) { + * // There are subscribers, prepare and publish message + * } + * ``` + * @since v15.1.0, v14.17.0 + * @param name The channel name + * @return If there are active subscribers + */ + function hasSubscribers(name: string | symbol): boolean; + /** + * This is the primary entry-point for anyone wanting to publish to a named + * channel. It produces a channel object which is optimized to reduce overhead at + * publish time as much as possible. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channel = diagnostics_channel.channel('my-channel'); + * ``` + * @since v15.1.0, v14.17.0 + * @param name The channel name + * @return The named channel object + */ + function channel(name: string | symbol): Channel; + type ChannelListener = (message: unknown, name: string | symbol) => void; + /** + * Register a message handler to subscribe to this channel. This message handler + * will be run synchronously whenever a message is published to the channel. Any + * errors thrown in the message handler will trigger an `'uncaughtException'`. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * diagnostics_channel.subscribe('my-channel', (message, name) => { + * // Received data + * }); + * ``` + * @since v18.7.0, v16.17.0 + * @param name The channel name + * @param onMessage The handler to receive channel messages + */ + function subscribe(name: string | symbol, onMessage: ChannelListener): void; + /** + * Remove a message handler previously registered to this channel with {@link subscribe}. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * function onMessage(message, name) { + * // Received data + * } + * + * diagnostics_channel.subscribe('my-channel', onMessage); + * + * diagnostics_channel.unsubscribe('my-channel', onMessage); + * ``` + * @since v18.7.0, v16.17.0 + * @param name The channel name + * @param onMessage The previous subscribed handler to remove + * @return `true` if the handler was found, `false` otherwise. + */ + function unsubscribe(name: string | symbol, onMessage: ChannelListener): boolean; + /** + * Creates a `TracingChannel` wrapper for the given `TracingChannel Channels`. If a name is given, the corresponding tracing + * channels will be created in the form of `tracing:${name}:${eventType}` where `eventType` corresponds to the types of `TracingChannel Channels`. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channelsByName = diagnostics_channel.tracingChannel('my-channel'); + * + * // or... + * + * const channelsByCollection = diagnostics_channel.tracingChannel({ + * start: diagnostics_channel.channel('tracing:my-channel:start'), + * end: diagnostics_channel.channel('tracing:my-channel:end'), + * asyncStart: diagnostics_channel.channel('tracing:my-channel:asyncStart'), + * asyncEnd: diagnostics_channel.channel('tracing:my-channel:asyncEnd'), + * error: diagnostics_channel.channel('tracing:my-channel:error'), + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param nameOrChannels Channel name or object containing all the `TracingChannel Channels` + * @return Collection of channels to trace with + */ + function tracingChannel< + StoreType = unknown, + ContextType extends object = StoreType extends object ? StoreType : object, + >(nameOrChannels: string | TracingChannelCollection): TracingChannel; + /** + * The class `Channel` represents an individual named channel within the data + * pipeline. It is used to track subscribers and to publish messages when there + * are subscribers present. It exists as a separate object to avoid channel + * lookups at publish time, enabling very fast publish speeds and allowing + * for heavy use while incurring very minimal cost. Channels are created with {@link channel}, constructing a channel directly + * with `new Channel(name)` is not supported. + * @since v15.1.0, v14.17.0 + */ + class Channel { + readonly name: string | symbol; + /** + * Check if there are active subscribers to this channel. This is helpful if + * the message you want to send might be expensive to prepare. + * + * This API is optional but helpful when trying to publish messages from very + * performance-sensitive code. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * if (channel.hasSubscribers) { + * // There are subscribers, prepare and publish message + * } + * ``` + * @since v15.1.0, v14.17.0 + */ + readonly hasSubscribers: boolean; + private constructor(name: string | symbol); + /** + * Publish a message to any subscribers to the channel. This will trigger + * message handlers synchronously so they will execute within the same context. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * channel.publish({ + * some: 'message', + * }); + * ``` + * @since v15.1.0, v14.17.0 + * @param message The message to send to the channel subscribers + */ + publish(message: unknown): void; + /** + * Register a message handler to subscribe to this channel. This message handler + * will be run synchronously whenever a message is published to the channel. Any + * errors thrown in the message handler will trigger an `'uncaughtException'`. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * channel.subscribe((message, name) => { + * // Received data + * }); + * ``` + * @since v15.1.0, v14.17.0 + * @deprecated Since v18.7.0,v16.17.0 - Use {@link subscribe(name, onMessage)} + * @param onMessage The handler to receive channel messages + */ + subscribe(onMessage: ChannelListener): void; + /** + * Remove a message handler previously registered to this channel with `channel.subscribe(onMessage)`. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * function onMessage(message, name) { + * // Received data + * } + * + * channel.subscribe(onMessage); + * + * channel.unsubscribe(onMessage); + * ``` + * @since v15.1.0, v14.17.0 + * @deprecated Since v18.7.0,v16.17.0 - Use {@link unsubscribe(name, onMessage)} + * @param onMessage The previous subscribed handler to remove + * @return `true` if the handler was found, `false` otherwise. + */ + unsubscribe(onMessage: ChannelListener): void; + /** + * When `channel.runStores(context, ...)` is called, the given context data + * will be applied to any store bound to the channel. If the store has already been + * bound the previous `transform` function will be replaced with the new one. + * The `transform` function may be omitted to set the given context data as the + * context directly. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * import { AsyncLocalStorage } from 'node:async_hooks'; + * + * const store = new AsyncLocalStorage(); + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * channel.bindStore(store, (data) => { + * return { data }; + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param store The store to which to bind the context data + * @param transform Transform context data before setting the store context + */ + bindStore(store: AsyncLocalStorage, transform?: (context: ContextType) => StoreType): void; + /** + * Remove a message handler previously registered to this channel with `channel.bindStore(store)`. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * import { AsyncLocalStorage } from 'node:async_hooks'; + * + * const store = new AsyncLocalStorage(); + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * channel.bindStore(store); + * channel.unbindStore(store); + * ``` + * @since v19.9.0 + * @experimental + * @param store The store to unbind from the channel. + * @return `true` if the store was found, `false` otherwise. + */ + unbindStore(store: any): void; + /** + * Applies the given data to any AsyncLocalStorage instances bound to the channel + * for the duration of the given function, then publishes to the channel within + * the scope of that data is applied to the stores. + * + * If a transform function was given to `channel.bindStore(store)` it will be + * applied to transform the message data before it becomes the context value for + * the store. The prior storage context is accessible from within the transform + * function in cases where context linking is required. + * + * The context applied to the store should be accessible in any async code which + * continues from execution which began during the given function, however + * there are some situations in which `context loss` may occur. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * import { AsyncLocalStorage } from 'node:async_hooks'; + * + * const store = new AsyncLocalStorage(); + * + * const channel = diagnostics_channel.channel('my-channel'); + * + * channel.bindStore(store, (message) => { + * const parent = store.getStore(); + * return new Span(message, parent); + * }); + * channel.runStores({ some: 'message' }, () => { + * store.getStore(); // Span({ some: 'message' }) + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param context Message to send to subscribers and bind to stores + * @param fn Handler to run within the entered storage context + * @param thisArg The receiver to be used for the function call. + * @param args Optional arguments to pass to the function. + */ + runStores(): void; + } + interface TracingChannelSubscribers { + start: (message: ContextType) => void; + end: ( + message: ContextType & { + error?: unknown; + result?: unknown; + }, + ) => void; + asyncStart: ( + message: ContextType & { + error?: unknown; + result?: unknown; + }, + ) => void; + asyncEnd: ( + message: ContextType & { + error?: unknown; + result?: unknown; + }, + ) => void; + error: ( + message: ContextType & { + error: unknown; + }, + ) => void; + } + interface TracingChannelCollection { + start: Channel; + end: Channel; + asyncStart: Channel; + asyncEnd: Channel; + error: Channel; + } + /** + * The class `TracingChannel` is a collection of `TracingChannel Channels` which + * together express a single traceable action. It is used to formalize and + * simplify the process of producing events for tracing application flow. {@link tracingChannel} is used to construct a `TracingChannel`. As with `Channel` it is recommended to create and reuse a + * single `TracingChannel` at the top-level of the file rather than creating them + * dynamically. + * @since v19.9.0 + * @experimental + */ + class TracingChannel implements TracingChannelCollection { + start: Channel; + end: Channel; + asyncStart: Channel; + asyncEnd: Channel; + error: Channel; + /** + * Helper to subscribe a collection of functions to the corresponding channels. + * This is the same as calling `channel.subscribe(onMessage)` on each channel + * individually. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channels = diagnostics_channel.tracingChannel('my-channel'); + * + * channels.subscribe({ + * start(message) { + * // Handle start message + * }, + * end(message) { + * // Handle end message + * }, + * asyncStart(message) { + * // Handle asyncStart message + * }, + * asyncEnd(message) { + * // Handle asyncEnd message + * }, + * error(message) { + * // Handle error message + * }, + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param subscribers Set of `TracingChannel Channels` subscribers + */ + subscribe(subscribers: TracingChannelSubscribers): void; + /** + * Helper to unsubscribe a collection of functions from the corresponding channels. + * This is the same as calling `channel.unsubscribe(onMessage)` on each channel + * individually. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channels = diagnostics_channel.tracingChannel('my-channel'); + * + * channels.unsubscribe({ + * start(message) { + * // Handle start message + * }, + * end(message) { + * // Handle end message + * }, + * asyncStart(message) { + * // Handle asyncStart message + * }, + * asyncEnd(message) { + * // Handle asyncEnd message + * }, + * error(message) { + * // Handle error message + * }, + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param subscribers Set of `TracingChannel Channels` subscribers + * @return `true` if all handlers were successfully unsubscribed, and `false` otherwise. + */ + unsubscribe(subscribers: TracingChannelSubscribers): void; + /** + * Trace a synchronous function call. This will always produce a `start event` and `end event` around the execution and may produce an `error event` if the given function throws an error. + * This will run the given function using `channel.runStores(context, ...)` on the `start` channel which ensures all + * events should have any bound stores set to match this trace context. + * + * To ensure only correct trace graphs are formed, events will only be published if subscribers are present prior to starting the trace. Subscriptions + * which are added after the trace begins will not receive future events from that trace, only future traces will be seen. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channels = diagnostics_channel.tracingChannel('my-channel'); + * + * channels.traceSync(() => { + * // Do something + * }, { + * some: 'thing', + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param fn Function to wrap a trace around + * @param context Shared object to correlate events through + * @param thisArg The receiver to be used for the function call + * @param args Optional arguments to pass to the function + * @return The return value of the given function + */ + traceSync( + fn: (this: ThisArg, ...args: Args) => any, + context?: ContextType, + thisArg?: ThisArg, + ...args: Args + ): void; + /** + * Trace a promise-returning function call. This will always produce a `start event` and `end event` around the synchronous portion of the + * function execution, and will produce an `asyncStart event` and `asyncEnd event` when a promise continuation is reached. It may also + * produce an `error event` if the given function throws an error or the + * returned promise rejects. This will run the given function using `channel.runStores(context, ...)` on the `start` channel which ensures all + * events should have any bound stores set to match this trace context. + * + * To ensure only correct trace graphs are formed, events will only be published if subscribers are present prior to starting the trace. Subscriptions + * which are added after the trace begins will not receive future events from that trace, only future traces will be seen. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channels = diagnostics_channel.tracingChannel('my-channel'); + * + * channels.tracePromise(async () => { + * // Do something + * }, { + * some: 'thing', + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param fn Promise-returning function to wrap a trace around + * @param context Shared object to correlate trace events through + * @param thisArg The receiver to be used for the function call + * @param args Optional arguments to pass to the function + * @return Chained from promise returned by the given function + */ + tracePromise( + fn: (this: ThisArg, ...args: Args) => Promise, + context?: ContextType, + thisArg?: ThisArg, + ...args: Args + ): void; + /** + * Trace a callback-receiving function call. This will always produce a `start event` and `end event` around the synchronous portion of the + * function execution, and will produce a `asyncStart event` and `asyncEnd event` around the callback execution. It may also produce an `error event` if the given function throws an error or + * the returned + * promise rejects. This will run the given function using `channel.runStores(context, ...)` on the `start` channel which ensures all + * events should have any bound stores set to match this trace context. + * + * The `position` will be -1 by default to indicate the final argument should + * be used as the callback. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * + * const channels = diagnostics_channel.tracingChannel('my-channel'); + * + * channels.traceCallback((arg1, callback) => { + * // Do something + * callback(null, 'result'); + * }, 1, { + * some: 'thing', + * }, thisArg, arg1, callback); + * ``` + * + * The callback will also be run with `channel.runStores(context, ...)` which + * enables context loss recovery in some cases. + * + * To ensure only correct trace graphs are formed, events will only be published if subscribers are present prior to starting the trace. Subscriptions + * which are added after the trace begins will not receive future events from that trace, only future traces will be seen. + * + * ```js + * import diagnostics_channel from 'node:diagnostics_channel'; + * import { AsyncLocalStorage } from 'node:async_hooks'; + * + * const channels = diagnostics_channel.tracingChannel('my-channel'); + * const myStore = new AsyncLocalStorage(); + * + * // The start channel sets the initial store data to something + * // and stores that store data value on the trace context object + * channels.start.bindStore(myStore, (data) => { + * const span = new Span(data); + * data.span = span; + * return span; + * }); + * + * // Then asyncStart can restore from that data it stored previously + * channels.asyncStart.bindStore(myStore, (data) => { + * return data.span; + * }); + * ``` + * @since v19.9.0 + * @experimental + * @param fn callback using function to wrap a trace around + * @param position Zero-indexed argument position of expected callback + * @param context Shared object to correlate trace events through + * @param thisArg The receiver to be used for the function call + * @param args Optional arguments to pass to the function + * @return The return value of the given function + */ + traceCallback any>( + fn: Fn, + position?: number, + context?: ContextType, + thisArg?: any, + ...args: Parameters + ): void; + } +} +declare module 'node:diagnostics_channel' { + export * from 'diagnostics_channel'; +} diff --git a/packages/node/src/integrations/fetch-breadcrumbs.ts b/packages/node/src/integrations/fetch-breadcrumbs.ts new file mode 100644 index 000000000000..e18473f3856c --- /dev/null +++ b/packages/node/src/integrations/fetch-breadcrumbs.ts @@ -0,0 +1,114 @@ +import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici'; +import { addBreadcrumb, defineIntegration } from '@sentry/core'; + +import type { Integration, IntegrationFn, SanitizedRequestData } from '@sentry/types'; +import { getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, parseUrl } from '@sentry/utils'; +import * as diagnosticsChannel from 'diagnostics_channel'; +import type { NodeClient } from '../sdk/client'; + +type OldIntegration = Integration & { breadcrumbsDisabled: boolean }; + +interface NodeFetchOptions { + /** + * Do not capture breadcrumbs for outgoing fetch requests to URLs where the given callback returns `true`. + */ + ignore?: (url: string) => boolean; +} + +const _fetchBreadcrumbsIntegration = ((options: NodeFetchOptions = {}) => { + function onRequestHeaders({ request, response }: { request: UndiciRequest; response: UndiciResponse }): void { + if (options.ignore) { + const url = getAbsoluteUrl(request.origin, request.path); + const shouldIgnore = options.ignore(url); + + if (shouldIgnore) { + return; + } + } + + addRequestBreadcrumb(request, response); + } + + return { + name: 'FetchBreadcrumbs', + setup: (client: NodeClient) => { + if (client.getOptions().fetchBreadcrumbs === false) { + return; + } + + const oldIntegration = client.getIntegrationByName('NodeFetch'); + if (oldIntegration?.breadcrumbsDisabled) { + return; + } + + diagnosticsChannel + .channel('undici:request:headers') + // eslint-disable-next-line deprecation/deprecation + .subscribe(onRequestHeaders as diagnosticsChannel.ChannelListener); + }, + }; +}) satisfies IntegrationFn; + +export const fetchBreadcrumbsIntegration = defineIntegration(_fetchBreadcrumbsIntegration); + +/** Add a breadcrumb for outgoing requests. */ +function addRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse): void { + const data = getBreadcrumbData(request); + const statusCode = response.statusCode; + const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); + + addBreadcrumb( + { + category: 'http', + data: { + status_code: statusCode, + ...data, + }, + type: 'http', + level, + }, + { + event: 'response', + request, + response, + }, + ); +} + +function getBreadcrumbData(request: UndiciRequest): Partial { + try { + const url = new URL(request.path, request.origin); + const parsedUrl = parseUrl(url.toString()); + + const data: Partial = { + url: getSanitizedUrlString(parsedUrl), + 'http.method': request.method || 'GET', + }; + + if (parsedUrl.search) { + data['http.query'] = parsedUrl.search; + } + if (parsedUrl.hash) { + data['http.fragment'] = parsedUrl.hash; + } + + return data; + } catch { + return {}; + } +} + +// 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.slice(1)}`; + } + + return `${url}${path}`; +} diff --git a/packages/node/src/integrations/node-fetch.ts b/packages/node/src/integrations/node-fetch.ts index 60abee504758..0bf2d276d1c6 100644 --- a/packages/node/src/integrations/node-fetch.ts +++ b/packages/node/src/integrations/node-fetch.ts @@ -1,25 +1,25 @@ import { context, propagation, trace } from '@opentelemetry/api'; -import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici'; import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici'; -import { - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - addBreadcrumb, - defineIntegration, - getCurrentScope, - hasTracingEnabled, -} from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, getCurrentScope, hasTracingEnabled } from '@sentry/core'; import { addOpenTelemetryInstrumentation, generateSpanContextForPropagationContext, getPropagationContextFromSpan, } from '@sentry/opentelemetry'; -import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; -import { getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, parseUrl } from '@sentry/utils'; - +import type { IntegrationFn } from '@sentry/types'; interface NodeFetchOptions { /** + * @deprecated Use `fetchBreadcrumbs` init option instead. + * ```js + * Sentry.init({ + * dsn: '__DSN__', + * fetchBreadcrumbs: false, + * }) + * ``` + * * Whether breadcrumbs should be recorded for requests. - * Defaults to true + * + * Defaults to `true` */ breadcrumbs?: boolean; @@ -31,7 +31,6 @@ interface NodeFetchOptions { } const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { - const _breadcrumbs = typeof options.breadcrumbs === 'undefined' ? true : options.breadcrumbs; const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; return { @@ -87,67 +86,17 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch', }; }, - responseHook: (_, { request, response }) => { - if (_breadcrumbs) { - addRequestBreadcrumb(request, response); - } - }, }); addOpenTelemetryInstrumentation(instrumentation); }, + // eslint-disable-next-line deprecation/deprecation + breadcrumbsDisabled: options.breadcrumbs === false, }; }) satisfies IntegrationFn; export const nativeNodeFetchIntegration = defineIntegration(_nativeNodeFetchIntegration); -/** Add a breadcrumb for outgoing requests. */ -function addRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse): void { - const data = getBreadcrumbData(request); - const statusCode = response.statusCode; - const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); - - addBreadcrumb( - { - category: 'http', - data: { - status_code: statusCode, - ...data, - }, - type: 'http', - level, - }, - { - event: 'response', - request, - response, - }, - ); -} - -function getBreadcrumbData(request: UndiciRequest): Partial { - try { - const url = new URL(request.path, request.origin); - const parsedUrl = parseUrl(url.toString()); - - const data: Partial = { - url: getSanitizedUrlString(parsedUrl), - 'http.method': request.method || 'GET', - }; - - if (parsedUrl.search) { - data['http.query'] = parsedUrl.search; - } - if (parsedUrl.hash) { - data['http.fragment'] = parsedUrl.hash; - } - - return data; - } catch { - return {}; - } -} - // Matching the behavior of the base instrumentation function getAbsoluteUrl(origin: string, path: string = '/'): string { const url = `${origin}`; diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index f6b08a394f8c..644abdbcb244 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -61,6 +61,13 @@ export interface BaseNodeOptions { /** Sets an optional server name (device name) */ serverName?: string; + /** + * Whether breadcrumbs should be recorded for fetch and undici requests. + * + * Defaults to `true`. + */ + fetchBreadcrumbs?: boolean; + /** * Include local variables with stack traces. * From c85c28505573c7eeca8bfb0e29794f4d89f9e263 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 18 Oct 2024 11:54:28 +0200 Subject: [PATCH 2/7] Include by default and export everywhere --- packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node/src/index.ts | 1 + packages/node/src/sdk/index.ts | 2 ++ packages/remix/src/index.server.ts | 1 + packages/solidstart/src/server/index.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + 8 files changed, 9 insertions(+) diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index dacb42643b99..249c585d2434 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -81,6 +81,7 @@ export { mysql2Integration, mysqlIntegration, nativeNodeFetchIntegration, + fetchBreadcrumbsIntegration, nestIntegration, NodeClient, nodeContextIntegration, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index ee70e8956c6f..e7972e3f3123 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -50,6 +50,7 @@ export { consoleIntegration, httpIntegration, nativeNodeFetchIntegration, + fetchBreadcrumbsIntegration, onUncaughtExceptionIntegration, onUnhandledRejectionIntegration, modulesIntegration, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index bda96f062966..138f95d2fe15 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -50,6 +50,7 @@ export { consoleIntegration, httpIntegration, nativeNodeFetchIntegration, + fetchBreadcrumbsIntegration, onUncaughtExceptionIntegration, onUnhandledRejectionIntegration, modulesIntegration, diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index f77ef88548f9..d77b7585030b 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -31,6 +31,7 @@ export { spotlightIntegration } from './integrations/spotlight'; export { genericPoolIntegration } from './integrations/tracing/genericPool'; export { dataloaderIntegration } from './integrations/tracing/dataloader'; export { amqplibIntegration } from './integrations/tracing/amqplib'; +export { fetchBreadcrumbsIntegration } from './integrations/fetch-breadcrumbs'; export { SentryContextManager } from './otel/contextManager'; export { generateInstrumentOnce } from './otel/instrument'; diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 7276e809875a..3b00526b5b48 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -30,6 +30,7 @@ import { consoleIntegration } from '../integrations/console'; import { nodeContextIntegration } from '../integrations/context'; import { contextLinesIntegration } from '../integrations/contextlines'; +import { fetchBreadcrumbsIntegration } from '../integrations/fetch-breadcrumbs'; import { httpIntegration } from '../integrations/http'; import { localVariablesIntegration } from '../integrations/local-variables'; import { modulesIntegration } from '../integrations/modules'; @@ -71,6 +72,7 @@ export function getDefaultIntegrationsWithoutPerformance(): Integration[] { contextLinesIntegration(), localVariablesIntegration(), nodeContextIntegration(), + fetchBreadcrumbsIntegration(), ...getCjsOnlyIntegrations(), ]; } diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 098bd1293080..3391741b1345 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -81,6 +81,7 @@ export { mysql2Integration, mysqlIntegration, nativeNodeFetchIntegration, + fetchBreadcrumbsIntegration, nestIntegration, NodeClient, nodeContextIntegration, diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts index d537ddd51e88..7b801f9319af 100644 --- a/packages/solidstart/src/server/index.ts +++ b/packages/solidstart/src/server/index.ts @@ -72,6 +72,7 @@ export { mysql2Integration, mysqlIntegration, nativeNodeFetchIntegration, + fetchBreadcrumbsIntegration, nestIntegration, NodeClient, nodeContextIntegration, diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 72b459e2fde3..58cd5360f883 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -74,6 +74,7 @@ export { mysql2Integration, mysqlIntegration, nativeNodeFetchIntegration, + fetchBreadcrumbsIntegration, nestIntegration, NodeClient, nodeContextIntegration, From a17da50bd6359a9018748ca618acd3d6a52f1e49 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 18 Oct 2024 12:57:53 +0200 Subject: [PATCH 3/7] Include diagnostic_channel types because they're experimental in older node --- packages/node/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/tsconfig.json b/packages/node/tsconfig.json index 8f38d240197e..6c7de7cb4e0f 100644 --- a/packages/node/tsconfig.json +++ b/packages/node/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../tsconfig.json", - "include": ["src/**/*"], + "include": ["src/**/*", "./src/integrations/diagnostic_channel.d.ts"], "compilerOptions": { "lib": ["es2018"], From 98b54e4f02b5c65f93988c339699eb6a5eac6b2b Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 18 Oct 2024 14:24:26 +0200 Subject: [PATCH 4/7] rename --- .../{diagnostics_channel.d.ts => diagnostic_channel.d.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/node/src/integrations/{diagnostics_channel.d.ts => diagnostic_channel.d.ts} (100%) diff --git a/packages/node/src/integrations/diagnostics_channel.d.ts b/packages/node/src/integrations/diagnostic_channel.d.ts similarity index 100% rename from packages/node/src/integrations/diagnostics_channel.d.ts rename to packages/node/src/integrations/diagnostic_channel.d.ts From 316a8e9434000db813900084d14f471e3b502287 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 18 Oct 2024 14:54:35 +0200 Subject: [PATCH 5/7] Fix test types --- packages/node/tsconfig.json | 2 +- packages/node/tsconfig.test.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/node/tsconfig.json b/packages/node/tsconfig.json index 6c7de7cb4e0f..8f38d240197e 100644 --- a/packages/node/tsconfig.json +++ b/packages/node/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../tsconfig.json", - "include": ["src/**/*", "./src/integrations/diagnostic_channel.d.ts"], + "include": ["src/**/*"], "compilerOptions": { "lib": ["es2018"], diff --git a/packages/node/tsconfig.test.json b/packages/node/tsconfig.test.json index 87f6afa06b86..b0c6b000999b 100644 --- a/packages/node/tsconfig.test.json +++ b/packages/node/tsconfig.test.json @@ -1,7 +1,7 @@ { "extends": "./tsconfig.json", - "include": ["test/**/*"], + "include": ["test/**/*", "./src/integrations/diagnostic_channel.d.ts"], "compilerOptions": { // should include all types from `./tsconfig.json` plus types for all test frameworks used From 508615fd72b84a539712b96430fb67eb162c9320 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 23 Oct 2024 21:11:25 +0200 Subject: [PATCH 6/7] Fix missing export --- .../node-exports-test-app/scripts/consistentExports.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts index a35bf4657c64..8352af9dac00 100644 --- a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts +++ b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts @@ -50,6 +50,8 @@ const DEPENDENTS: Dependent[] = [ ignoreExports: [ // not supported in bun: 'NodeClient', + // Doesn't have these events + 'fetchBreadcrumbsIntegration', ], }, { From ef5e713d151a432c3195c8722b375982aab69470 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 23 Oct 2024 21:25:27 +0200 Subject: [PATCH 7/7] Add tests and ensure legacy disable still works --- .../scenario-disabled-legacy.ts | 27 +++++++++ .../fetch-breadcrumbs/scenario-disabled.ts | 28 +++++++++ .../requests/fetch-breadcrumbs/test.ts | 58 +++++++++++++++++++ .../src/integrations/fetch-breadcrumbs.ts | 21 ++++--- 4 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled-legacy.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled-legacy.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled-legacy.ts new file mode 100644 index 000000000000..c98db3561d04 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled-legacy.ts @@ -0,0 +1,27 @@ +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', + tracePropagationTargets: [/\/v0/, 'v1'], + integrations: [Sentry.nativeNodeFetchIntegration({ breadcrumbs: false })], + transport: loggingTransport, + tracesSampleRate: 0.0, +}); + +async function run(): Promise { + Sentry.addBreadcrumb({ message: 'manual breadcrumb' }); + + // Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented + await new Promise(resolve => setTimeout(resolve, 100)); + await fetch(`${process.env.SERVER_URL}/api/v0`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v2`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v3`).then(res => res.text()); + + Sentry.captureException(new Error('foo')); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled.ts new file mode 100644 index 000000000000..1f16b59a654b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/scenario-disabled.ts @@ -0,0 +1,28 @@ +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', + tracePropagationTargets: [/\/v0/, 'v1'], + integrations: [], + transport: loggingTransport, + tracesSampleRate: 0.0, + fetchBreadcrumbs: false, +}); + +async function run(): Promise { + Sentry.addBreadcrumb({ message: 'manual breadcrumb' }); + + // Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented + await new Promise(resolve => setTimeout(resolve, 100)); + await fetch(`${process.env.SERVER_URL}/api/v0`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v2`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/v3`).then(res => res.text()); + + Sentry.captureException(new Error('foo')); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts index c0d783aaa594..fc22af8db99e 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts @@ -75,4 +75,62 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { .start(closeTestServer); }); }); + + test('outgoing fetch requests should not create breadcrumbs when disabled', done => { + createTestServer(done) + .start() + .then(([SERVER_URL, closeTestServer]) => { + createRunner(__dirname, 'scenario-disabled.ts') + .withEnv({ SERVER_URL }) + .ensureNoErrorOutput() + .expect({ + event: { + breadcrumbs: [ + { + message: 'manual breadcrumb', + timestamp: expect.any(Number), + }, + ], + exception: { + values: [ + { + type: 'Error', + value: 'foo', + }, + ], + }, + }, + }) + .start(closeTestServer); + }); + }); + + test('outgoing fetch requests should not create breadcrumbs when legacy disabled', done => { + createTestServer(done) + .start() + .then(([SERVER_URL, closeTestServer]) => { + createRunner(__dirname, 'scenario-disabled-legacy.ts') + .withEnv({ SERVER_URL }) + .ensureNoErrorOutput() + .expect({ + event: { + breadcrumbs: [ + { + message: 'manual breadcrumb', + timestamp: expect.any(Number), + }, + ], + exception: { + values: [ + { + type: 'Error', + value: 'foo', + }, + ], + }, + }, + }) + .start(closeTestServer); + }); + }); }); diff --git a/packages/node/src/integrations/fetch-breadcrumbs.ts b/packages/node/src/integrations/fetch-breadcrumbs.ts index e18473f3856c..7ade1be94d56 100644 --- a/packages/node/src/integrations/fetch-breadcrumbs.ts +++ b/packages/node/src/integrations/fetch-breadcrumbs.ts @@ -36,15 +36,18 @@ const _fetchBreadcrumbsIntegration = ((options: NodeFetchOptions = {}) => { return; } - const oldIntegration = client.getIntegrationByName('NodeFetch'); - if (oldIntegration?.breadcrumbsDisabled) { - return; - } - - diagnosticsChannel - .channel('undici:request:headers') - // eslint-disable-next-line deprecation/deprecation - .subscribe(onRequestHeaders as diagnosticsChannel.ChannelListener); + // We need to ensure all other integrations have been setup first + setImmediate(() => { + const oldIntegration = client.getIntegrationByName('NodeFetch'); + if (oldIntegration?.breadcrumbsDisabled) { + return; + } + + diagnosticsChannel + .channel('undici:request:headers') + // eslint-disable-next-line deprecation/deprecation + .subscribe(onRequestHeaders as diagnosticsChannel.ChannelListener); + }); }, }; }) satisfies IntegrationFn;