Skip to content

Commit 97974ba

Browse files
lforstlforst
authored
ref(nextjs): Improve app router routing instrumentation accuracy (#13695)
Improves the Next.js routing instrumentation by patching the Next.js router and instrumenting window popstates. A few details on this PR that might explain weird-looking logic: - The patching of the router is in a setInterval because Next.js may take a while to write the router to the window object and we don't have a cue when that has happened. - We are using a combination of patching `router.back`/`router.forward` and the `popstate` event to emit a properly named transaction, because `router.back` and `router.forward` aren't passed any useful strings we could use as txn names. - Because there is a slight delay between the `router.back`/`router.forward` calls and the `popstate` event, we temporarily give the navigation span an invalid name that we use as an indicator to drop if one may leak through.
1 parent cf0152a commit 97974ba

File tree

14 files changed

+352
-229
lines changed

14 files changed

+352
-229
lines changed

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/[param]/browser-back/page.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import Link from 'next/link';
2+
3+
export const dynamic = 'force-dynamic';
4+
5+
export default function Page() {
6+
return <Link href="/navigation">Go back home</Link>;
7+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/[param]/link-replace/page.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import Link from 'next/link';
2+
3+
export const dynamic = 'force-dynamic';
4+
5+
export default function Page() {
6+
return <Link href="/navigation">Go back home</Link>;
7+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/[param]/link/page.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import Link from 'next/link';
2+
3+
export const dynamic = 'force-dynamic';
4+
5+
export default function Page() {
6+
return <Link href="/navigation">Go back home</Link>;
7+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/[param]/router-back/page.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import Link from 'next/link';
2+
3+
export const dynamic = 'force-dynamic';
4+
5+
export default function Page() {
6+
return <Link href="/navigation">Go back home</Link>;
7+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/[param]/router-push/page.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export const dynamic = 'force-dynamic';
2+
3+
export default function Page() {
4+
return <p>hello world</p>;
5+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/[param]/router-replace/page.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export const dynamic = 'force-dynamic';
2+
3+
export default function Page() {
4+
return <p>hello world</p>;
5+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/navigation/page.tsx

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use client';
2+
3+
import Link from 'next/link';
4+
import { useRouter } from 'next/navigation';
5+
6+
export default function Page() {
7+
const router = useRouter();
8+
9+
return (
10+
<ul>
11+
<li>
12+
<button
13+
onClick={() => {
14+
router.push('/navigation/42/router-push');
15+
}}
16+
>
17+
router.push()
18+
</button>
19+
</li>
20+
<li>
21+
<button
22+
onClick={() => {
23+
router.replace('/navigation/42/router-replace');
24+
}}
25+
>
26+
router.replace()
27+
</button>
28+
</li>
29+
<li>
30+
<button
31+
onClick={() => {
32+
router.forward();
33+
}}
34+
>
35+
router.forward()
36+
</button>
37+
</li>
38+
<li>
39+
<button
40+
onClick={() => {
41+
router.back();
42+
}}
43+
>
44+
router.back()
45+
</button>
46+
</li>
47+
<li>
48+
<Link href="/navigation/42/link">Normal Link</Link>
49+
</li>
50+
<li>
51+
<Link href="/navigation/42/link-replace" replace>
52+
Link Replace
53+
</Link>
54+
</li>
55+
</ul>
56+
);
57+
}

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/client-app-routing-instrumentation.test.ts

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,148 @@ test('Creates a navigation transaction for app router routes', async ({ page })
5353
expect(await clientNavigationTransactionPromise).toBeDefined();
5454
expect(await serverComponentTransactionPromise).toBeDefined();
5555
});
56+
57+
test('Creates a navigation transaction for `router.push()`', async ({ page }) => {
58+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
59+
return (
60+
transactionEvent?.transaction === `/navigation/42/router-push` &&
61+
transactionEvent.contexts?.trace?.op === 'navigation' &&
62+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.push'
63+
);
64+
});
65+
66+
await page.goto('/navigation');
67+
await page.waitForTimeout(3000);
68+
await page.getByText('router.push()').click();
69+
70+
expect(await navigationTransactionPromise).toBeDefined();
71+
});
72+
73+
test('Creates a navigation transaction for `router.replace()`', async ({ page }) => {
74+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
75+
return (
76+
transactionEvent?.transaction === `/navigation/42/router-replace` &&
77+
transactionEvent.contexts?.trace?.op === 'navigation' &&
78+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.replace'
79+
);
80+
});
81+
82+
await page.goto('/navigation');
83+
await page.waitForTimeout(3000);
84+
await page.getByText('router.replace()').click();
85+
86+
expect(await navigationTransactionPromise).toBeDefined();
87+
});
88+
89+
test('Creates a navigation transaction for `router.back()`', async ({ page }) => {
90+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
91+
return (
92+
transactionEvent?.transaction === `/navigation/1337/router-back` &&
93+
transactionEvent.contexts?.trace?.op === 'navigation'
94+
);
95+
});
96+
97+
await page.goto('/navigation/1337/router-back');
98+
await page.waitForTimeout(3000);
99+
await page.getByText('Go back home').click();
100+
await page.waitForTimeout(3000);
101+
await page.getByText('router.back()').click();
102+
103+
expect(await navigationTransactionPromise).toMatchObject({
104+
contexts: {
105+
trace: {
106+
data: {
107+
'navigation.type': 'router.back',
108+
},
109+
},
110+
},
111+
});
112+
});
113+
114+
test('Creates a navigation transaction for `router.forward()`', async ({ page }) => {
115+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
116+
return (
117+
transactionEvent?.transaction === `/navigation/42/router-push` &&
118+
transactionEvent.contexts?.trace?.op === 'navigation' &&
119+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.forward'
120+
);
121+
});
122+
123+
await page.goto('/navigation');
124+
await page.waitForTimeout(3000);
125+
await page.getByText('router.push()').click();
126+
await page.waitForTimeout(3000);
127+
await page.goBack();
128+
await page.waitForTimeout(3000);
129+
await page.getByText('router.forward()').click();
130+
131+
expect(await navigationTransactionPromise).toBeDefined();
132+
});
133+
134+
test('Creates a navigation transaction for `<Link />`', async ({ page }) => {
135+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
136+
return (
137+
transactionEvent?.transaction === `/navigation/42/link` &&
138+
transactionEvent.contexts?.trace?.op === 'navigation' &&
139+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.push'
140+
);
141+
});
142+
143+
await page.goto('/navigation');
144+
await page.getByText('Normal Link').click();
145+
146+
expect(await navigationTransactionPromise).toBeDefined();
147+
});
148+
149+
test('Creates a navigation transaction for `<Link replace />`', async ({ page }) => {
150+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
151+
return (
152+
transactionEvent?.transaction === `/navigation/42/link-replace` &&
153+
transactionEvent.contexts?.trace?.op === 'navigation' &&
154+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'router.replace'
155+
);
156+
});
157+
158+
await page.goto('/navigation');
159+
await page.waitForTimeout(3000);
160+
await page.getByText('Link Replace').click();
161+
162+
expect(await navigationTransactionPromise).toBeDefined();
163+
});
164+
165+
test('Creates a navigation transaction for browser-back', async ({ page }) => {
166+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
167+
return (
168+
transactionEvent?.transaction === `/navigation/42/browser-back` &&
169+
transactionEvent.contexts?.trace?.op === 'navigation' &&
170+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'browser.popstate'
171+
);
172+
});
173+
174+
await page.goto('/navigation/42/browser-back');
175+
await page.waitForTimeout(3000);
176+
await page.getByText('Go back home').click();
177+
await page.waitForTimeout(3000);
178+
await page.goBack();
179+
180+
expect(await navigationTransactionPromise).toBeDefined();
181+
});
182+
183+
test('Creates a navigation transaction for browser-forward', async ({ page }) => {
184+
const navigationTransactionPromise = waitForTransaction('nextjs-app-dir', transactionEvent => {
185+
return (
186+
transactionEvent?.transaction === `/navigation/42/router-push` &&
187+
transactionEvent.contexts?.trace?.op === 'navigation' &&
188+
transactionEvent.contexts.trace.data?.['navigation.type'] === 'browser.popstate'
189+
);
190+
});
191+
192+
await page.goto('/navigation');
193+
await page.getByText('router.push()').click();
194+
await page.waitForTimeout(3000);
195+
await page.goBack();
196+
await page.waitForTimeout(3000);
197+
await page.goForward();
198+
199+
expect(await navigationTransactionPromise).toBeDefined();
200+
});

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"clean:build": "lerna run clean",
1818
"clean:caches": "yarn rimraf eslintcache .nxcache && yarn jest --clearCache",
1919
"clean:deps": "lerna clean --yes && rm -rf node_modules && yarn",
20-
"clean:tarballs": "rimraf **/*.tgz",
20+
"clean:tarballs": "rimraf -g **/*.tgz",
2121
"clean:all": "run-s clean:build clean:tarballs clean:caches clean:deps",
2222
"fix": "run-s fix:biome fix:prettier fix:lerna",
2323
"fix:lerna": "lerna run fix",

