8000 fix(sveltekit): Ensure navigations and redirects always create a new … · GingerAdonis/sentry-javascript@05bb609 · GitHub
[go: up one dir, main page]
Skip to content

Commit 05bb609

Browse files
Lms24mydea
Lms24
and
mydea
authored
fix(sveltekit): Ensure navigations and redirects always create a new transaction (getsentry#10656)
Adjust the root span/transaction creation behaviour of the SvelteKit router instrumentation: - We no longer check for an active span to create a new navigation span - This is possible without any `pageloadOngoing` flag because, luckily, pageload and navigation instrumentation is completely decoupled - Consequently, navigations happening directly after the first pageload will now create their own transaction instead of being attached to the pageload transaction - Consequently, redirects within a (user perceived) navigation are now treated as separate transactions, where the "current" navigation finishes the transaction of the previous redirect. Additional changes: * Removed tag for routing instrumentation (we now have `sentry.origin` for this anyway) * Added more navigation info to the spans: * `sentry.sveltekit.navigation.type`: indicates the source of the navigation (link click, back/forward button press, goto/redirect call, etc) * `sentry.sveltekit.navigation.from`: parameterized navigation start * `sentry.sveltekit.navigation.to`: parameterized navigation end --------- Co-authored-by: Francesco Novy <francesco.novy@sentry.io>
1 parent 4917612 commit 05bb609

File tree

8 files changed

+298
-53
lines changed

8 files changed

+298
-53
lines changed

‎dev-packages/e2e-tests/test-applications/sveltekit-2/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"dev": "vite dev",
77
"build": "vite build",
88
"preview": "vite preview",
9+
"proxy": "ts-node-script start-event-proxy.ts",
910
"clean": "npx rimraf node_modules,pnpm-lock.yaml",
1011
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
1112
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",

‎dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+layout.svelte

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
<script lang="ts">
2-
import { onMount } from "svelte";
2+
import { onMount } from "svelte";
3+
4+
import { navigating } from "$app/stores";
5+
6+
onMount(() => {
7+
// Indicate that the SvelteKit app was hydrated
8+
document.body.classList.add("hydrated");
9+
});
10+
311
4-
onMount(() => {
5-
// Indicate that the SvelteKit app was hydrated
6-
document.body.classList.add("hydrated");
7-
});
812
</script>
913

1014
<h1>Sveltekit E2E Test app</h1>

‎dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte

Lines changed: 4 additions & 1 deletion
< 9E88 /tr>
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
<a href="/server-route-error">Server Route error</a>
1616
</li>
1717
<li>
18-
<a href="/users/123abc">Route with Params</a>
18+
<a id="routeWithParamsLink" href="/users/123abc">Route with Params</a>
1919
</li>
2020
<li>
2121
<a href="/users">Route with Server Load</a>
2222
</li>
2323
<li>
2424
<a href="/universal-load-fetch">Route with fetch in universal load</a>
2525
</li>
26+
<li>
27+
<a id="redirectLink" href="/redirect1">Redirect</a>
28+
</li>
2629
</ul>

‎dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect1/+page.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { redirect } from '@sveltejs/kit';
2+
3+
export const load = async () => {
4+
redirect(301, '/redirect2');
5+
};

‎dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect2/+page.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { redirect } from '@sveltejs/kit';
2+
3+
export const load = async () => {
4+
redirect(301, '/users/789');
5+
};

‎dev-packages/e2e-tests/test-applications/sveltekit-2/test/performance.test.ts

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,4 +184,222 @@ test.describe('performance events', () => {
184184
},
185185
});
186186
});
187+
188+
test('captures a navigation transaction directly after pageload', async ({ page }) => {
189+
await page.goto('/');
190+
191+
const clientPageloadTxnPromise = waitForTransaction('sveltekit-2', txnEvent => {
192+
return txnEvent?.contexts?.trace?.op === 'pageload' && txnEvent?.tags?.runtime === 'browser';
193+
});
194+
195+
const clientNavigationTxnPromise = waitForTransaction('sveltekit-2', txnEvent => {
196+
return txnEvent?.contexts?.trace?.op === 'navigation' && txnEvent?.tags?.runtime === 'browser';
197+
});
198+
199+
const navigationClickPromise = page.locator('#routeWithParamsLink').click();
200+
201+
const [pageloadTxnEvent, navigationTxnEvent, _] = await Promise.all([
202+
clientPageloadTxnPromise,
203+
clientNavigationTxnPromise,
204+
navigationClickPromise,
205+
]);
206+
207+
expect(pageloadTxnEvent).toMatchObject({
208+
transaction: '/',
209+
tags: { runtime: 'browser' },
210+
transaction_info: { source: 'route' },
211+
type: 'transaction',
212+
contexts: {
213+
trace: {
214+
op: 'pageload',
215+
origin: 'auto.pageload.sveltekit',
216+
},
217+
},
218+
});
219+
220+
expect(navigationTxnEvent).toMatchObject({
221+
transaction: '/users/[id]',
222+
tags: { runtime: 'browser' },
223+
transaction_info: { source: 'route' },
224+
type: 'transaction',
225+
contexts: {
226+
trace: {
227+
op: 'navigation',
228+
origin: 'auto.navigation.sveltekit',
229+
data: {
230+
'sentry.sveltekit.navigation.from': '/',
231+
'sentry.sveltekit.navigation.to': '/users/[id]',
232+
'sentry.sveltekit.navigation.type': 'link',
233+
},
234+
},
235+
},
236+
});
237+
238+
const routingSpans = navigationTxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing');
239+
expect(routingSpans).toHaveLength(1);
240+
241+
const routingSpan = routingSpans && routingSpans[0];
242+
expect(routingSpan).toMatchObject({
243+
op: 'ui.sveltekit.routing',
244+
description: 'SvelteKit Route Change',
245+
data: {
246+
'sentry.op': 'ui.sveltekit.routing',
247+
'sentry.origin': 'auto.ui.sveltekit',
248+
'sentry.sveltekit.navigation.from': '/',
249+
'sentry.sveltekit.navigation.to': '/users/[id]',
250+
'sentry.sveltekit.navigation.type': 'link',
251+
},
252+
});
253+
});
254+
255+
test('captures one navigation transaction per redirect', async ({ page }) => {
256+
await page.goto('/');
257+
258+
const clientNavigationRedirect1TxnPromise = waitForTransaction('sveltekit-2', txnEvent => {
259+
return (
260+
txnEvent?.contexts?.trace?.op === 'navigation' &&
261+
txnEvent?.tags?.runtime === 'browser' &&
262+
txnEvent?.transaction === '/redirect1'
263+
);
264+
});
265+
266+
const clientNavigationRedirect2TxnPromise = waitForTransaction('sveltekit-2', txnEvent => {
267+
return (
268+
txnEvent?.contexts?.trace?.op === 'navigation' &&
269+
txnEvent?.tags?.runtime === 'browser' &&
270+
txnEvent?.transaction === '/redirect2'
271+
);
272+
});
273+
274+
const clientNavigationRedirect3TxnPromise = waitForTransaction('sveltekit-2', txnEvent => {
275+
return (
276+
txnEvent?.contexts?.trace?.op === 'navigation' &&
277+
txnEvent?.tags?.runtime === 'browser' &&
278+
txnEvent?.transaction === '/users/[id]'
279+
);
280+
});
281+
282+
const navigationClickPromise = page.locator('#redirectLink').click();
283+
284+
const [redirect1TxnEvent, redirect2TxnEvent, redirect3TxnEvent, _] = await Promise.all([
285+
clientNavigationRedirect1TxnPromise,
286+
clientNavigationRedirect2TxnPromise,
287+
clientNavigationRedirect3TxnPromise,
288+
navigationClickPromise,
289+
]);
290+
291+
expect(redirect1TxnEvent).toMatchObject({
292+
transaction: '/redirect1',
293+
tags: { runtime: 'browser' },
294+
transaction_info: { source: 'route' },
295+
type: 'transaction',
296+
contexts: {
297+
trace: {
298+
op: 'navigation',
299+
origin: 'auto.navigation.sveltekit',
300+
data: {
301+
'sentry.origin': 'auto.navigation.sveltekit',
302+
'sentry.op': 'navigation',
303+
'sentry.source': 'route',
304+
'sentry.sveltekit.navigation.type': 'link',
305+
'sentry.sveltekit.navigation.from': '/',
306+
'sentry.sveltekit.navigation.to': '/redirect1',
307+
'sentry.sample_rate': 1,
308+
},
309+
},
310+
},
311+
});
312+
313+
const redirect1Spans = redirect1TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing');
314+
expect(redirect1Spans).toHaveLength(1);
315+
316+
const redirect1Span = redirect1Spans && redirect1Spans[0];
317+
expect(redirect1Span).toMatchObject({
318+
op: 'ui.sveltekit.routing',
319+
description: 'SvelteKit Route Change',
320+
data: {
321+
'sentry.op': 'ui.sveltekit.routing',
322+
'sentry.origin': 'auto.ui.sveltekit',
323+
'sentry.sveltekit.navigation.from': '/',
324+
'sentry.sveltekit.navigation.to': '/redirect1',
325+
'sentry.sveltekit.navigation.type': 'link',
326+
},
327+
});
328+
329+
expect(redirect2TxnEvent).toMatchObject({
330+
transaction: '/redirect2',
331+
tags: { runtime: 'browser' },
332+
transaction_info: { source: 'route' },
333+
type: 'transaction',
334+
contexts: {
335+
trace: {
336+
op: 'navigation',
337+
origin: 'auto.navigation.sveltekit',
338+
data: {
339+
'sentry.origin': 'auto.navigation.sveltekit',
340+
'sentry.op': 'navigation',
341+
'sentry.source': 'route',
342+
'sentry.sveltekit.navigation.type': 'goto',
343+
'sentry.sveltekit.navigation.from': '/',
344+
'sentry.sveltekit.navigation.to': '/redirect2',
345+
'sentry.sample_rate': 1,
346+
},
347+
},
348+
},
349+
});
350+
351+
const redirect2Spans = redirect2TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing');
352+
expect(redirect2Spans).toHaveLength(1);
353+
354+
const redirect2Span = redirect2Spans && redirect2Spans[0];
355+
expect(redirect2Span).toMatchObject({
356+
op: 'ui.sveltekit.routing',
357+
description: 'SvelteKit Route Change',
358+
data: {
359+
'sentry.op': 'ui.sveltekit.routing',
360+
'sentry.origin': 'auto.ui.sveltekit',
361+
'sentry.sveltekit.navigation.from': '/',
362+
'sentry.sveltekit.navigation.to': '/redirect2',
363+
'sentry.sveltekit.navigation.type': 'goto',
364+
},
365+
});
366+
367+
expect(redirect3TxnEvent).toMatchObject({
368+
transaction: '/users/[id]',
369+
tags: { runtime: 'browser' },
370+
transaction_info: { source: 'route' },
371+
type: 'transaction',
372+
contexts: {
373+
trace: {
374+
op: 'navigation',
375+
origin: 'auto.navigation.sveltekit',
376+
data: {
377+
'sentry.origin': 'auto.navigation.sveltekit',
378+
'sentry.op': 'navigation',
379+
'sentry.source': 'route',
380+
'sentry.sveltekit.navigation.type': 'goto',
381+
'sentry.sveltekit.navigation.from': '/',
382+
'sentry.sveltekit.navigation.to': '/users/[id]',
383+
'sentry.sample_rate': 1,
384+
},
385+
},
386+
},
387+
});
388+
389+
const redirect3Spans = redirect3TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing');
390+
expect(redirect3Spans).toHaveLength(1);
391+
392+
const redirect3Span = redirect3Spans && redirect3Spans[0];
393+
expect(redirect3Span).toMatchObject({
394+
op: 'ui.sveltekit.routing',
395+
description: 'SvelteKit Route Change',
396+
data: {
397+
'sentry.op': 'ui.sveltekit.routing',
398+
'sentry.origin': 'auto.ui.sveltekit',
399+
'sentry.sveltekit.navigation.from': '/',
400+
'sentry.sveltekit.navigation.to': '/users/[id]',
401+
'sentry.sveltekit.navigation.type': 'goto',
402+
},
403+
});
404+
});
187405
});

0 commit comments

Comments
 (0)
0