From 6408cff882f227bd5eb952d5d849f7c7b855d45f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 15 Oct 2024 09:36:52 +0000 Subject: [PATCH 1/4] fix(nextjs): Don't drop transactions for server components on navigations --- .../nextjs-app-dir/app/layout.tsx | 32 ++++++++++++++----- ...client-app-routing-instrumentation.test.ts | 1 + packages/nextjs/src/server/index.ts | 7 +--- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx index 15c3b9789ee3..006a01fcfa76 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/layout.tsx @@ -9,22 +9,34 @@ export default function Layout({ children }: { children: React.ReactNode }) {

Layout (/)

{children} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts index abfe9b323d0f..c3d5637d9f8a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts @@ -38,6 +38,7 @@ test('Creates a navigation transaction for app router routes', async ({ page }) }); const serverComponentTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => { + console.log('t', transactionEvent.transaction, transactionEvent.contexts?.trace); return ( // It seems to differ between Next.js versions whether the route is parameterized or not (transactionEvent?.transaction === 'GET /server-component/parameter/foo/bar/baz' || diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index ddf3668e27b3..0ac3fe190433 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -238,6 +238,7 @@ export function init(options: NodeOptions): NodeClient | undefined { // Filter transactions that we explicitly want to drop. if (event.contexts?.trace?.data?.[TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION]) { + console.log('Dropping transaction:', event.transaction, event.contexts.trace); return null; } @@ -307,12 +308,6 @@ export function init(options: NodeOptions): NodeClient | undefined { if (typeof method === 'string' && typeof route === 'string') { event.transaction = `${method} ${route.replace(/\/route$/, '')}`; event.contexts.trace.data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; - } else { - // If we cannot hoist the route (or rather parameterize the transaction) for BaseServer.handleRequest spans, - // we drop it because the chance that it is a low-quality transaction we don't want is pretty high. - // This is important in the case of edge-runtime where Next.js will also create unnecessary Node.js root - // spans, that are not parameterized. - return null; } } From bf504468d39f7525de5124be7ee93caaaa0b7d3b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 15 Oct 2024 09:37:23 +0000 Subject: [PATCH 2/4] Remove debugging --- .../tests/client-app-routing-instrumentation.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts index c3d5637d9f8a..abfe9b323d0f 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts @@ -38,7 +38,6 @@ test('Creates a navigation transaction for app router routes', async ({ page }) }); const serverComponentTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => { - console.log('t', transactionEvent.transaction, transactionEvent.contexts?.trace); return ( // It seems to differ between Next.js versions whether the route is parameterized or not (transactionEvent?.transaction === 'GET /server-component/parameter/foo/bar/baz' || From 2cd7e5b93e857054dcc50d483e8ad615c77d668a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 15 Oct 2024 09:55:26 +0000 Subject: [PATCH 3/4] Rm console log --- packages/nextjs/src/server/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 0ac3fe190433..168288e081fe 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -238,7 +238,6 @@ export function init(options: NodeOptions): NodeClient | undefined { // Filter transactions that we explicitly want to drop. if (event.contexts?.trace?.data?.[TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION]) { - console.log('Dropping transaction:', event.transaction, event.contexts.trace); return null; } From 7b607134bdd272f5613df4054e47c8f43b394f23 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 15 Oct 2024 09:58:40 +0000 Subject: [PATCH 4/4] Narrow test down --- .../nextjs-app-dir/tests/edge-route.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts index 0e2eb7417cee..810e76eaa690 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts @@ -4,7 +4,9 @@ import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; test('Should create a transaction for edge routes', async ({ request }) => { const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => { return ( - transactionEvent?.transaction === 'GET /api/edge-endpoint' && transactionEvent?.contexts?.trace?.status === 'ok' + transactionEvent?.transaction === 'GET /api/edge-endpoint' && + transactionEvent?.contexts?.trace?.status === 'ok' && + transactionEvent.contexts?.runtime?.name === 'vercel-edge' ); }); @@ -19,7 +21,6 @@ test('Should create a transaction for edge routes', async ({ request }) => { expect(edgerouteTransaction.contexts?.trace?.status).toBe('ok'); expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server'); - expect(edgerouteTransaction.contexts?.runtime?.name).toBe('vercel-edge'); expect(edgerouteTransaction.request?.headers?.['x-yeet']).toBe('test-value'); });