ref(node): Clean up Undici options (#7646)

Author: AbhiPrasad

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<UndiciOptions> = {}) {
     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,20 +103,14 @@ export class Undici implements Integration {
         return;
       }
 
-      const hub = getCurrentHub();
       const client = hub.getClient<NodeClient>();
       const scope = hub.getScope();
 
       const activeSpan = scope.getSpan();
 
       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<string, unknown> = {};
@@ -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: {

packages/node/src/types.ts

@@ -32,27 +32,27 @@ 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'],
    * });
    * ```
    */
   tracePropagationTargets?: TracePropagationTargets;
 
   // 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({

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<UndiciOptions>): () => 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 };
+      }
+    }
+  };
+}