Skip to content

Commit 43fdebb

Browse files
s1gr1ds1gr1d
committed
review suggestions
1 parent 8fcfe9b commit 43fdebb

File tree

7 files changed

+94
-23
lines changed

7 files changed

+94
-23
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: 58 additions & 6 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',
33+
'sentry.origin': 'auto.pageload.astro',
3434
'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}/),
@@ -200,10 +200,10 @@ test.describe('nested SSR routes (client, server, server request)', () => {
200200
contexts: {
201201
trace: {
202202
op: 'pageload',
203-
origin: 'auto.pageload.browser',
203+
origin: 'auto.pageload.astro',
204204
data: {
205205
'sentry.op': 'pageload',
206-
'sentry.origin': 'auto.pageload.browser',
206+
'sentry.origin': 'auto.pageload.astro',
207207
'sentry.source': 'route',
208208
},
209209
},
@@ -286,10 +286,10 @@ test.describe('nested SSR routes (client, server, server request)', () => {
286286
contexts: {
287287
trace: {
288288
op: 'pageload',
289-
origin: 'auto.pageload.browser',
289+
origin: 'auto.pageload.astro',
290290
data: {
291291
'sentry.op': 'pageload',
292-
'sentry.origin': 'auto.pageload.browser',
292+
'sentry.origin': 'auto.pageload.astro',
293293
'sentry.source': 'route',
294294
},
295295
},
@@ -315,3 +315,55 @@ test.describe('nested SSR routes (client, server, server request)', () => {
315315
});
316316
});
317317
});
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: 2 additions & 2 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',
38+
'sentry.origin': 'auto.pageload.astro',
3939
'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,

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

Lines changed: 8 additions & 8 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',
33+
'sentry.origin': 'auto.pageload.astro',
3434
'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}/),
@@ -203,10 +203,10 @@ test.describe('nested SSR routes (client, server, server request)', () => {
203203
contexts: {
204204
trace: {
205205
op: 'pageload',
206-
origin: 'auto.pageload.browser',
206+
origin: 'auto.pageload.astro',
207207
data: {
208208
'sentry.op': 'pageload',
209-
'sentry.origin': 'auto.pageload.browser',
209+
'sentry.origin': 'auto.pageload.astro',
210210
'sentry.source': 'route',
211211
},
212212
},
@@ -289,10 +289,10 @@ test.describe('nested SSR routes (client, server, server request)', () => {
289289
contexts: {
290290
trace: {
291291
op: 'pageload',
292-
origin: 'auto.pageload.browser',
292+
origin: 'auto.pageload.astro',
293293
data: {
294294
'sentry.op': 'pageload',
295-
'sentry.origin': 'auto.pageload.browser',
295+
'sentry.origin': 'auto.pageload.astro',
296296
'sentry.source': 'route',
297297
},
298298
},
@@ -341,10 +341,10 @@ test.describe('parametrized vs static paths', () => {
341341
contexts: {
342342
trace: {
343343
op: 'pageload',
344-
origin: 'auto.pageload.browser',
344+
origin: 'auto.pageload.astro',
345345
data: {
346346
'sentry.op': 'pageload',
347-
'sentry.origin': 'auto.pageload.browser',
347+
'sentry.origin': 'auto.pageload.astro',
348348
'sentry.source': 'route',
349349
},
350350
},

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

Lines changed: 2 additions & 2 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',
35+
'sentry.origin': 'auto.pageload.astro',
3636
'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,

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

Lines changed: 2 additions & 2 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',
38+
'sentry.origin': 'auto.pageload.astro',
3939
'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,

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { browserTracingIntegration as originalBrowserTracingIntegration, WINDOW } from '@sentry/browser';
22
import type { Integration, TransactionSource } from '@sentry/core';
3-
import { debug, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
3+
import { debug, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
44
import { DEBUG_BUILD } from '../debug-build';
55

66
/**
@@ -30,12 +30,21 @@ export function browserTracingIntegration(
3030
const routeNameFromMetaTags = getMetaContent('sentry-route-name');
3131

3232
if (routeNameFromMetaTags) {
33-
const decodedRouteName = decodeURIComponent(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+
}
3440

3541
DEBUG_BUILD && debug.log(`[Tracing] Using route name from Sentry HTML meta-tag: ${decodedRouteName}`);
3642

3743
pageLoadSpan.updateName(decodedRouteName);
38-
pageLoadSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route' as TransactionSource);
44+
pageLoadSpan.setAttributes({
45+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' as TransactionSource,
46+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.astro',
47+
});
3948
}
4049
});
4150
},

0 commit comments

Comments
 (0)