diff --git a/dev-packages/e2e-tests/test-applications/astro-4/src/pages/user-page/settings.astro b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/user-page/settings.astro new file mode 100644 index 000000000000..5a46ac891a24 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/astro-4/src/pages/user-page/settings.astro @@ -0,0 +1,10 @@ +--- +import Layout from '../../layouts/Layout.astro'; + +export const prerender = false; + +--- + + +

User Settings

+ diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts index 2699e827718e..07e0467382da 100644 --- a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts @@ -30,11 +30,11 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { trace: { data: expect.objectContaining({ 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }), op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', span_id: expect.stringMatching(/[a-f0-9]{16}/), parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -55,9 +55,7 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { start_timestamp: expect.any(Number), timestamp: expect.any(Number), transaction: '/test-ssr', - transaction_info: { - source: 'url', - }, + transaction_info: { source: 'route' }, type: 'transaction', }); @@ -113,9 +111,7 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { start_timestamp: expect.any(Number), timestamp: expect.any(Number), transaction: 'GET /test-ssr', - transaction_info: { - source: 'route', - }, + transaction_info: { source: 'route' }, type: 'transaction', }); }); @@ -194,18 +190,21 @@ test.describe('nested SSR routes (client, server, server request)', () => { span => span.op === 'http.client' && span.description?.includes('/api/user/'), ); + const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content'); + expect(routeNameMetaContent).toBe('%2Fuser-page%2F%5BuserId%5D'); + // Client pageload transaction - actual URL with pageload operation expect(clientPageloadTxn).toMatchObject({ - transaction: '/user-page/myUsername123', // todo: parametrize - transaction_info: { source: 'url' }, + transaction: '/user-page/[userId]', + transaction_info: { source: 'route' }, contexts: { trace: { op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', data: { 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }, }, }, @@ -275,20 +274,23 @@ test.describe('nested SSR routes (client, server, server request)', () => { await page.goto('/catchAll/hell0/whatever-do'); + const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content'); + expect(routeNameMetaContent).toBe('%2FcatchAll%2F%5Bpath%5D'); + const clientPageloadTxn = await clientPageloadTxnPromise; const serverPageRequestTxn = await serverPageRequestTxnPromise; expect(clientPageloadTxn).toMatchObject({ - transaction: '/catchAll/hell0/whatever-do', // todo: parametrize - transaction_info: { source: 'url' }, + transaction: '/catchAll/[path]', + transaction_info: { source: 'route' }, contexts: { trace: { op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', data: { 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }, }, }, @@ -313,3 +315,55 @@ test.describe('nested SSR routes (client, server, server request)', () => { }); }); }); + +// Case for `user-page/[id]` vs. `user-page/settings` static routes +test.describe('parametrized vs static paths', () => { + test('should use static route name for static route in parametrized path', async ({ page }) => { + const clientPageloadTxnPromise = waitForTransaction('astro-4', txnEvent => { + return txnEvent?.transaction?.startsWith('/user-page/') ?? false; + }); + + const serverPageRequestTxnPromise = waitForTransaction('astro-4', txnEvent => { + return txnEvent?.transaction?.startsWith('GET /user-page/') ?? false; + }); + + await page.goto('/user-page/settings'); + + const clientPageloadTxn = await clientPageloadTxnPromise; + const serverPageRequestTxn = await serverPageRequestTxnPromise; + + expect(clientPageloadTxn).toMatchObject({ + transaction: '/user-page/settings', + transaction_info: { source: 'route' }, + contexts: { + trace: { + op: 'pageload', + origin: 'auto.pageload.astro', + data: { + 'sentry.op': 'pageload', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', + }, + }, + }, + }); + + expect(serverPageRequestTxn).toMatchObject({ + transaction: 'GET /user-page/settings', + transaction_info: { source: 'route' }, + contexts: { + trace: { + op: 'http.server', + origin: 'auto.http.astro', + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.astro', + 'sentry.source': 'route', + url: expect.stringContaining('/user-page/settings'), + }, + }, + }, + request: { url: expect.stringContaining('/user-page/settings') }, + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts index c04bbb568f2e..30bcbee1a026 100644 --- a/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts @@ -35,11 +35,11 @@ test.describe('tracing in static/pre-rendered routes', () => { trace: { data: expect.objectContaining({ 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }), op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', parent_span_id: metaParentSpanId, span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: metaTraceId, @@ -48,12 +48,12 @@ test.describe('tracing in static/pre-rendered routes', () => { platform: 'javascript', transaction: '/test-static', transaction_info: { - source: 'url', + source: 'route', }, type: 'transaction', }); - expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static%2F'); // URL-encoded for 'GET /test-static/' + expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static'); // URL-encoded for 'GET /test-static' expect(baggageMetaTagContent).toContain('sentry-sampled=true'); await page.waitForTimeout(1000); // wait another sec to ensure no server transaction is sent diff --git a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts index 8267adcb4ea9..b7dda807c65c 100644 --- a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts @@ -30,11 +30,11 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { trace: { data: expect.objectContaining({ 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }), op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', span_id: expect.stringMatching(/[a-f0-9]{16}/), parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), @@ -56,7 +56,7 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => { timestamp: expect.any(Number), transaction: '/test-ssr', transaction_info: { - source: 'url', + source: 'route', }, type: 'transaction', }); @@ -193,18 +193,21 @@ test.describe('nested SSR routes (client, server, server request)', () => { span => span.op === 'http.client' && span.description?.includes('/api/user/'), ); + const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content'); + expect(routeNameMetaContent).toBe('%2Fuser-page%2F%5BuserId%5D'); + // Client pageload transaction - actual URL with pageload operation expect(clientPageloadTxn).toMatchObject({ - transaction: '/user-page/myUsername123', // todo: parametrize to '/user-page/[userId]' - transaction_info: { source: 'url' }, + transaction: '/user-page/[userId]', + transaction_info: { source: 'route' }, contexts: { trace: { op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', data: { 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }, }, }, @@ -274,20 +277,23 @@ test.describe('nested SSR routes (client, server, server request)', () => { await page.goto('/catchAll/hell0/whatever-do'); + const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content'); + expect(routeNameMetaContent).toBe('%2FcatchAll%2F%5B...path%5D'); + const clientPageloadTxn = await clientPageloadTxnPromise; const serverPageRequestTxn = await serverPageRequestTxnPromise; expect(clientPageloadTxn).toMatchObject({ - transaction: '/catchAll/hell0/whatever-do', // todo: parametrize to '/catchAll/[...path]' - transaction_info: { source: 'url' }, + transaction: '/catchAll/[...path]', + transaction_info: { source: 'route' }, contexts: { trace: { op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', data: { 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }, }, }, @@ -331,15 +337,15 @@ test.describe('parametrized vs static paths', () => { expect(clientPageloadTxn).toMatchObject({ transaction: '/user-page/settings', - transaction_info: { source: 'url' }, + transaction_info: { source: 'route' }, contexts: { trace: { op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', data: { 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }, }, }, diff --git a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts index dda88afe4714..202051e7a57e 100644 --- a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts @@ -32,11 +32,11 @@ test.describe('tracing in static routes with server islands', () => { trace: { data: expect.objectContaining({ 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }), op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', parent_span_id: metaParentSpanId, span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: metaTraceId, @@ -45,7 +45,7 @@ test.describe('tracing in static routes with server islands', () => { platform: 'javascript', transaction: '/server-island', transaction_info: { - source: 'url', + source: 'route', }, type: 'transaction', }); diff --git a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts index 90b26fb645e9..7593d6823d9b 100644 --- a/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts @@ -35,11 +35,11 @@ test.describe('tracing in static/pre-rendered routes', () => { trace: { data: expect.objectContaining({ 'sentry.op': 'pageload', - 'sentry.origin': 'auto.pageload.browser', - 'sentry.source': 'url', + 'sentry.origin': 'auto.pageload.astro', + 'sentry.source': 'route', }), op: 'pageload', - origin: 'auto.pageload.browser', + origin: 'auto.pageload.astro', parent_span_id: metaParentSpanId, span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: metaTraceId, @@ -48,7 +48,7 @@ test.describe('tracing in static/pre-rendered routes', () => { platform: 'javascript', transaction: '/test-static', transaction_info: { - source: 'url', + source: 'route', }, type: 'transaction', }); diff --git a/packages/astro/src/client/browserTracingIntegration.ts b/packages/astro/src/client/browserTracingIntegration.ts new file mode 100644 index 000000000000..7f8576671635 --- /dev/null +++ b/packages/astro/src/client/browserTracingIntegration.ts @@ -0,0 +1,52 @@ +import { browserTracingIntegration as originalBrowserTracingIntegration, WINDOW } from '@sentry/browser'; +import type { Integration, TransactionSource } from '@sentry/core'; +import { debug, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { DEBUG_BUILD } from '../debug-build'; + +/** + * Returns the value of a meta-tag + */ +function getMetaContent(metaName: string): string | undefined { + const optionalDocument = WINDOW.document as (typeof WINDOW)['document'] | undefined; + const metaTag = optionalDocument?.querySelector(`meta[name=${metaName}]`); + return metaTag?.getAttribute('content') || undefined; +} + +/** + * A custom browser tracing integrations for Astro. + */ +export function browserTracingIntegration( + options: Parameters[0] = {}, +): Integration { + const integration = originalBrowserTracingIntegration(options); + + return { + ...integration, + setup(client) { + // Original integration setup call + integration.setup?.(client); + + client.on('afterStartPageLoadSpan', pageLoadSpan => { + const routeNameFromMetaTags = getMetaContent('sentry-route-name'); + + if (routeNameFromMetaTags) { + let decodedRouteName; + try { + decodedRouteName = decodeURIComponent(routeNameFromMetaTags); + } catch { + // We ignore errors here, e.g. if the value cannot be URL decoded. + return; + } + + DEBUG_BUILD && debug.log(`[Tracing] Using route name from Sentry HTML meta-tag: ${decodedRouteName}`); + + pageLoadSpan.updateName(decodedRouteName); + pageLoadSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' as TransactionSource, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.astro', + }); + } + }); + }, + }; +} diff --git a/packages/astro/src/client/sdk.ts b/packages/astro/src/client/sdk.ts index f04725d1ef1e..21c5770f255f 100644 --- a/packages/astro/src/client/sdk.ts +++ b/packages/astro/src/client/sdk.ts @@ -1,11 +1,8 @@ import type { BrowserOptions } from '@sentry/browser'; -import { - browserTracingIntegration, - getDefaultIntegrations as getBrowserDefaultIntegrations, - init as initBrowserSdk, -} from '@sentry/browser'; +import { getDefaultIntegrations as getBrowserDefaultIntegrations, init as initBrowserSdk } from '@sentry/browser'; import type { Client, Integration } from '@sentry/core'; import { applySdkMetadata } from '@sentry/core'; +import { browserTracingIntegration } from './browserTracingIntegration'; // Tree-shakable guard to remove all code related to tracing declare const __SENTRY_TRACING__: boolean; diff --git a/packages/astro/src/debug-build.ts b/packages/astro/src/debug-build.ts new file mode 100644 index 000000000000..60aa50940582 --- /dev/null +++ b/packages/astro/src/debug-build.ts @@ -0,0 +1,8 @@ +declare const __DEBUG_BUILD__: boolean; + +/** + * This serves as a build time flag that will be true by default, but false in non-debug builds or if users replace `__SENTRY_DEBUG__` in their generated code. + * + * ATTENTION: This constant must never cross package boundaries (i.e. be exported) to guarantee that it can be used for tree shaking. + */ +export const DEBUG_BUILD = __DEBUG_BUILD__; diff --git a/packages/astro/src/index.client.ts b/packages/astro/src/index.client.ts index 2b85c05c3af1..55a6c8d3915b 100644 --- a/packages/astro/src/index.client.ts +++ b/packages/astro/src/index.client.ts @@ -1,3 +1,6 @@ export * from '@sentry/browser'; +// Override the browserTracingIntegration with the custom Astro version +export { browserTracingIntegration } from './client/browserTracingIntegration'; + export { init } from './client/sdk'; diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index ec9b8cd2f82f..9f04d5427fcf 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -213,7 +213,7 @@ async function instrumentRequest( try { for await (const chunk of bodyReporter()) { const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html); + const modifiedHtml = addMetaTagToHead(html, parametrizedRoute); controller.enqueue(new TextEncoder().encode(modifiedHtml)); } } catch (e) { @@ -253,11 +253,13 @@ async function instrumentRequest( * This function optimistically assumes that the HTML coming in chunks will not be split * within the tag. If this still happens, we simply won't replace anything. */ -function addMetaTagToHead(htmlChunk: string): string { +function addMetaTagToHead(htmlChunk: string, parametrizedRoute?: string): string { if (typeof htmlChunk !== 'string') { return htmlChunk; } - const metaTags = getTraceMetaTags(); + const metaTags = parametrizedRoute + ? `${getTraceMetaTags()}\n\n` + : getTraceMetaTags(); if (!metaTags) { return htmlChunk; @@ -317,26 +319,30 @@ export function interpolateRouteFromUrlAndParams( return acc.replace(key, `[${valuesToMultiSegmentParams[key]}]`); }, decodedUrlPathname); - return urlWithReplacedMultiSegmentParams - .split('/') - .map(segment => { - if (!segment) { - return ''; - } + return ( + urlWithReplacedMultiSegmentParams + .split('/') + .map(segment => { + if (!segment) { + return ''; + } - if (valuesToParams[segment]) { - return replaceWithParamName(segment); - } + if (valuesToParams[segment]) { + return replaceWithParamName(segment); + } - // astro permits multiple params in a single path segment, e.g. /[foo]-[bar]/ - const segmentParts = segment.split('-'); - if (segmentParts.length > 1) { - return segmentParts.map(part => replaceWithParamName(part)).join('-'); - } + // astro permits multiple params in a single path segment, e.g. /[foo]-[bar]/ + const segmentParts = segment.split('-'); + if (segmentParts.length > 1) { + return segmentParts.map(part => replaceWithParamName(part)).join('-'); + } - return segment; - }) - .join('/'); + return segment; + }) + .join('/') + // Remove trailing slash (only if it's not the only segment) + .replace(/^(.+?)\/$/, '$1') + ); } function tryDecodeUrl(url: string): string | undefined { diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index e8c16fa570d3..6430a5f47eb7 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -482,4 +482,11 @@ describe('interpolateRouteFromUrlAndParams', () => { const expectedRoute = '/usernames/user'; expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute); }); + + it('removes trailing slashes from the route', () => { + const rawUrl = '/users/123/'; + const params = { id: '123' }; + const expectedRoute = '/users/[id]'; + expect(interpolateRouteFromUrlAndParams(rawUrl, params)).toEqual(expectedRoute); + }); });