Add tests and ensure legacy disable still works

Author: timfish

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://[email protected]/1337',
+  release: '1.0',
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  integrations: [Sentry.nativeNodeFetchIntegration({ breadcrumbs: false })],
+  transport: loggingTransport,
+  tracesSampleRate: 0.0,
+});
+
+async function run(): Promise<void> {
+  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();

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://[email protected]/1337',
+  release: '1.0',
+  tracePropagationTargets: [/\/v0/, 'v1'],
+  integrations: [],
+  transport: loggingTransport,
+  tracesSampleRate: 0.0,
+  fetchBreadcrumbs: false,
+});
+
+async function run(): Promise<void> {
+  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();

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);
+      });
+  });
 });

packages/node/src/integrations/fetch-breadcrumbs.ts

@@ -36,15 +36,18 @@ const _fetchBreadcrumbsIntegration = ((options: NodeFetchOptions = {}) => {
         return;
       }
 
-      const oldIntegration = client.getIntegrationByName<OldIntegration>('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<OldIntegration>('NodeFetch');
+        if (oldIntegration?.breadcrumbsDisabled) {
+          return;
+        }
+
+        diagnosticsChannel
+          .channel('undici:request:headers')
+          // eslint-disable-next-line deprecation/deprecation
+          .subscribe(onRequestHeaders as diagnosticsChannel.ChannelListener);
+      });
     },
   };
 }) satisfies IntegrationFn;