From da4d96987324cc6307f756d66fb364bda821df4c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 29 Mar 2023 14:55:04 +0200 Subject: [PATCH 1/2] ref(node): Clean up Undici options --- .../node/src/integrations/undici/index.ts | 44 ++++++++++-------- packages/node/src/types.ts | 20 ++++---- .../node/test/integrations/undici.test.ts | 46 +++++++++++++++++-- 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index 11549b5d75dc..80c48a996f71 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -27,12 +27,13 @@ export interface UndiciOptions { * Defaults to true */ breadcrumbs: boolean; + /** + * Function determining whether or not to create spans to track outgoing requests to the given URL. + * By default, spans will be created for all outgoing requests. + */ + shouldCreateSpanForRequest: (url: string) => boolean; } -const DEFAULT_UNDICI_OPTIONS: UndiciOptions = { - breadcrumbs: true, -}; - // Please note that you cannot use `console.log` to debug the callbacks registered to the `diagnostics_channel` API. // To debug, you can use `writeFileSync` to write to a file: // https://nodejs.org/api/async_hooks.html#printing-in-asynchook-callbacks @@ -60,8 +61,8 @@ export class Undici implements Integration { public constructor(_options: Partial = {}) { this._options = { - ...DEFAULT_UNDICI_OPTIONS, - ..._options, + breadcrumbs: _options.breadcrumbs === undefined ? true : _options.breadcrumbs, + shouldCreateSpanForRequest: _options.shouldCreateSpanForRequest || (() => true), }; } @@ -88,6 +89,11 @@ export class Undici implements Integration { // https://github.com/nodejs/undici/blob/e6fc80f809d1217814c044f52ed40ef13f21e43c/docs/api/DiagnosticsChannel.md ds.subscribe(ChannelName.RequestCreate, message => { + const hub = getCurrentHub(); + if (!hub.getIntegration(Undici)) { + return; + } + const { request } = message as RequestCreateMessage; const url = new URL(request.path, request.origin); @@ -97,7 +103,6 @@ export class Undici implements Integration { return; } - const hub = getCurrentHub(); const client = hub.getClient(); const scope = hub.getScope(); @@ -105,12 +110,7 @@ export class Undici implements Integration { if (activeSpan && client) { const clientOptions = client.getOptions(); - - // eslint-disable-next-line deprecation/deprecation - const shouldCreateSpan = clientOptions.shouldCreateSpanForRequest - ? // eslint-disable-next-line deprecation/deprecation - clientOptions.shouldCreateSpanForRequest(stringUrl) - : true; + const shouldCreateSpan = this._options.shouldCreateSpanForRequest(stringUrl); if (shouldCreateSpan) { const data: Record = {}; @@ -129,10 +129,8 @@ export class Undici implements Integration { }); request.__sentry__ = span; - // eslint-disable-next-line deprecation/deprecation const shouldPropagate = clientOptions.tracePropagationTargets - ? // eslint-disable-next-line deprecation/deprecation - stringMatchesSomePattern(stringUrl, clientOptions.tracePropagationTargets) + ? stringMatchesSomePattern(stringUrl, clientOptions.tracePropagationTargets) : true; if (shouldPropagate) { @@ -150,6 +148,11 @@ export class Undici implements Integration { }); ds.subscribe(ChannelName.RequestEnd, message => { + const hub = getCurrentHub(); + if (!hub.getIntegration(Undici)) { + return; + } + const { request, response } = message as RequestEndMessage; const url = new URL(request.path, request.origin); @@ -166,7 +169,7 @@ export class Undici implements Integration { } if (this._options.breadcrumbs) { - getCurrentHub().addBreadcrumb( + hub.addBreadcrumb( { category: 'http', data: { @@ -186,6 +189,11 @@ export class Undici implements Integration { }); ds.subscribe(ChannelName.RequestError, message => { + const hub = getCurrentHub(); + if (!hub.getIntegration(Undici)) { + return; + } + const { request } = message as RequestErrorMessage; const url = new URL(request.path, request.origin); @@ -202,7 +210,7 @@ export class Undici implements Integration { } if (this._options.breadcrumbs) { - getCurrentHub().addBreadcrumb( + hub.addBreadcrumb( { category: 'http', data: { diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index dd4713e97952..3e464d1c6457 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -32,19 +32,19 @@ export interface BaseNodeOptions { */ includeLocalVariables?: boolean; - // TODO (v8): Remove this in v8 /** - * @deprecated Moved to constructor options of the `Http` integration. + * List of strings/regex controlling to which outgoing requests + * the SDK will attach tracing headers. + * + * By default the SDK will attach those headers to all outgoing + * requests. If this option is provided, the SDK will match the + * request URL of outgoing requests against the items in this + * array, and only attach tracing headers if a match was found. + * * @example * ```js * Sentry.init({ - * integrations: [ - * new Sentry.Integrations.Http({ - * tracing: { - * tracePropagationTargets: ['api.site.com'], - * } - * }); - * ], + * tracePropagationTargets: ['api.site.com'], * }); * ``` */ @@ -52,7 +52,7 @@ export interface BaseNodeOptions { // TODO (v8): Remove this in v8 /** - * @deprecated Moved to constructor options of the `Http` integration. + * @deprecated Moved to constructor options of the `Http` and `Undici` integration. * @example * ```js * Sentry.init({ diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index d16a18497e0a..ecea01031af2 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -4,6 +4,7 @@ import * as http from 'http'; import type { fetch as FetchType } from 'undici'; import { NodeClient } from '../../src/client'; +import type { UndiciOptions } from '../../src/integrations/undici'; import { Undici } from '../../src/integrations/undici'; import { getDefaultNodeClientOptions } from '../helper/node-client-options'; import { conditionalTest } from '../utils'; @@ -150,8 +151,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; hub.getScope().setSpan(transaction); - const client = new NodeClient({ ...DEFAULT_OPTIONS, shouldCreateSpanForRequest: url => url.includes('yes') }); - hub.bindClient(client); + const undoPatch = patchUndici(hub, { shouldCreateSpanForRequest: url => url.includes('yes') }); await fetch('http://localhost:18099/no', { method: 'POST' }); @@ -160,6 +160,8 @@ conditionalTest({ min: 16 })('Undici integration', () => { await fetch('http://localhost:18099/yes', { method: 'POST' }); expect(transaction.spanRecorder?.spans.length).toBe(2); + + undoPatch(); }); it('attaches the sentry trace and baggage headers', async () => { @@ -181,8 +183,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction; hub.getScope().setSpan(transaction); - const client = new NodeClient({ ...DEFAULT_OPTIONS, shouldCreateSpanForRequest: url => url.includes('yes') }); - hub.bindClient(client); + const undoPatch = patchUndici(hub, { shouldCreateSpanForRequest: url => url.includes('yes') }); await fetch('http://localhost:18099/no', { method: 'POST' }); @@ -193,6 +194,8 @@ conditionalTest({ min: 16 })('Undici integration', () => { expect(requestHeaders['sentry-trace']).toBeDefined(); expect(requestHeaders['baggage']).toBeDefined(); + + undoPatch(); }); it('uses tracePropagationTargets', async () => { @@ -270,6 +273,16 @@ conditionalTest({ min: 16 })('Undici integration', () => { // ignore } }); + + it('does not add a breadcrumb if disabled', async () => { + expect.assertions(0); + + const undoPatch = patchUndici(hub, { breadcrumbs: false }); + + await fetch('http://localhost:18099', { method: 'POST' }); + + undoPatch(); + }); }); interface TestServerOptions { @@ -318,3 +331,28 @@ function setupTestServer() { testServer?.on('listening', resolve); }); } + +function patchUndici(hub: Hub, userOptions: Partial): () => void { + let options: any = {}; + const client = hub.getClient(); + if (client) { + const undici = client.getIntegration(Undici); + if (undici) { + // @ts-ignore need to access private property + options = { ...undici._options }; + // @ts-ignore need to access private property + undici._options = Object.assign(undici._options, userOptions); + } + } + + return () => { + const client = hub.getClient(); + if (client) { + const undici = client.getIntegration(Undici); + if (undici) { + // @ts-ignore need to access private property + undici._options = { ...options }; + } + } + }; +} From fba636d4c4c9e06b0f2a3ecfe51f0c496d615bb5 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 29 Mar 2023 15:02:58 +0200 Subject: [PATCH 2/2] feat(nextjs): Add Undici integration automatically --- packages/nextjs/src/server/index.ts | 2 ++ packages/nextjs/test/serverSdk.test.ts | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 386c2ff2e4d7..1eeafddb4251 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -174,6 +174,8 @@ function addServerIntegrations(options: NodeOptions): void { }); } + integrations = addOrUpdateIntegration(new Integrations.Undici(), integrations); + options.integrations = integrations; } diff --git a/packages/nextjs/test/serverSdk.test.ts b/packages/nextjs/test/serverSdk.test.ts index 21fc17dfe185..bb55d9f6e184 100644 --- a/packages/nextjs/test/serverSdk.test.ts +++ b/packages/nextjs/test/serverSdk.test.ts @@ -161,6 +161,15 @@ describe('Server init()', () => { expect(consoleIntegration).toBeDefined(); }); + it('adds the Undici integration', () => { + init({}); + + const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions; + const undiciIntegration = findIntegrationByName(nodeInitOptions.integrations, 'Undici'); + + expect(undiciIntegration).toBeDefined(); + }); + describe('`Http` integration', () => { it('adds `Http` integration with tracing enabled if `tracesSampleRate` is set', () => { init({ tracesSampleRate: 1.0 });