feat(node): Drop 401-404 and 3xx status code spans by default (#16972)

In addition to 404, we now also drop 401, 402, 403 and 3xx (e.g. 301
MOVED_PERMANTENTLY) spans by default.

These are usually not helpful.

Noticed this in some react-router E2E test where 301 spans were messing
with the test, but these should not even be captured really.

---------

Co-authored-by: Andrei Borza <[email protected]>

Author: 2 people

dev-packages/e2e-tests/test-applications/nextjs-14/app/generation-functions/with-redirect/page.tsx

@@ -108,20 +108,3 @@ test('Should send a transaction and an error event for a faulty generateViewport
 
   expect(errorEvent.transaction).toBe('Page.generateViewport (/generation-functions)');
 });
-
-test('Should send a transaction event with correct status for a generateMetadata() function invocation with redirect()', async ({
-  page,
-}) => {
-  const testTitle = 'redirect-foobar';
-
-  const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
-    return (
-      transactionEvent.contexts?.trace?.data?.['http.target'] ===
-      `/generation-functions/with-redirect?metadataTitle=${testTitle}`
-    );
-  });
-
-  await page.goto(`/generation-functions/with-redirect?metadataTitle=${testTitle}`);
-
-  expect((await transactionPromise).contexts?.trace?.status).toBe('ok');
-});

dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts

@@ -5,5 +5,5 @@ export async function GET() {
 }
 
 export async function POST() {
-  return NextResponse.json({ name: 'John Doe' }, { status: 403 });
+  return NextResponse.json({ name: 'John Doe' }, { status: 400 });
 }

dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/route.ts

@@ -28,7 +28,7 @@ test('Should create a transaction for route handlers and correctly set span stat
 
   const routehandlerTransaction = await routehandlerTransactionPromise;
 
-  expect(routehandlerTransaction.contexts?.trace?.status).toBe('permission_denied');
+  expect(routehandlerTransaction.contexts?.trace?.status).toBe('invalid_argument');
   expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server');
 });
 

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts

