Skip to content

Commit d6eaacf

Browse files
s1gr1ds1gr1d
authored
feat(v9/astro): Parametrize routes on client-side (#17143)
Backport of #17133
1 parent 7c2e347 commit d6eaacf

File tree

12 files changed

+218
-75
lines changed

12 files changed

+218
-75
lines changed

‎dev-packages/e2e-tests/test-applications/astro-4/src/pages/user-page/settings.astro‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
import Layout from '../../layouts/Layout.astro';
3+
4+
export const prerender = false;
5+
6+
---
7+
8+
<Layout title="User Settings">
9+
<h1>User Settings</h1>
10+
</Layout>

‎dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.dynamic.test.ts‎

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
3030
trace: {
3131
data: expect.objectContaining({
3232
'sentry.op': 'pageload',
33-
'sentry.origin': 'auto.pageload.browser',
34-
'sentry.source': 'url',
33+
'sentry.origin': 'auto.pageload.astro',
34+
'sentry.source': 'route',
3535
}),
3636
op: 'pageload',
37-
origin: 'auto.pageload.browser',
37+
origin: 'auto.pageload.astro',
3838
span_id: expect.stringMatching(/[a-f0-9]{16}/),
3939
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
4040
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
@@ -55,9 +55,7 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
5555
start_timestamp: expect.any(Number),
5656
timestamp: expect.any(Number),
5757
transaction: '/test-ssr',
58-
transaction_info: {
59-
source: 'url',
60-
},
58+
transaction_info: { source: 'route' },
6159
type: 'transaction',
6260
});
6361