‎packages/nextjs/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
"@opentelemetry/instrumentation-http": "0.53.0",
7272
"@opentelemetry/semantic-conventions": "^1.27.0",
7373
"@rollup/plugin-commonjs": "26.0.1",
74+
"@sentry-internal/browser-utils": "8.30.0",
7475
"@sentry/core": "8.30.0",
7576
"@sentry/node": "8.30.0",
7677
"@sentry/opentelemetry": "8.30.0",

‎packages/nextjs/src/client/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolica
88
import { getVercelEnv } from '../common/getVercelEnv';
99
import { browserTracingIntegration } from './browserTracingIntegration';
1010
import { nextjsClientStackFrameNormalizationIntegration } from './clientNormalizationIntegration';
11+
import { INCOMPLETE_APP_ROUTER_INSTRUMENTATION_TRANSACTION_NAME } from './routing/appRouterRoutingInstrumentation';
1112
import { applyTunnelRouteOption } from './tunnelRoute';
1213

1314
export * from '@sentry/react';
@@ -39,6 +40,13 @@ export function init(options: BrowserOptions): Client | undefined {
3940
filterTransactions.id = 'NextClient404Filter';
4041
addEventProcessor(filterTransactions);
4142

43+
const filterIncompleteNavigationTransactions: EventProcessor = event =>
44+
event.type === 'transaction' && event.transaction === INCOMPLETE_APP_ROUTER_INSTRUMENTATION_TRANSACTION_NAME
45+
? null
46+
: event;
47+
filterIncompleteNavigationTransactions.id = 'IncompleteTransactionFilter';
48+
addEventProcessor(filterIncompleteNavigationTransactions);
49+
4250
if (process.env.NODE_ENV === 'development') {
4351
addEventProcessor(devErrorSymbolicationEventProcessor);
4452
}

0 commit comments

Comments
 (0)