Skip to content

Commit

Permalink
fix(nextjs): Don't drop transactions for server components on navigat…
Browse files Browse the repository at this point in the history
…ions (#13980)

We were dropping a few too many transactions.
  • Loading branch information
lforst authored Oct 15, 2024
1 parent 88656c2 commit e979c75
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,49 @@ export default function Layout({ children }: { children: React.ReactNode }) {
<h1>Layout (/)</h1>
<ul>
<li>
<Link href="/">/</Link>
<Link href="/" prefetch={false}>
/
</Link>
</li>
<li>
<Link href="/client-component">/client-component</Link>
<Link href="/client-component" prefetch={false}>
/client-component
</Link>
</li>
<li>
<Link href="/client-component/parameter/42">/client-component/parameter/42</Link>
<Link href="/client-component/parameter/42" prefetch={false}>
/client-component/parameter/42
</Link>
</li>
<li>
<Link href="/client-component/parameter/foo/bar/baz">/client-component/parameter/foo/bar/baz</Link>
<Link href="/client-component/parameter/foo/bar/baz" prefetch={false}>
/client-component/parameter/foo/bar/baz
</Link>
</li>
<li>
<Link href="/server-component">/server-component</Link>
<Link href="/server-component" prefetch={false}>
/server-component
</Link>
</li>
<li>
<Link href="/server-component/parameter/42">/server-component/parameter/42</Link>
<Link href="/server-component/parameter/42" prefetch={false}>
/server-component/parameter/42
</Link>
</li>
<li>
<Link href="/server-component/parameter/foo/bar/baz" prefetch={false}>
/server-component/parameter/foo/bar/baz
</Link>
</li>
<li>
<Link href="/not-found">/not-found</Link>
<Link href="/not-found" prefetch={false}>
/not-found
</Link>
</li>
<li>
<Link href="/redirect">/redirect</Link>
<Link href="/redirect" prefetch={false}>
/redirect
</Link>
</li>
</ul>
<SpanContextProvider>{children}</SpanContextProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
});

Expand All @@ -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');
});

Expand Down
6 changes: 0 additions & 6 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,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;
}
}

Expand Down

0 comments on commit e979c75

Please sign in to comment.