diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/package.json b/dev-packages/e2e-tests/test-applications/sveltekit-2/package.json index a323c3c415be..dbdb431960de 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/package.json +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/package.json @@ -6,6 +6,7 @@ "dev": "vite dev", "build": "vite build", "preview": "vite preview", + "proxy": "ts-node-script start-event-proxy.ts", "clean": "npx rimraf node_modules,pnpm-lock.yaml", "check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json", "check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch", diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+layout.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+layout.svelte index 08c4b6468a93..8eb402c9eda6 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+layout.svelte +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+layout.svelte @@ -1,10 +1,14 @@

Sveltekit E2E Test app

diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte index aeb12d58603f..29ce9ec693a8 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte @@ -15,7 +15,7 @@ Server Route error
  • - Route with Params + Route with Params
  • Route with Server Load @@ -23,4 +23,7 @@
  • Route with fetch in universal load
  • +
  • + Redirect +
  • diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect1/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect1/+page.ts new file mode 100644 index 000000000000..3f462bf810fd --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect1/+page.ts @@ -0,0 +1,5 @@ +import { redirect } from '@sveltejs/kit'; + +export const load = async () => { + redirect(301, '/redirect2'); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect2/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect2/+page.ts new file mode 100644 index 000000000000..99a810761d18 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/redirect2/+page.ts @@ -0,0 +1,5 @@ +import { redirect } from '@sveltejs/kit'; + +export const load = async () => { + redirect(301, '/users/789'); +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/test/performance.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/test/performance.test.ts index aed2040392e7..53b1ea128f00 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/test/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/test/performance.test.ts @@ -184,4 +184,222 @@ test.describe('performance events', () => { }, }); }); + + test('captures a navigation transaction directly after pageload', async ({ page }) => { + await page.goto('/'); + + const clientPageloadTxnPromise = waitForTransaction('sveltekit-2', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'pageload' && txnEvent?.tags?.runtime === 'browser'; + }); + + const clientNavigationTxnPromise = waitForTransaction('sveltekit-2', txnEvent => { + return txnEvent?.contexts?.trace?.op === 'navigation' && txnEvent?.tags?.runtime === 'browser'; + }); + + const navigationClickPromise = page.locator('#routeWithParamsLink').click(); + + const [pageloadTxnEvent, navigationTxnEvent, _] = await Promise.all([ + clientPageloadTxnPromise, + clientNavigationTxnPromise, + navigationClickPromise, + ]); + + expect(pageloadTxnEvent).toMatchObject({ + transaction: '/', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.sveltekit', + }, + }, + }); + + expect(navigationTxnEvent).toMatchObject({ + transaction: '/users/[id]', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'link', + }, + }, + }, + }); + + const routingSpans = navigationTxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(routingSpans).toHaveLength(1); + + const routingSpan = routingSpans && routingSpans[0]; + expect(routingSpan).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'link', + }, + }); + }); + + test('captures one navigation transaction per redirect', async ({ page }) => { + await page.goto('/'); + + const clientNavigationRedirect1TxnPromise = waitForTransaction('sveltekit-2', txnEvent => { + return ( + txnEvent?.contexts?.trace?.op === 'navigation' && + txnEvent?.tags?.runtime === 'browser' && + txnEvent?.transaction === '/redirect1' + ); + }); + + const clientNavigationRedirect2TxnPromise = waitForTransaction('sveltekit-2', txnEvent => { + return ( + txnEvent?.contexts?.trace?.op === 'navigation' && + txnEvent?.tags?.runtime === 'browser' && + txnEvent?.transaction === '/redirect2' + ); + }); + + const clientNavigationRedirect3TxnPromise = waitForTransaction('sveltekit-2', txnEvent => { + return ( + txnEvent?.contexts?.trace?.op === 'navigation' && + txnEvent?.tags?.runtime === 'browser' && + txnEvent?.transaction === '/users/[id]' + ); + }); + + const navigationClickPromise = page.locator('#redirectLink').click(); + + const [redirect1TxnEvent, redirect2TxnEvent, redirect3TxnEvent, _] = await Promise.all([ + clientNavigationRedirect1TxnPromise, + clientNavigationRedirect2TxnPromise, + clientNavigationRedirect3TxnPromise, + navigationClickPromise, + ]); + + expect(redirect1TxnEvent).toMatchObject({ + transaction: '/redirect1', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.origin': 'auto.navigation.sveltekit', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + 'sentry.sveltekit.navigation.type': 'link', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect1', + 'sentry.sample_rate': 1, + }, + }, + }, + }); + + const redirect1Spans = redirect1TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(redirect1Spans).toHaveLength(1); + + const redirect1Span = redirect1Spans && redirect1Spans[0]; + expect(redirect1Span).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect1', + 'sentry.sveltekit.navigation.type': 'link', + }, + }); + + expect(redirect2TxnEvent).toMatchObject({ + transaction: '/redirect2', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.origin': 'auto.navigation.sveltekit', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + 'sentry.sveltekit.navigation.type': 'goto', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect2', + 'sentry.sample_rate': 1, + }, + }, + }, + }); + + const redirect2Spans = redirect2TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(redirect2Spans).toHaveLength(1); + + const redirect2Span = redirect2Spans && redirect2Spans[0]; + expect(redirect2Span).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/redirect2', + 'sentry.sveltekit.navigation.type': 'goto', + }, + }); + + expect(redirect3TxnEvent).toMatchObject({ + transaction: '/users/[id]', + tags: { runtime: 'browser' }, + transaction_info: { source: 'route' }, + type: 'transaction', + contexts: { + trace: { + op: 'navigation', + origin: 'auto.navigation.sveltekit', + data: { + 'sentry.origin': 'auto.navigation.sveltekit', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + 'sentry.sveltekit.navigation.type': 'goto', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sample_rate': 1, + }, + }, + }, + }); + + const redirect3Spans = redirect3TxnEvent.spans?.filter(s => s.op === 'ui.sveltekit.routing'); + expect(redirect3Spans).toHaveLength(1); + + const redirect3Span = redirect3Spans && redirect3Spans[0]; + expect(redirect3Span).toMatchObject({ + op: 'ui.sveltekit.routing', + description: 'SvelteKit Route Change', + data: { + 'sentry.op': 'ui.sveltekit.routing', + 'sentry.origin': 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'goto', + }, + }); + }); }); diff --git a/packages/sveltekit/src/client/browserTracingIntegration.ts b/packages/sveltekit/src/client/browserTracingIntegration.ts index 5e80cdc92f28..07de01e86c11 100644 --- a/packages/sveltekit/src/client/browserTracingIntegration.ts +++ b/packages/sveltekit/src/client/browserTracingIntegration.ts @@ -4,12 +4,12 @@ import { BrowserTracing as OriginalBrowserTracing, WINDOW, browserTracingIntegration as originalBrowserTracingIntegration, - getActiveSpan, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan, startInactiveSpan, } from '@sentry/svelte'; import type { Client, Integration, Span } from '@sentry/types'; +import { dropUndefinedKeys } from '@sentry/utils'; import { svelteKitRoutingInstrumentation } from './router'; /** @@ -64,7 +64,7 @@ export function browserTracingIntegration( function _instrumentPageload(client: Client): void { const initialPath = WINDOW && WINDOW.location && WINDOW.location.pathname; - startBrowserTracingPageLoadSpan(client, { + const pageloadSpan = startBrowserTracingPageLoadSpan(client, { name: initialPath, op: 'pageload', description: initialPath, @@ -76,8 +76,9 @@ function _instrumentPageload(client: Client): void { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', }, }); - - const pageloadSpan = getActiveSpan(); + if (!pageloadSpan) { + return; + } page.subscribe(page => { if (!page) { @@ -86,7 +87,7 @@ function _instrumentPageload(client: Client): void { const routeId = page.route && page.route.id; - if (pageloadSpan && routeId) { + if (routeId) { pageloadSpan.updateName(routeId); pageloadSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); } @@ -98,7 +99,6 @@ function _instrumentPageload(client: Client): void { */ function _instrumentNavigations(client: Client): void { let routingSpan: Span | undefined; - let activeSpan: Span | undefined; navigating.subscribe(navigation => { if (!navigation) { @@ -129,36 +129,42 @@ function _instrumentNavigations(client: Client): void { const parameterizedRouteOrigin = from && from.route.id; const parameterizedRouteDestination = to && to.route.id; - activeSpan = getActiveSpan(); - - if (!activeSpan) { - startBrowserTracingNavigationSpan(client, { - name: parameterizedRouteDestination || rawRouteDestination || 'unknown', - op: 'navigation', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: parameterizedRouteDestination ? 'route' : 'url', - }, - tags: { - 'routing.instrumentation': '@sentry/sveltekit', - }, - }); - activeSpan = getActiveSpan(); + if (routingSpan) { + // If a routing span is still open from a previous navigation, we finish it. + // This is important for e.g. redirects when a new navigation root span finishes + // the first root span. If we don't `.end()` the previous span, it will get + // status 'cancelled' which isn't entirely correct. + routingSpan.end(); } - if (activeSpan) { - if (routingSpan) { - // If a routing span is still open from a previous navigation, we finish it. - routingSpan.end(); - } - routingSpan = startInactiveSpan({ - op: 'ui.sveltekit.routing', - name: 'SvelteKit Route Change', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.sveltekit', - }, - }); - activeSpan.setAttribute('sentry.sveltekit.navigation.from', parameterizedRouteOrigin || undefined); - } + const navigationInfo = dropUndefinedKeys({ + // `navigation.type` denotes the origin of the navigation. e.g.: + // - link (clicking on a link) + // - goto (programmatic via goto() or redirect()) + // - popstate (back/forward navigation) + 'sentry.sveltekit.navigation.type': navigation.type, + 'sentry.sveltekit.navigation.from': parameterizedRouteOrigin || undefined, + 'sentry.sveltekit.navigation.to': parameterizedRouteDestination || undefined, + }); + + startBrowserTracingNavigationSpan(client, { + name: parameterizedRouteDestination || rawRouteDestination || 'unknown', + op: 'navigation', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: parameterizedRouteDestination ? 'route' : 'url', + ...navigationInfo, + }, + }); + + routingSpan = startInactiveSpan({ + op: 'ui.sveltekit.routing', + name: 'SvelteKit Route Change', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.sveltekit', + ...navigationInfo, + }, + onlyIfParent: true, + }); }); } diff --git a/packages/sveltekit/test/client/browserTracingIntegration.test.ts b/packages/sveltekit/test/client/browserTracingIntegration.test.ts index 415637c05560..fba7136bd5eb 100644 --- a/packages/sveltekit/test/client/browserTracingIntegration.test.ts +++ b/packages/sveltekit/test/client/browserTracingIntegration.test.ts @@ -45,6 +45,7 @@ describe('browserTracingIntegration', () => { }), setTag: vi.fn(), }; + return createdRootSpan; }); const startBrowserTracingNavigationSpanSpy = vi @@ -56,6 +57,7 @@ describe('browserTracingIntegration', () => { setAttribute: vi.fn(), setTag: vi.fn(), }; + return createdRootSpan; }); const fakeClient = { getOptions: () => ({}), on: () => {} }; @@ -173,6 +175,7 @@ describe('browserTracingIntegration', () => { navigating.set({ from: { route: { id: '/users' }, url: { pathname: '/users' } }, to: { route: { id: '/users/[id]' }, url: { pathname: '/users/7762' } }, + type: 'link', }); // This should update the transaction name with the parameterized route: @@ -183,9 +186,9 @@ describe('browserTracingIntegration', () => { attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }, - tags: { - 'routing.instrumentation': '@sentry/sveltekit', + 'sentry.sveltekit.navigation.from': '/users', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'link', }, }); @@ -195,12 +198,13 @@ describe('browserTracingIntegration', () => { name: 'SvelteKit Route Change', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/users', + 'sentry.sveltekit.navigation.to': '/users/[id]', + 'sentry.sveltekit.navigation.type': 'link', }, + onlyIfParent: true, }); - // eslint-disable-next-line deprecation/deprecation - expect(createdRootSpan?.setAttribute).toHaveBeenCalledWith('sentry.sveltekit.navigation.from', '/users'); - // We emit `null` here to simulate the end of the navigation lifecycle // @ts-expect-error - page is a writable but the types say it's just readable navigating.set(null); @@ -246,9 +250,8 @@ describe('browserTracingIntegration', () => { attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.sveltekit', - }, - tags: { - 'routing.instrumentation': '@sentry/sveltekit', + 'sentry.sveltekit.navigation.from': '/users/[id]', + 'sentry.sveltekit.navigation.to': '/users/[id]', }, }); @@ -258,11 +261,11 @@ describe('browserTracingIntegration', () => { name: 'SvelteKit Route Change', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.sveltekit', + 'sentry.sveltekit.navigation.from': '/users/[id]', + 'sentry.sveltekit.navigation.to': '/users/[id]', }, + onlyIfParent: true, }); - - // eslint-disable-next-line deprecation/deprecation - expect(createdRootSpan?.setAttribute).toHaveBeenCalledWith('sentry.sveltekit.navigation.from', '/users/[id]'); }); it('falls back to `window.location.pathname` to determine the raw origin', () => {