Skip to content

feat(nextjs): Use spans generated by Next.js for App Router #12729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,27 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

### Important Changes

- **feat(nextjs): Use spans generated by Next.js for App Router (#12729)**

Previously, the `@sentry/nextjs` SDK automatically recorded spans in the form of transactions for each of your top-level
server components (pages, layouts, ...). This approach had a few drawbacks, the main ones being that traces didn't have
a root span, and more importantly, if you had data stream to the client, its duration was not captured because the
server component spans had finished before the data could finish streaming.

With this release, we will capture the duration of App Router requests in their entirety as a single transaction with
server component spans being descendants of that transaction. This means you will get more data that is also more
accurate. Note that this does not apply to the Edge runtime. For the Edge runtime, the SDK will emit transactions as it
has before.

Generally speaking, this change means that you will see less _transactions_ and more _spans_ in Sentry. Your will no
longer receive server component transactions like `Page Server Component (/path/to/route)` (unless using the Edge
runtime), and you will instead receive transactions for your App Router SSR requests that look like
`GET /path/to/route`.

If you are on Sentry SaaS, this may have an effect on your quota consumption: Less transactions, more spans.

## 8.15.0

- feat(core): allow unregistering callback through `on` (#11710)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"test:assert": "pnpm test:prod && pnpm test:dev"
},
"dependencies": {
"@next/font": "13.0.7",
"@sentry/nextjs": "latest || *",
"@types/node": "18.11.17",
"@types/react": "18.0.26",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { NextResponse } from 'next/server';
export const dynamic = 'force-dynamic';

export async function GET() {
const data = await fetch(`http://localhost:3030/propagation/test-outgoing-fetch-external-disallowed/check`).then(
res => res.json(),
);
const data = await fetch(`http://localhost:3030/propagation/test-outgoing-fetch-external-disallowed/check`, {
cache: 'no-store',
}).then(res => res.json());
return NextResponse.json(data);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { NextResponse } from 'next/server';
export const dynamic = 'force-dynamic';

export async function GET() {
const data = await fetch(`http://localhost:3030/propagation/test-outgoing-fetch/check`).then(res => res.json());
const data = await fetch(`http://localhost:3030/propagation/test-outgoing-fetch/check`, { cache: 'no-store' }).then(
res => res.json(),
);
return NextResponse.json(data);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@ if (!testEnv) {
throw new Error('No test env defined');
}

const config = getPlaywrightConfig({
startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030',
port: 3030,
});
const config = getPlaywrightConfig(
{
startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030',
port: 3030,
},
{
// This comes with the risk of tests leaking into each other but the tests run quite slow so we should parallelize
workers: '100%',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thought was: If the tests leak, the tests are bad anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

},
);

export default config;
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
import { expect, test } from '@playwright/test';
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';

test('Should send a transaction event for a generateMetadata() function invokation', async ({ page }) => {
const testTitle = 'foobarasdf';
test('Should emit a span for a generateMetadata() function invokation', async ({ page }) => {
const testTitle = 'should-emit-span';

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions)' &&
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
transactionEvent.contexts?.trace?.data?.['http.target'] === `/generation-functions?metadataTitle=${testTitle}`
);
});

await page.goto(`/generation-functions?metadataTitle=${testTitle}`);

expect(await transactionPromise).toBeDefined();
const transaction = await transactionPromise;

expect(transaction.spans).toContainEqual(
expect.objectContaining({
description: 'generateMetadata /generation-functions/page',
origin: 'manual',
parent_span_id: expect.any(String),
span_id: expect.any(String),
status: 'ok',
trace_id: expect.any(String),
}),
);

const pageTitle = await page.title();
expect(pageTitle).toBe(testTitle);
Expand All @@ -22,12 +32,12 @@ test('Should send a transaction event for a generateMetadata() function invokati
test('Should send a transaction and an error event for a faulty generateMetadata() function invokation', async ({
page,
}) => {
const testTitle = 'foobarbaz';
const testTitle = 'should-emit-error';

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent.transaction === 'Page.generateMetadata (/generation-functions)' &&
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
transactionEvent.contexts?.trace?.data?.['http.target'] ===
`/generation-functions?metadataTitle=${testTitle}&shouldThrowInGenerateMetadata=1`
);
});

Expand All @@ -54,14 +64,23 @@ test('Should send a transaction event for a generateViewport() function invokati

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page.generateViewport (/generation-functions)' &&
(transactionEvent.extra?.route_data as any)?.searchParams?.viewportThemeColor === testTitle
transactionEvent.contexts?.trace?.data?.['http.target'] ===
`/generation-functions?viewportThemeColor=${testTitle}`
);
});

await page.goto(`/generation-functions?viewportThemeColor=${testTitle}`);

expect(await transactionPromise).toBeDefined();
expect((await transactionPromise).spans).toContainEqual(
expect.objectContaining({
description: 'generateViewport /generation-functions/page',
origin: 'manual',
parent_span_id: expect.any(String),
span_id: expect.any(String),
status: 'ok',
trace_id: expect.any(String),
}),
);
});

test('Should send a transaction and an error event for a faulty generateViewport() function invokation', async ({
Expand All @@ -71,8 +90,8 @@ test('Should send a transaction and an error event for a faulty generateViewport

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page.generateViewport (/generation-functions)' &&
(transactionEvent.extra?.route_data as any)?.searchParams?.viewportThemeColor === testTitle
transactionEvent.contexts?.trace?.data?.['http.target'] ===
`/generation-functions?viewportThemeColor=${testTitle}&shouldThrowInGenerateViewport=1`
);
});

Expand All @@ -97,8 +116,8 @@ test('Should send a transaction event with correct status for a generateMetadata

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions/with-redirect)' &&
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
transactionEvent.contexts?.trace?.data?.['http.target'] ===
`/generation-functions/with-redirect?metadataTitle=${testTitle}`
);
});