@@ -143,8 +143,8 @@ export const appRouter = t.router({
     .mutation(() => {
       throw new Error('I crashed in a trpc handler');
     }),
-  unauthorized: procedure.mutation(() => {
-    throw new TRPCError({ code: 'UNAUTHORIZED', cause: new Error('Unauthorized') });
+  badRequest: procedure.mutation(() => {
+    throw new TRPCError({ code: 'BAD_REQUEST', cause: new Error('Bad Request') });
   }),
 });
 

dev-packages/e2e-tests/test-applications/node-express/src/app.ts

@@ -109,12 +109,12 @@ test('Should record transaction and error for a trpc handler that returns a stat
   const transactionEventPromise = waitForTransaction('node-express', transactionEvent => {
     return (
       transactionEvent.transaction === 'POST /trpc' &&
-      !!transactionEvent.spans?.find(span => span.description === 'trpc/unauthorized')
+      !!transactionEvent.spans?.find(span => span.description === 'trpc/badRequest')
     );
   });
 
   const errorEventPromise = waitForError('node-express', errorEvent => {
-    return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('Unauthorized'));
+    return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('Bad Request'));
   });
 
   const trpcClient = createTRPCProxyClient<AppRouter>({
@@ -125,7 +125,7 @@ test('Should record transaction and error for a trpc handler that returns a stat
     ],
   });
 
-  await expect(trpcClient.unauthorized.mutate()).rejects.toBeDefined();
+  await expect(trpcClient.badRequest.mutate()).rejects.toBeDefined();
 
   await expect(transactionEventPromise).resolves.toBeDefined();
   await expect(errorEventPromise).resolves.toBeDefined();

dev-packages/e2e-tests/test-applications/node-express/tests/trpc.test.ts

@@ -8,6 +8,18 @@ app.get('/', (_req, res) => {
   res.send({ response: 'response 0' });
 });
 
+app.get('/401', (_req, res) => {
+  res.status(401).send({ response: 'response 401' });
+});
+
+app.get('/402', (_req, res) => {
+  res.status(402).send({ response: 'response 402' });
+});
+
+app.get('/403', (_req, res) => {
+  res.status(403).send({ response: 'response 403' });
+});
+
 app.get('/499', (_req, res) => {
   res.status(499).send({ response: 'response 499' });
 });

dev-packages/node-integration-tests/suites/express-v5/tracing/scenario-filterStatusCode.mjs

@@ -15,6 +15,18 @@ app.get('/', (_req, res) => {
   res.send({ response: 'response 0' });
 });
 
+app.get('/401', (_req, res) => {
+  res.status(401).send({ response: 'response 401' });
+});
+
+app.get('/402', (_req, res) => {
+  res.status(402).send({ response: 'response 402' });
+});
+
+app.get('/403', (_req, res) => {
+  res.status(403).send({ response: 'response 403' });
+});
+
 app.get('/test/express', (_req, res) => {
   res.send({ response: 'response 1' });
 });

dev-packages/node-integration-tests/suites/express-v5/tracing/scenario.mjs

@@ -89,16 +89,16 @@ describe('express v5 tracing', () => {
       await runner.completed();
     });
 
-    test('ignores 404 routes by default', async () => {
+    test.each(['/401', '/402', '/403', '/does-not-exist'])('ignores %s route by default', async (url: string) => {
       const runner = createRunner()
         .expect({
-          // No transaction is sent for the 404 route
+          // No transaction is sent for the 401, 402, 403, 404 routes
           transaction: {
             transaction: 'GET /',
           },
         })
         .start();
-      runner.makeRequest('get', '/does-not-exist', { expectError: true });
+      runner.makeRequest('get', url, { expectError: true });
       runner.makeRequest('get', '/');
       await runner.completed();
     });
@@ -284,33 +284,41 @@ describe('express v5 tracing', () => {
       'scenario-filterStatusCode.mjs',
       'instrument-filterStatusCode.mjs',
       (createRunner, test) => {
-        // We opt-out of the default 404 filtering in order to test how 404 spans are handled
-        test('handles 404 route correctly', async () => {
-          const runner = createRunner()
-            .expect({
-              transaction: {
-                transaction: 'GET /does-not-exist',
-                contexts: {
-                  trace: {
-                    span_id: expect.stringMatching(/[a-f0-9]{16}/),
-                    trace_id: expect.stringMatching(/[a-f0-9]{32}/),
-                    data: {
-                      'http.response.status_code': 404,
-                      url: expect.stringMatching(/\/does-not-exist$/),
-                      'http.method': 'GET',
-                      'http.url': expect.stringMatching(/\/does-not-exist$/),
-                      'http.target': '/does-not-exist',
+        // We opt-out of the default [401, 404] fitering in order to test how these spans are handled
+        test.each([
+          { status_code: 401, url: '/401', status: 'unauthenticated' },
+          { status_code: 402, url: '/402', status: 'invalid_argument' },
+          { status_code: 403, url: '/403', status: 'permission_denied' },
+          { status_code: 404, url: '/does-not-exist', status: 'not_found' },
+        ])(
+          'handles %s route correctly',
+          async ({ status_code, url, status }: { status_code: number; url: string; status: string }) => {
+            const runner = createRunner()
+              .expect({
+                transaction: {
+                  transaction: `GET ${url}`,
+                  contexts: {
+                    trace: {
+                      span_id: expect.stringMatching(/[a-f0-9]{16}/),
+                      trace_id: expect.stringMatching(/[a-f0-9]{32}/),
+                      data: {
+                        'http.response.status_code': status_code,
+                        url: expect.stringMatching(url),
+                        'http.method': 'GET',
+                        'http.url': expect.stringMatching(url),
+                        'http.target': url,
+                      },
+                      op: 'http.server',
+                      status,
                     },
-                    op: 'http.server',
-                    status: 'not_found',
                   },
                 },
-              },
-            })
-            .start();
-          runner.makeRequest('get', '/does-not-exist', { expectError: true });
-          await runner.completed();
-        });
+              })
+              .start();
+            runner.makeRequest('get', url, { expectError: true });
+            await runner.completed();
+          },
+        );
 
         test('filters defined status codes', async () => {
           const runner = createRunner()

dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts

@@ -8,6 +8,18 @@ app.get('/', (_req, res) => {
   res.send({ response: 'response 0' });
 });
 
+app.get('/401', (_req, res) => {
+  res.status(401).send({ response: 'response 401' });
+});
+
+app.get('/402', (_req, res) => {
+  res.status(402).send({ response: 'response 402' });
+});
+
+app.get('/403', (_req, res) => {
+  res.status(403).send({ response: 'response 403' });
+});
+
 app.get('/499', (_req, res) => {
   res.status(499).send({ response: 'response 499' });
 });