Skip to content

Commit 5441df9

Browse files
mydeaLms24
andauthored
fix(astro): Ensure traces are correctly propagated for static routes (#17536)
This refactors the astro middleware a bit to be more correct & future proof. Right now, http.server spans are not emitted by the node http integration because import-in-the-middle does not work with Astro. So we emit our own. This PR makes astro ready to also handle the base http.server spans and enhance them with routing information. It also handles static routes better: these do not emit spans anymore now, and do not continue traces, but instead only propagate the parametrized route to the client, no trace data. One fundamental problem remains that is not fixed in this PR: `Sentry.init()` is only injected via `page-ssr` into SSR pages. This means that only once any SSR page is hit, the server-side part of Sentry is initialized. If you hit a prerendered (static) page first, sentry will not be initialized and thus neither errors are captured nor spans created. For regular prerendered pages this should be fine because we do not need to run anything at runtime there. However, when you hit a prerendered page that has a server-island, the server island (which is dynamic) will not be instrumented either because Sentry is not initialized yet. I don't think we can fix this with the current set of primitives we have, so this is a future problem to look into... --------- Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
1 parent e7beae6 commit 5441df9

File tree

11 files changed

+447
-307
lines changed

11 files changed

+447
-307
lines changed

dev-packages/e2e-tests/test-applications/astro-4/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
"version": "0.0.1",
55
"scripts": {
66
"dev": "astro dev --force",
7-
"start": "astro dev",
87
"build": "astro check && astro build",
98
"preview": "astro preview",
9+
"start": "node ./dist/server/entry.mjs",
1010
"astro": "astro",
1111
"test:build": "pnpm install && pnpm build",
1212
"test:assert": "TEST_ENV=production playwright test"

dev-packages/e2e-tests/test-applications/astro-4/playwright.config.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ if (!testEnv) {
77
}
88

99
const config = getPlaywrightConfig({
10-
startCommand: 'node ./dist/server/entry.mjs',
10+
startCommand: 'pnpm start',
1111
});
1212

1313
export default config;

dev-packages/e2e-tests/test-applications/astro-4/tests/errors.client.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ test.describe('client-side errors', () => {
7070
contexts: {
7171
trace: {
7272
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
73-
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
7473
span_id: expect.stringMatching(/[a-f0-9]{16}/),
7574
},
7675
},

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ test.describe('tracing in static/pre-rendered routes', () => {
2121
const clientPageloadTraceId = clientPageloadTxn.contexts?.trace?.trace_id;
2222
const clientPageloadParentSpanId = clientPageloadTxn.contexts?.trace?.parent_span_id;
2323

24-
const sentryTraceMetaTagContent = await page.locator('meta[name="sentry-trace"]').getAttribute('content');
25-
const baggageMetaTagContent = await page.locator('meta[name="baggage"]').getAttribute('content');
24+
const sentryTraceMetaTags = await page.locator('meta[name="sentry-trace"]').count();
25+
expect(sentryTraceMetaTags).toBe(0);
2626

27-
const [metaTraceId, metaParentSpanId, metaSampled] = sentryTraceMetaTagContent?.split('-') || [];
27+
const baggageMetaTags = await page.locator('meta[name="baggage"]').count();
28+
expect(baggageMetaTags).toBe(0);
2829

2930
expect(clientPageloadTraceId).toMatch(/[a-f0-9]{32}/);
30-
expect(clientPageloadParentSpanId).toMatch(/[a-f0-9]{16}/);
31-
expect(metaSampled).toBe('1');
31+
expect(clientPageloadParentSpanId).toBeUndefined();
3232

3333
expect(clientPageloadTxn).toMatchObject({
3434
contexts: {
@@ -40,9 +40,8 @@ test.describe('tracing in static/pre-rendered routes', () => {
4040
}),
4141
op: 'pageload',
4242
origin: 'auto.pageload.astro',
43-
parent_span_id: metaParentSpanId,
4443
span_id: expect.stringMatching(/[a-f0-9]{16}/),
45-
trace_id: metaTraceId,
44+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
4645
},
4746
},
4847
platform: 'javascript',
@@ -53,9 +52,6 @@ test.describe('tracing in static/pre-rendered routes', () => {
5352
type: 'transaction',
5453
});
5554

56-
expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static'); // URL-encoded for 'GET /test-static'
57-
expect(baggageMetaTagContent).toContain('sentry-sampled=true');
58-
5955
await page.waitForTimeout(1000); // wait another sec to ensure no server transaction is sent
6056
});
6157
});

dev-packages/e2e-tests/test-applications/astro-5/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"build": "astro build",
88
"preview": "astro preview",
99
"astro": "astro",
10+
"start": "node ./dist/server/entry.mjs",
1011
"test:build": "pnpm install && pnpm build",
1112
"test:assert": "TEST_ENV=production playwright test"
1213
},

dev-packages/e2e-tests/test-applications/astro-5/playwright.config.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ if (!testEnv) {
77
}
88

99
const config = getPlaywrightConfig({
10-
startCommand: 'node ./dist/server/entry.mjs',
10+
startCommand: 'pnpm start',
1111
});
1212

1313
export default config;

dev-packages/e2e-tests/test-applications/astro-5/tests/errors.client.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ test.describe('client-side errors', () => {
7070
contexts: {
7171
trace: {
7272
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
73-
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
7473
span_id: expect.stringMatching(/[a-f0-9]{16}/),
7574
},
7675
},

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

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,27 @@ import { waitForTransaction } from '@sentry-internal/test-utils';
44
test.describe('tracing in static routes with server islands', () => {
55
test('only sends client pageload transaction and server island endpoint transaction', async ({ page }) => {
66
const clientPageloadTxnPromise = waitForTransaction('astro-5', txnEvent => {
7-
return txnEvent?.transaction === '/server-island';
7+
return txnEvent.transaction === '/server-island';
88
});
99

1010
const serverIslandEndpointTxnPromise = waitForTransaction('astro-5', evt => {
11-
return !!evt.transaction?.startsWith('GET /_server-islands');
11+
return evt.transaction === 'GET /_server-islands/[name]';
1212
});
1313

1414
await page.goto('/server-island');
1515

1616
const clientPageloadTxn = await clientPageloadTxnPromise;
17-
1817
const clientPageloadTraceId = clientPageloadTxn.contexts?.trace?.trace_id;
1918
const clientPageloadParentSpanId = clientPageloadTxn.contexts?.trace?.parent_span_id;
2019

21-
const sentryTraceMetaTagContent = await page.locator('meta[name="sentry-trace"]').getAttribute('content');
22-
const baggageMetaTagContent = await page.locator('meta[name="baggage"]').getAttribute('content');
20+
const sentryTraceMetaTags = await page.locator('meta[name="sentry-trace"]').count();
21+
expect(sentryTraceMetaTags).toBe(0);
2322

24-
const [metaTraceId, metaParentSpanId, metaSampled] = sentryTraceMetaTagContent?.split('-') || [];
23+
const baggageMetaTags = await page.locator('meta[name="baggage"]').count();
24+
expect(baggageMetaTags).toBe(0);
2525

2626
expect(clientPageloadTraceId).toMatch(/[a-f0-9]{32}/);
27-
expect(clientPageloadParentSpanId).toMatch(/[a-f0-9]{16}/);
28-
expect(metaSampled).toBe('1');
27+
expect(clientPageloadParentSpanId).toBeUndefined();
2928

3029
expect(clientPageloadTxn).toMatchObject({
3130
contexts: {
@@ -37,9 +36,8 @@ test.describe('tracing in static routes with server islands', () => {
3736
}),
3837
op: 'pageload',
3938
origin: 'auto.pageload.astro',
40-
parent_span_id: metaParentSpanId,
4139
span_id: expect.stringMatching(/[a-f0-9]{16}/),
42-
trace_id: metaTraceId,
40+
trace_id: clientPageloadTraceId,
4341
},
4442
},
4543
platform: 'javascript',
@@ -63,9 +61,6 @@ test.describe('tracing in static routes with server islands', () => {
6361
]),
6462
);
6563

66-
expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Fserver-island'); // URL-encoded for 'GET /server-island'
67-
expect(baggageMetaTagContent).toContain('sentry-sampled=true');
68-
6964
const serverIslandEndpointTxn = await serverIslandEndpointTxnPromise;
7065

7166
expect(serverIslandEndpointTxn).toMatchObject({

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ test.describe('tracing in static/pre-rendered routes', () => {
2121
const clientPageloadTraceId = clientPageloadTxn.contexts?.trace?.trace_id;
2222
const clientPageloadParentSpanId = clientPageloadTxn.contexts?.trace?.parent_span_id;
2323

24-
const sentryTraceMetaTagContent = await page.locator('meta[name="sentry-trace"]').getAttribute('content');
25-
const baggageMetaTagContent = await page.locator('meta[name="baggage"]').getAttribute('content');
24+
const sentryTraceMetaTags = await page.locator('meta[name="sentry-trace"]').count();
25+
expect(sentryTraceMetaTags).toBe(0);
2626

27-
const [metaTraceId, metaParentSpanId, metaSampled] = sentryTraceMetaTagContent?.split('-') || [];
27+
const baggageMetaTags = await page.locator('meta[name="baggage"]').count();
28+
expect(baggageMetaTags).toBe(0);
2829

2930
expect(clientPageloadTraceId).toMatch(/[a-f0-9]{32}/);
30-
expect(clientPageloadParentSpanId).toMatch(/[a-f0-9]{16}/);
31-
expect(metaSampled).toBe('1');
31+
expect(clientPageloadParentSpanId).toBeUndefined();
3232

3333
expect(clientPageloadTxn).toMatchObject({
3434
contexts: {
@@ -40,9 +40,8 @@ test.describe('tracing in static/pre-rendered routes', () => {
4040
}),
4141
op: 'pageload',
4242
origin: 'auto.pageload.astro',
43-
parent_span_id: metaParentSpanId,
4443
span_id: expect.stringMatching(/[a-f0-9]{16}/),
45-
trace_id: metaTraceId,
44+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
4645
},
4746
},
4847
platform: 'javascript',
@@ -53,9 +52,6 @@ test.describe('tracing in static/pre-rendered routes', () => {
5352
type: 'transaction',
5453
});
5554

56-
expect(baggageMetaTagContent).toContain('sentry-transaction=GET%20%2Ftest-static'); // URL-encoded for 'GET /test-static'
57-
expect(baggageMetaTagContent).toContain('sentry-sampled=true');
58-
5955
await page.waitForTimeout(1000); // wait another sec to ensure no server transaction is sent
6056
});
6157
});

0 commit comments

Comments
 (0)