Expand All @@ -114,8 +133,8 @@ test('Should send a transaction event with correct status for a generateMetadata

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions/with-notfound)' &&
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
transactionEvent.contexts?.trace?.data?.['http.target'] ===
`/generation-functions/with-notfound?metadataTitle=${testTitle}`
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { waitForTransaction } from '@sentry-internal/test-utils';

test('Should send a transaction with a fetch span', async ({ page }) => {
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return transactionEvent?.transaction === 'Page Server Component (/request-instrumentation)';
return transactionEvent?.transaction === 'GET /request-instrumentation';
});

await page.goto(`/request-instrumentation`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export default async function Page() {
}

export async function generateMetadata() {
(await fetch('http://example.com/')).text();
(await fetch('http://example.com/', { cache: 'no-store' })).text();

return {
title: 'my title',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@ if (!testEnv) {
throw new Error('No test env defined');
}

const config = getPlaywrightConfig({
startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030',
port: 3030,
});
const config = getPlaywrightConfig(
{
startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030',
port: 3030,
},
{
// This comes with the risk of tests leaking into each other but the tests run quite slow so we should parallelize
workers: '100%',
},
);

export default config;
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';

test('all server component transactions should be attached to the pageload request span', async ({ page }) => {
const pageServerComponentTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Page Server Component (/pageload-tracing)';
});

const layoutServerComponentTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Layout Server Component (/pageload-tracing)';
});

const metadataTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Page.generateMetadata (/pageload-tracing)';
test('App router transactions should be attached to the pageload request span', async ({ page }) => {
const serverTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'GET /pageload-tracing';
});

const pageloadTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
Expand All @@ -20,18 +12,13 @@ test('all server component transactions should be attached to the pageload reque

await page.goto(`/pageload-tracing`);

const [pageServerComponentTransaction, layoutServerComponentTransaction, metadataTransaction, pageloadTransaction] =
await Promise.all([
pageServerComponentTransactionPromise,
layoutServerComponentTransactionPromise,
metadataTransactionPromise,
pageloadTransactionPromise,
]);
const [serverTransaction, pageloadTransaction] = await Promise.all([
serverTransactionPromise,
pageloadTransactionPromise,
]);

const pageloadTraceId = pageloadTransaction.contexts?.trace?.trace_id;

expect(pageloadTraceId).toBeTruthy();
expect(pageServerComponentTransaction.contexts?.trace?.trace_id).toBe(pageloadTraceId);
expect(layoutServerComponentTransaction.contexts?.trace?.trace_id).toBe(pageloadTraceId);
expect(metadataTransaction.contexts?.trace?.trace_id).toBe(pageloadTraceId);
expect(serverTransaction.contexts?.trace?.trace_id).toBe(pageloadTraceId);
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';

test('should not capture React-internal errors for PPR rendering', async ({ page }) => {
const pageServerComponentTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Page Server Component (/ppr-error/[param])';
return transactionEvent?.transaction === 'GET /ppr-error/[param]';
});

let errorEventReceived = false;
waitForError('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Page Server Component (/ppr-error/[param])';
waitForError('nextjs-15', async errorEvent => {
return errorEvent?.transaction === 'Page Server Component (/ppr-error/[param])';
}).then(() => {
errorEventReceived = true;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@ import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';

test('should not capture serverside suspense errors', async ({ page }) => {
const pageServerComponentTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Page Server Component (/suspense-error)';
return transactionEvent?.transaction === 'GET /suspense-error';
});

let errorEvent;
waitForError('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'Page Server Component (/suspense-error)';
waitForError('nextjs-15', async errorEvent => {
return errorEvent?.transaction === 'Page Server Component (/suspense-error)';
}).then(event => {
errorEvent = event;
});

await page.goto(`/suspense-error`);

// Just to be a little bit more sure
await page.waitForTimeout(5000);

const pageServerComponentTransaction = await pageServerComponentTransactionPromise;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
"test:assert": "pnpm test:test-build && pnpm test:prod && pnpm test:dev"
},
"dependencies": {
"@next/font": "13.0.7",
"@sentry/nextjs": "latest || *",
"@types/node": "18.11.17",
"@types/react": "18.0.26",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@ if (!testEnv) {
throw new Error('No test env defined');
}

const config = getPlaywrightConfig({
startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030',
port: 3030,
});
const config = getPlaywrightConfig(
{
startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030',
port: 3030,
},
{
// This comes with the risk of tests leaking into each other but the tests run quite slow so we should parallelize
workers: '100%',
},
);

export default config;
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test('Creates a navigation transaction for app router routes', async ({ page })

await page.goto(`/server-component/parameter/${randomRoute}`);
await clientPageloadTransactionPromise;
await page.getByText('Page (/server-component/parameter/[parameter])').isVisible();
await page.getByText('Page (/server-component/[parameter])').isVisible();

const clientNavigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
return (
Expand All @@ -39,7 +39,7 @@ test('Creates a navigation transaction for app router routes', async ({ page })

const serverComponentTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
return (
transactionEvent?.transaction === 'Page Server Component (/server-component/parameter/[...parameters])' &&
transactionEvent?.transaction === 'GET /server-component/parameter/foo/bar/baz' &&
(await clientNavigationTransactionPromise).contexts?.trace?.trace_id ===
transactionEvent.contexts?.trace?.trace_id
);
Expand Down
Loading
Loading