@@ -113,9 +111,7 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
113111
start_timestamp: expect.any(Number),
114112
timestamp: expect.any(Number),
115113
transaction: 'GET /test-ssr',
116-
transaction_info: {
117-
source: 'route',
118-
},
114+
transaction_info: { source: 'route' },
119115
type: 'transaction',
120116
});
121117
});
@@ -194,18 +190,21 @@ test.describe('nested SSR routes (client, server, server request)', () => {
194190
span => span.op === 'http.client' && span.description?.includes('/api/user/'),
195191
);
196192

193+
const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content');
194+
expect(routeNameMetaContent).toBe('%2Fuser-page%2F%5BuserId%5D');
195+
197196
// Client pageload transaction - actual URL with pageload operation
198197
expect(clientPageloadTxn).toMatchObject({
199-
transaction: '/user-page/myUsername123', // todo: parametrize
200-
transaction_info: { source: 'url' },
198+
transaction: '/user-page/[userId]',
199+
transaction_info: { source: 'route' },
201200
contexts: {
202201
trace: {
203202
op: 'pageload',
204-
origin: 'auto.pageload.browser',
203+
origin: 'auto.pageload.astro',
205204
data: {
206205
'sentry.op': 'pageload',
207-
'sentry.origin': 'auto.pageload.browser',
208-
'sentry.source': 'url',
206+
'sentry.origin': 'auto.pageload.astro',
207+
'sentry.source': 'route',
209208
},
210209
},
211210
},
@@ -275,20 +274,23 @@ test.describe('nested SSR routes (client, server, server request)', () => {
275274

276275
await page.goto('/catchAll/hell0/whatever-do');
277276

277+
const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content');
278+
expect(routeNameMetaContent).toBe('%2FcatchAll%2F%5Bpath%5D');
279+
278280
const clientPageloadTxn = await clientPageloadTxnPromise;
279281
const serverPageRequestTxn = await serverPageRequestTxnPromise;
280282

281283
expect(clientPageloadTxn).toMatchObject({
282-
transaction: '/catchAll/hell0/whatever-do', // todo: parametrize
283-
transaction_info: { source: 'url' },
284+
transaction: '/catchAll/[path]',
285+
transaction_info: { source: 'route' },
284286
contexts: {
285287
trace: {
286288
op: 'pageload',
287-
origin: 'auto.pageload.browser',
289+
origin: 'auto.pageload.astro',
288290
data: {
289291
'sentry.op': 'pageload',
290-
'sentry.origin': 'auto.pageload.browser',
291-
'sentry.source': 'url',
292+
'sentry.origin': 'auto.pageload.astro',
293+
'sentry.source': 'route',
292294
},
293295
},
294296
},
@@ -313,3 +315,55 @@ test.describe('nested SSR routes (client, server, server request)', () => {
313315
});
314316
});
315317
});
318+
319+
// Case for `user-page/[id]` vs. `user-page/settings` static routes
320+
test.describe('parametrized vs static paths', () => {
321+
test('should use static route name for static route in parametrized path', async ({ page }) => {
322+
const clientPageloadTxnPromise = waitForTransaction('astro-4', txnEvent => {
323+
return txnEvent?.transaction?.startsWith('/user-page/') ?? false;
324+
});
325+
326+
const serverPageRequestTxnPromise = waitForTransaction('astro-4', txnEvent => {
327+
return txnEvent?.transaction?.startsWith('GET /user-page/') ?? false;
328+
});
329+
330+
await page.goto('/user-page/settings');
331+
332+
const clientPageloadTxn = await clientPageloadTxnPromise;
333+
const serverPageRequestTxn = await serverPageRequestTxnPromise;
334+
335+
expect(clientPageloadTxn).toMatchObject({
336+
transaction: '/user-page/settings',
337+
transaction_info: { source: 'route' },
338+
contexts: {
339+
trace: {
340+
op: 'pageload',
341+
origin: 'auto.pageload.astro',
342+
data: {
343+
'sentry.op': 'pageload',
344+
'sentry.origin': 'auto.pageload.astro',
345+
'sentry.source': 'route',
346+
},
347+
},
348+
},
349+
});
350+
351+
expect(serverPageRequestTxn).toMatchObject({
352+
transaction: 'GET /user-page/settings',
353+
transaction_info: { source: 'route' },
354+
contexts: {
355+
trace: {
356+
op: 'http.server',
357+
origin: 'auto.http.astro',
358+
data: {
359+
'sentry.op': 'http.server',
360+
'sentry.origin': 'auto.http.astro',
361+
'sentry.source': 'route',
362+
url: expect.stringContaining('/user-page/settings'),
363+
},
364+
},
365+
},
366+
request: { url: expect.stringContaining('/user-page/settings') },
367+
});
368+
});
369+
});

‎dev-packages/e2e-tests/test-applications/astro-4/tests/tracing.static.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ test.describe('tracing in static/pre-rendered routes', () => {
3535
trace: {
3636
data: expect.objectContaining({
3737
'sentry.op': 'pageload',
38-
'sentry.origin': 'auto.pageload.browser',
39-
'sentry.source': 'url',
38+
'sentry.origin': 'auto.pageload.astro',
39+
'sentry.source': 'route',
4040
}),
4141
op: 'pageload',
42-
origin: 'auto.pageload.browser',
42+
origin: 'auto.pageload.astro',
4343
parent_span_id: metaParentSpanId,
4444
span_id: expect.stringMatching(/[a-f0-9]{16}/),
4545
trace_id: metaTraceId,
@@ -48,12 +48,12 @@ test.describe('tracing in static/pre-rendered routes', () => {
4848
platform: 'javascript',
4949
transaction: '/test-static',
5050
transaction_info: {
51-
source: 'url',
51+
source: 'route',
5252
},
5353
type: 'transaction',
5454
});
5555

56-
expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static%2F'); // URL-encoded for 'GET /test-static/'
56+
expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static'); // URL-encoded for 'GET /test-static'
5757
expect(baggageMetaTagContent).toContain('sentry-sampled=true');
5858

5959
await page.waitForTimeout(1000); // wait another sec to ensure no server transaction is sent

‎dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.dynamic.test.ts‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
3030
trace: {
3131
data: expect.objectContaining({
3232
'sentry.op': 'pageload',
33-
'sentry.origin': 'auto.pageload.browser',
34-
'sentry.source': 'url',
33+
'sentry.origin': 'auto.pageload.astro',
34+
'sentry.source': 'route',
3535
}),
3636
op: 'pageload',
37-
origin: 'auto.pageload.browser',
37+
origin: 'auto.pageload.astro',
3838
span_id: expect.stringMatching(/[a-f0-9]{16}/),
3939
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
4040
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
@@ -56,7 +56,7 @@ test.describe('tracing in dynamically rendered (ssr) routes', () => {
5656
timestamp: expect.any(Number),
5757
transaction: '/test-ssr',
5858
transaction_info: {
59-
source: 'url',
59+
source: 'route',
6060
},
6161
type: 'transaction',
6262
});
@@ -193,18 +193,21 @@ test.describe('nested SSR routes (client, server, server request)', () => {
193193
span => span.op === 'http.client' && span.description?.includes('/api/user/'),
194194
);
195195

196+
const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content');
197+
expect(routeNameMetaContent).toBe('%2Fuser-page%2F%5BuserId%5D');
198+
196199
// Client pageload transaction - actual URL with pageload operation
197200
expect(clientPageloadTxn).toMatchObject({
198-
transaction: '/user-page/myUsername123', // todo: parametrize to '/user-page/[userId]'
199-
transaction_info: { source: 'url' },
201+
transaction: '/user-page/[userId]',
202+
transaction_info: { source: 'route' },
200203
contexts: {
201204
trace: {
202205
op: 'pageload',
203-
origin: 'auto.pageload.browser',
206+
origin: 'auto.pageload.astro',
204207
data: {
205208
'sentry.op': 'pageload',
206-
'sentry.origin': 'auto.pageload.browser',
207-
'sentry.source': 'url',
209+
'sentry.origin': 'auto.pageload.astro',
210+
'sentry.source': 'route',
208211
},
209212
},
210213
},
@@ -274,20 +277,23 @@ test.describe('nested SSR routes (client, server, server request)', () => {
274277

275278
await page.goto('/catchAll/hell0/whatever-do');
276279

280+
const routeNameMetaContent = await page.locator('meta[name="sentry-route-name"]').getAttribute('content');
281+
expect(routeNameMetaContent).toBe('%2FcatchAll%2F%5B...path%5D');
282+
277283
const clientPageloadTxn = await clientPageloadTxnPromise;
278284
const serverPageRequestTxn = await serverPageRequestTxnPromise;
279285

280286
expect(clientPageloadTxn).toMatchObject({
281-
transaction: '/catchAll/hell0/whatever-do', // todo: parametrize to '/catchAll/[...path]'
282-
transaction_info: { source: 'url' },
287+
transaction: '/catchAll/[...path]',
288+
transaction_info: { source: 'route' },
283289
contexts: {
284290
trace: {
285291
op: 'pageload',
286-
origin: 'auto.pageload.browser',
292+
origin: 'auto.pageload.astro',
287293
data: {
288294
'sentry.op': 'pageload',
289-
'sentry.origin': 'auto.pageload.browser',
290-
'sentry.source': 'url',
295+
'sentry.origin': 'auto.pageload.astro',
296+
'sentry.source': 'route',
291297
},
292298
},
293299
},
@@ -331,15 +337,15 @@ test.describe('parametrized vs static paths', () => {
331337

332338
expect(clientPageloadTxn).toMatchObject({
333339
transaction: '/user-page/settings',
334-
transaction_info: { source: 'url' },
340+
transaction_info: { source: 'route' },
335341
contexts: {
336342
trace: {
337343
op: 'pageload',
338-
origin: 'auto.pageload.browser',
344+
origin: 'auto.pageload.astro',
339345
data: {
340346
'sentry.op': 'pageload',
341-
'sentry.origin': 'auto.pageload.browser',
342-
'sentry.source': 'url',
347+
'sentry.origin': 'auto.pageload.astro',
348+
'sentry.source': 'route',
343349
},
344350
},
345351
},

‎dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.serverIslands.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ test.describe('tracing in static routes with server islands', () => {
3232
trace: {
3333
data: expect.objectContaining({
3434
'sentry.op': 'pageload',
35-
'sentry.origin': 'auto.pageload.browser',
36-
'sentry.source': 'url',
35+
'sentry.origin': 'auto.pageload.astro',
36+
'sentry.source': 'route',
3737
}),
3838
op: 'pageload',
39-
origin: 'auto.pageload.browser',
39+
origin: 'auto.pageload.astro',
4040
parent_span_id: metaParentSpanId,
4141
span_id: expect.stringMatching(/[a-f0-9]{16}/),
4242
trace_id: metaTraceId,
@@ -45,7 +45,7 @@ test.describe('tracing in static routes with server islands', () => {
4545
platform: 'javascript',
4646
transaction: '/server-island',
4747
transaction_info: {
48-
source: 'url',
48+
source: 'route',
4949
},
5050
type: 'transaction',
5151
});

‎dev-packages/e2e-tests/test-applications/astro-5/tests/tracing.static.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ test.describe('tracing in static/pre-rendered routes', () => {
3535
trace: {
3636
data: expect.objectContaining({
3737
'sentry.op': 'pageload',
38-
'sentry.origin': 'auto.pageload.browser',
39-
'sentry.source': 'url',
38+
'sentry.origin': 'auto.pageload.astro',
39+
'sentry.source': 'route',
4040
}),
4141
op: 'pageload',
42-
origin: 'auto.pageload.browser',
42+
origin: 'auto.pageload.astro',
4343
parent_span_id: metaParentSpanId,
4444
span_id: expect.stringMatching(/[a-f0-9]{16}/),
4545
trace_id: metaTraceId,
@@ -48,7 +48,7 @@ test.describe('tracing in static/pre-rendered routes', () => {
4848
platform: 'javascript',
4949
transaction: '/test-static',
5050
transaction_info: {
51-
source: 'url',
51+
source: 'route',
5252
},
5353
type: 'transaction',
5454
});

‎packages/astro/src/client/browserTracingIntegration.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { browserTracingIntegration as originalBrowserTracingIntegration, WINDOW } from '@sentry/browser';
2+
import type { Integration, TransactionSource } from '@sentry/core';
3+
import { debug, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
4+
import { DEBUG_BUILD } from '../debug-build';
5+
6+
/**
7+
* Returns the value of a meta-tag
8+
*/
9+
function getMetaContent(metaName: string): string | undefined {
10+
const optionalDocument = WINDOW.document as (typeof WINDOW)['document'] | undefined;
11+
const metaTag = optionalDocument?.querySelector(`meta[name=${metaName}]`);
12+
return metaTag?.getAttribute('content') || undefined;
13+
}
14+
15+
/**
16+
* A custom browser tracing integrations for Astro.
17+
*/
18+
export function browserTracingIntegration(
19+
options: Parameters<typeof originalBrowserTracingIntegration>[0] = {},
20+
): Integration {
21+
const integration = originalBrowserTracingIntegration(options);
22+
23+
return {
24+
...integration,
25+
setup(client) {
26+
// Original integration setup call
27+
integration.setup?.(client);
28+
29+
client.on('afterStartPageLoadSpan', pageLoadSpan => {
30+
const routeNameFromMetaTags = getMetaContent('sentry-route-name');
31+
32+
if (routeNameFromMetaTags) {
33+
let decodedRouteName;
34+
try {
35+
decodedRouteName = decodeURIComponent(routeNameFromMetaTags);
36+
} catch {
37+
// We ignore errors here, e.g. if the value cannot be URL decoded.
38+
return;
39+
}
40+
41+
DEBUG_BUILD && debug.log(`[Tracing] Using route name from Sentry HTML meta-tag: ${decodedRouteName}`);
42+
43+
pageLoadSpan.updateName(decodedRouteName);
44+
pageLoadSpan.setAttributes({
45+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' as TransactionSource,
46+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.astro',
47+
});
48+
}
49+
});
50+
},
51+
};
52+
}

0 commit comments

Comments
 (0)