From 4c6e295eefe43e339e6ccc64a5a0b65b2627600d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 30 Jan 2025 10:27:37 +0100 Subject: [PATCH] feat(node): Extract Sentry-specific node-fetch instrumentation To allow users to opt-out of using the otel instrumentation. --- .../fetch-no-tracing-no-spans/scenario.ts | 24 ++ .../fetch-no-tracing-no-spans/test.ts | 47 ++++ .../SentryNodeFetchInstrumentation.ts | 266 ++++++++++++++++++ .../node/src/integrations/node-fetch/index.ts | 182 +++++------- .../node/src/integrations/node-fetch/types.ts | 47 ++++ packages/node/src/nodeVersion.ts | 1 + packages/opentelemetry/src/propagator.ts | 10 + 7 files changed, 463 insertions(+), 114 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/test.ts create mode 100644 packages/node/src/integrations/node-fetch/SentryNodeFetchInstrumentation.ts create mode 100644 packages/node/src/integrations/node-fetch/types.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/scenario.ts new file mode 100644 index 000000000000..8c2ed31ee1f8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/scenario.ts @@ -0,0 +1,24 @@ +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({ spans: false })], + transport: loggingTransport, +}); + +async function run(): Promise { + // 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-no-tracing-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/test.ts new file mode 100644 index 000000000000..98fc6bd38c52 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing-no-spans/test.ts @@ -0,0 +1,47 @@ +import { createRunner } from '../../../../utils/runner'; +import { createTestServer } from '../../../../utils/server'; + +describe('outgoing fetch', () => { + test('outgoing fetch requests are correctly instrumented with tracing & spans are disabled', done => { + expect.assertions(11); + + createTestServer(done) + .get('/api/v0', headers => { + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/)); + expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000'); + expect(headers['baggage']).toEqual(expect.any(String)); + }) + .get('/api/v1', headers => { + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/)); + expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000'); + expect(headers['baggage']).toEqual(expect.any(String)); + }) + .get('/api/v2', headers => { + expect(headers['baggage']).toBeUndefined(); + expect(headers['sentry-trace']).toBeUndefined(); + }) + .get('/api/v3', headers => { + expect(headers['baggage']).toBeUndefined(); + expect(headers['sentry-trace']).toBeUndefined(); + }) + .start() + .then(([SERVER_URL, closeTestServer]) => { + createRunner(__dirname, 'scenario.ts') + .withEnv({ SERVER_URL }) + .ensureNoErrorOutput() + .expect({ + event: { + exception: { + values: [ + { + type: 'Error', + value: 'foo', + }, + ], + }, + }, + }) + .start(closeTestServer); + }); + }); +}); diff --git a/packages/node/src/integrations/node-fetch/SentryNodeFetchInstrumentation.ts b/packages/node/src/integrations/node-fetch/SentryNodeFetchInstrumentation.ts new file mode 100644 index 000000000000..f6787a9126d2 --- /dev/null +++ b/packages/node/src/integrations/node-fetch/SentryNodeFetchInstrumentation.ts @@ -0,0 +1,266 @@ +import { VERSION } from '@opentelemetry/core'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase } from '@opentelemetry/instrumentation'; +import type { SanitizedRequestData } from '@sentry/core'; +import { LRUMap, getClient, getTraceData } from '@sentry/core'; +import { addBreadcrumb, getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, parseUrl } from '@sentry/core'; +import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry'; +import * as diagch from 'diagnostics_channel'; +import { NODE_MAJOR, NODE_MINOR } from '../../nodeVersion'; +import type { UndiciRequest, UndiciResponse } from './types'; + +export type SentryNodeFetchInstrumentationOptions = InstrumentationConfig & { + /** + * Whether breadcrumbs should be recorded for requests. + * + * @default `true` + */ + breadcrumbs?: boolean; + + /** + * Do not capture breadcrumbs for outgoing fetch requests to URLs where the given callback returns `true`. + * For the scope of this instrumentation, this callback only controls breadcrumb creation. + * The same option can be passed to the top-level httpIntegration where it controls both, breadcrumb and + * span creation. + * + * @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. + */ + ignoreOutgoingRequests?: (url: string) => boolean; +}; + +interface ListenerRecord { + name: string; + unsubscribe: () => void; +} + +/** + * This custom node-fetch instrumentation is used to instrument outgoing fetch requests. + * It does not emit any spans. + * + * The reason this is isolated from the OpenTelemetry instrumentation is that users may overwrite this, + * which would lead to Sentry not working as expected. + * + * This is heavily inspired & adapted from: + * https://github.com/open-telemetry/opentelemetry-js-contrib/blob/28e209a9da36bc4e1f8c2b0db7360170ed46cb80/plugins/node/instrumentation-undici/src/undici.ts + */ +export class SentryNodeFetchInstrumentation extends InstrumentationBase { + // Keep ref to avoid https://github.com/nodejs/node/issues/42170 bug and for + // unsubscribing. + private _channelSubs: Array; + private _propagationDecisionMap: LRUMap; + + public constructor(config: SentryNodeFetchInstrumentationOptions = {}) { + super('@sentry/instrumentation-node-fetch', VERSION, config); + this._channelSubs = []; + this._propagationDecisionMap = new LRUMap(100); + } + + /** No need to instrument files/modules. */ + public init(): void { + return undefined; + } + + /** Disable the instrumentation. */ + public disable(): void { + super.disable(); + this._channelSubs.forEach(sub => sub.unsubscribe()); + this._channelSubs = []; + } + + /** Enable the instrumentation. */ + public 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 + if (this._channelSubs.length > 0) { + return; + } + + this._subscribeToChannel('undici:request:create', this._onRequestCreated.bind(this)); + this._subscribeToChannel('undici:request:headers', this._onResponseHeaders.bind(this)); + } + + /** + * This method is called when a request is created. + * You can still mutate the request here before it is sent. + */ + private _onRequestCreated({ request }: { request: UndiciRequest }): void { + const config = this.getConfig(); + const enabled = config.enabled !== false; + + if (!enabled) { + return; + } + + // Add trace propagation headers + const url = getAbsoluteUrl(request.origin, request.path); + const _ignoreOutgoingRequests = config.ignoreOutgoingRequests; + const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url); + + if (shouldIgnore) { + return; + } + + // Manually add the trace headers, if it applies + // Note: We do not use `propagation.inject()` here, because our propagator relies on an active span + // Which we do not have in this case + // The propagator _may_ overwrite this, but this should be fine as it is the same data + const tracePropagationTargets = getClient()?.getOptions().tracePropagationTargets; + const addedHeaders = shouldPropagateTraceForUrl(url, tracePropagationTargets, this._propagationDecisionMap) + ? getTraceData() + : {}; + + // We do not want to overwrite existing headers here + // If the core UndiciInstrumentation is registered, it will already have set the headers + // We do not want to add any then + if (Array.isArray(request.headers)) { + const requestHeaders = request.headers; + Object.entries(addedHeaders) + .filter(([k]) => { + // If the header already exists, we do not want to set it again + return !requestHeaders.includes(`${k}:`); + }) + .forEach(headers => requestHeaders.push(...headers)); + } else { + const requestHeaders = request.headers; + request.headers += Object.entries(addedHeaders) + .filter(([k]) => { + // If the header already exists, we do not want to set it again + return !requestHeaders.includes(`${k}:`); + }) + .map(([k, v]) => `${k}: ${v}\r\n`) + .join(''); + } + } + + /** + * This method is called when a response is received. + */ + private _onResponseHeaders({ request, response }: { request: UndiciRequest; response: UndiciResponse }): void { + const config = this.getConfig(); + const enabled = config.enabled !== false; + + if (!enabled) { + return; + } + + const _breadcrumbs = config.breadcrumbs; + const breadCrumbsEnabled = typeof _breadcrumbs === 'undefined' ? true : _breadcrumbs; + + const _ignoreOutgoingRequests = config.ignoreOutgoingRequests; + const shouldCreateBreadcrumb = + typeof _ignoreOutgoingRequests === 'function' + ? !_ignoreOutgoingRequests(getAbsoluteUrl(request.origin, request.path)) + : true; + + if (breadCrumbsEnabled && shouldCreateBreadcrumb) { + addRequestBreadcrumb(request, response); + } + } + + /** Subscribe to a diagnostics channel. */ + private _subscribeToChannel( + diagnosticChannel: string, + onMessage: (message: unknown, name: string | symbol) => void, + ): void { + // `diagnostics_channel` had a ref counting bug until v18.19.0. + // https://github.com/nodejs/node/pull/47520 + const useNewSubscribe = NODE_MAJOR > 18 || (NODE_MAJOR === 18 && NODE_MINOR >= 19); + + let unsubscribe: () => void; + if (useNewSubscribe) { + diagch.subscribe?.(diagnosticChannel, onMessage); + unsubscribe = () => diagch.unsubscribe?.(diagnosticChannel, onMessage); + } else { + const channel = diagch.channel(diagnosticChannel); + channel.subscribe(onMessage); + unsubscribe = () => channel.unsubscribe(onMessage); + } + + this._channelSubs.push({ + name: diagnosticChannel, + unsubscribe, + }); + } +} + +/** 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 = getAbsoluteUrl(request.origin, request.path); + const parsedUrl = parseUrl(url); + + 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 {}; + } +} + +function getAbsoluteUrl(origin: string, path: string = '/'): string { + try { + const url = new URL(path, origin); + return url.toString(); + } catch { + // fallback: Construct it on our own + 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/index.ts b/packages/node/src/integrations/node-fetch/index.ts index a4678be7239a..dc0df9b5ef57 100644 --- a/packages/node/src/integrations/node-fetch/index.ts +++ b/packages/node/src/integrations/node-fetch/index.ts @@ -1,20 +1,14 @@ -import { registerInstrumentations } from '@opentelemetry/instrumentation'; -import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici'; +import type { UndiciInstrumentationConfig } from '@opentelemetry/instrumentation-undici'; import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici'; -import type { IntegrationFn, SanitizedRequestData } from '@sentry/core'; -import { - LRUMap, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - addBreadcrumb, - defineIntegration, - getBreadcrumbLogLevelFromHttpStatusCode, - getClient, - getSanitizedUrlString, - getTraceData, - hasTracingEnabled, - parseUrl, -} from '@sentry/core'; -import { shouldPropagateTraceForUrl } from '@sentry/opentelemetry'; +import type { IntegrationFn } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, getClient } from '@sentry/core'; +import { generateInstrumentOnce } from '../../otel/instrument'; +import type { NodeClient } from '../../sdk/client'; +import type { NodeClientOptions } from '../../types'; +import type { SentryNodeFetchInstrumentationOptions } from './SentryNodeFetchInstrumentation'; +import { SentryNodeFetchInstrumentation } from './SentryNodeFetchInstrumentation'; + +const INTEGRATION_NAME = 'NodeFetch'; interface NodeFetchOptions { /** @@ -23,6 +17,15 @@ interface NodeFetchOptions { */ 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; + /** * 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. @@ -30,114 +33,39 @@ interface NodeFetchOptions { ignoreOutgoingRequests?: (url: string) => boolean; } -const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { - const _breadcrumbs = typeof options.breadcrumbs === 'undefined' ? true : options.breadcrumbs; - const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; +const instrumentOtelNodeFetch = generateInstrumentOnce(INTEGRATION_NAME, config => { + return new UndiciInstrumentation(config); +}); + +const instrumentSentryNodeFetch = generateInstrumentOnce( + `${INTEGRATION_NAME}.sentry`, + config => { + return new SentryNodeFetchInstrumentation(config); + }, +); +const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { return { name: 'NodeFetch', setupOnce() { - const propagationDecisionMap = new LRUMap(100); - - const instrumentation = new UndiciInstrumentation({ - requireParentforSpans: false, - ignoreRequestHook: request => { - const url = getAbsoluteUrl(request.origin, request.path); - const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url); - - if (shouldIgnore) { - return true; - } - - // If tracing is disabled, we still want to propagate traces - // So we do that manually here, matching what the instrumentation does otherwise - if (!hasTracingEnabled()) { - const tracePropagationTargets = getClient()?.getOptions().tracePropagationTargets; - const addedHeaders = shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap) - ? getTraceData() - : {}; - - const requestHeaders = request.headers; - if (Array.isArray(requestHeaders)) { - Object.entries(addedHeaders).forEach(headers => requestHeaders.push(...headers)); - } else { - request.headers += Object.entries(addedHeaders) - .map(([k, v]) => `${k}: ${v}\r\n`) - .join(''); - } - - // Prevent starting a span for this request - return true; - } - - return false; - }, - startSpanHook: () => { - return { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch', - }; - }, - responseHook: (_, { request, response }) => { - if (_breadcrumbs) { - addRequestBreadcrumb(request, response); - } - }, - }); - - registerInstrumentations({ instrumentations: [instrumentation] }); + const instrumentSpans = _shouldInstrumentSpans(options, getClient()?.getOptions()); + + // This is the "regular" OTEL instrumentation that emits spans + if (instrumentSpans) { + const instrumentationConfig = getConfigWithDefaults(options); + instrumentOtelNodeFetch(instrumentationConfig); + } + + // 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 + instrumentSentryNodeFetch(options); }, }; }) 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}`; @@ -152,3 +80,29 @@ function getAbsoluteUrl(origin: string, path: string = '/'): string { return `${url}${path}`; } + +function _shouldInstrumentSpans(options: NodeFetchOptions, clientOptions: Partial = {}): boolean { + // If `spans` is passed in, it takes precedence + // Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true` + return typeof options.spans === 'boolean' ? options.spans : !clientOptions.skipOpenTelemetrySetup; +} + +function getConfigWithDefaults(options: Partial = {}): UndiciInstrumentationConfig { + const instrumentationConfig = { + requireParentforSpans: false, + ignoreRequestHook: request => { + const url = getAbsoluteUrl(request.origin, request.path); + const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; + const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url); + + return !!shouldIgnore; + }, + startSpanHook: () => { + return { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch', + }; + }, + } satisfies UndiciInstrumentationConfig; + + return instrumentationConfig; +} diff --git a/packages/node/src/integrations/node-fetch/types.ts b/packages/node/src/integrations/node-fetch/types.ts new file mode 100644 index 000000000000..0139dadde413 --- /dev/null +++ b/packages/node/src/integrations/node-fetch/types.ts @@ -0,0 +1,47 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Vendored from https://github.com/open-telemetry/opentelemetry-js-contrib/blob/28e209a9da36bc4e1f8c2b0db7360170ed46cb80/plugins/node/instrumentation-undici/src/types.ts + */ + +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 v6 + */ + headers: 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; + body: unknown; +} + +export interface UndiciResponse { + headers: Buffer[]; + statusCode: number; + statusText: string; +} diff --git a/packages/node/src/nodeVersion.ts b/packages/node/src/nodeVersion.ts index 1ec745743620..86c761543de7 100644 --- a/packages/node/src/nodeVersion.ts +++ b/packages/node/src/nodeVersion.ts @@ -2,3 +2,4 @@ import { parseSemver } from '@sentry/core'; export const NODE_VERSION = parseSemver(process.versions.node) as { major: number; minor: number; patch: number }; export const NODE_MAJOR = NODE_VERSION.major; +export const NODE_MINOR = NODE_VERSION.minor; diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index f92331d7739f..024b30255f35 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -264,6 +264,16 @@ function getExistingBaggage(carrier: unknown): string | undefined { } } +/** Try to get the existing sentry-trace header so we can merge this in. */ +function getExistingSentryTrace(carrier: unknown): string | undefined { + try { + const sentryTrace = (carrier as Record)[SENTRY_TRACE_HEADER]; + return Array.isArray(sentryTrace) ? sentryTrace.join(',') : sentryTrace; + } catch { + return undefined; + } +} + /** * It is pretty tricky to get access to the outgoing request URL of a request in the propagator. * As we only have access to the context of the span to be sent and the carrier (=headers),