Skip to content

Commit b96fd7f

Browse files
mydeamydea
committed
WIP: nextjs http.client spans
1 parent 56c9fcb commit b96fd7f

File tree

8 files changed

+211
-63
lines changed

8 files changed

+211
-63
lines changed

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/edge/route.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { NextResponse } from 'next/server';
55
export const runtime = 'edge';
66

77
export async function PATCH() {
8+
// Test that actual fetch requests are captured
9+
await fetch('https://github.com');
810
return NextResponse.json({ name: 'John Doe' }, { status: 401 });
911
}
1012

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts

Lines changed: 0 additions & 61 deletions
This file was deleted.

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-router-edge-route.test.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
3+
4+
test('Should create a transaction for edge routes', async ({ request }) => {
5+
const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
6+
return (
7+
transactionEvent?.transaction === 'GET /api/edge-endpoint' &&
8+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
9+
);
10+
});
11+
12+
const response = await request.get('/api/edge-endpoint', {
13+
headers: {
14+
'x-yeet': 'test-value',
15+
},
16+
});
17+
expect(await response.json()).toStrictEqual({ name: 'Jim Halpert' });
18+
19+
const edgerouteTransaction = await edgerouteTransactionPromise;
20+
21+
expect(edgerouteTransaction.contexts?.trace?.status).toBe('ok');
22+
expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server');
23+
expect(edgerouteTransaction.request?.headers?.['x-yeet']).toBe('test-value');
24+
});
25+
26+
test('Faulty edge routes', async ({ request }) => {
27+
const edgerouteTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => {
28+
return (
29+
transactionEvent?.transaction === 'GET /api/error-edge-endpoint' &&
30+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
31+
);
32+
});
33+
34+
const errorEventPromise = waitForError('nextjs-app-dir', errorEvent => {
35+
return (
36+
errorEvent?.exception?.values?.[0]?.value === 'Edge Route Error' &&
37+
errorEvent.contexts?.runtime?.name === 'vercel-edge'
38+
);
39+
});
40+
41+
request.get('/api/error-edge-endpoint').catch(() => {
42+
// Noop
43+
});
44+
45+
const [edgerouteTransaction, errorEvent] = await Promise.all([
46+
test.step('should create a transaction', () => edgerouteTransactionPromise),
47+
test.step('should create an error event', () => errorEventPromise),
48+
]);
49+
50+
test.step('should create transactions with the right fields', () => {
51+
expect(edgerouteTransaction.contexts?.trace?.status).toBe('unknown_error');
52+
expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server');
53+
});
54+
55+
test.step('should have scope isolation', () => {
56+
expect(edgerouteTransaction.tags?.['my-isolated-tag']).toBe(true);
57+
expect(edgerouteTransaction.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
58+
expect(errorEvent.tags?.['my-isolated-tag']).toBe(true);
59+
expect(errorEvent.tags?.['my-global-scope-isolated-tag']).not.toBeDefined();
60+
});
61+
});
62+
63+
test('Should not create spans for outgoing Sentry requests on edge routes', async ({ request }) => {
64+
// Ensure no http.client transaction is created for any orphaned request
65+
waitForTransaction('nextjs-app-dir', async transactionEvent => {
66+
if (transactionEvent.contexts?.trace?.op === 'http.client') {
67+
throw new Error(`Should not receive http.client transaction, but got: ${transactionEvent.transaction}`);
68+
}
69+
return false;
70+
});
71+
72+
// We hit the endpoint three times and check that nowhere a http.client span for Sentry is to be found
73+
// this way, we ensure that nothing is sent to Sentry in a follow up span
74+
const edgerouteTransactionPromise1 = waitForTransaction('nextjs-app-dir', async transactionEvent => {
75+
return (
76+
transactionEvent?.transaction === 'GET /api/edge-endpoint' &&
77+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
78+
);
79+
});
80+
81+
await request.get('/api/edge-endpoint', {
82+
headers: {
83+
'x-yeet': 'test-value',
84+
},
85+
});
86+
87+
const edgerouteTransactionPromise2 = waitForTransaction('nextjs-app-dir', async transactionEvent => {
88+
return (
89+
transactionEvent?.transaction === 'GET /api/edge-endpoint' &&
90+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
91+
);
92+
});
93+
94+
await request.get('/api/edge-endpoint', {
95+
headers: {
96+
'x-yeet': 'test-value-2',
97+
},
98+
});
99+
100+
const edgerouteTransactionPromise3 = waitForTransaction('nextjs-app-dir', async transactionEvent => {
101+
return (
102+
transactionEvent?.transaction === 'GET /api/edge-endpoint' &&
103+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
104+
);
105+
});
106+
107+
await request.get('/api/edge-endpoint', {
108+
headers: {
109+
'x-yeet': 'test-value-3',
110+
},
111+
});
112+
113+
const [edgerouteTransaction1, edgerouteTransaction2, edgerouteTransaction3] = await Promise.all([
114+
edgerouteTransactionPromise1,
115+
edgerouteTransactionPromise2,
116+
edgerouteTransactionPromise3,
117+
]);
118+
119+
expect(edgerouteTransaction1.contexts?.trace?.op).toBe('http.server');
120+
expect(edgerouteTransaction2.contexts?.trace?.op).toBe('http.server');
121+
expect(edgerouteTransaction3.contexts?.trace?.op).toBe('http.server');
122+
123+
expect(edgerouteTransaction1.spans?.length).toBe(1);
124+
expect(edgerouteTransaction2.spans?.length).toBe(1);
125+
expect(edgerouteTransaction3.spans?.length).toBe(1);
126+
127+
expect(edgerouteTransaction1.spans?.[0].description).toBe('handler (/api/edge-endpoint)');
128+
expect(edgerouteTransaction2.spans?.[0].description).toBe('handler (/api/edge-endpoint)');
129+
expect(edgerouteTransaction3.spans?.[0].description).toBe('handler (/api/edge-endpoint)');
130+
});

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,63 @@ test.describe('Edge runtime', () => {
119119

120120
expect(routehandlerError.transaction).toBe('DELETE /route-handlers/[param]/edge');
121121
});
122+
123+
test('should not create spans for outgoing Sentry requests on edge routes', async ({ request }) => {
124+
// Ensure no http.client transaction is created for any orphaned request
125+
waitForTransaction('nextjs-app-dir', async transactionEvent => {
126+
if (transactionEvent.contexts?.trace?.op === 'http.client') {
127+
throw new Error(`Should not receive http.client transaction, but got: ${transactionEvent.transaction}`);
128+
}
129+
return false;
130+
});
131+
132+
// We hit the endpoint three times and check that nowhere a http.client span for Sentry is to be found
133+
// this way, we ensure that nothing is sent to Sentry in a follow up span
134+
const edgerouteTransactionPromise1 = waitForTransaction('nextjs-app-dir', async transactionEvent => {
135+
return (
136+
transactionEvent?.transaction === 'PATCH /route-handlers/[param]/edge' &&
137+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
138+
);
139+
});
140+
141+
await request.patch('/route-handlers/bar/edge');
142+
143+
const edgerouteTransactionPromise2 = waitForTransaction('nextjs-app-dir', async transactionEvent => {
144+
return (
145+
transactionEvent?.transaction === 'PATCH /route-handlers/[param]/edge' &&
146+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
147+
);
148+
});
149+
150+
await request.patch('/route-handlers/bar/edge');
151+
152+
const edgerouteTransactionPromise3 = waitForTransaction('nextjs-app-dir', async transactionEvent => {
153+
return (
154+
transactionEvent?.transaction === 'PATCH /route-handlers/[param]/edge' &&
155+
transactionEvent.contexts?.runtime?.name === 'vercel-edge'
156+
);
157+
});
158+
159+
await request.patch('/route-handlers/bar/edge');
160+
161+
const [edgerouteTransaction1, edgerouteTransaction2, edgerouteTransaction3] = await Promise.all([
162+
edgerouteTransactionPromise1,
163+
edgerouteTransactionPromise2,
164+
edgerouteTransactionPromise3,
165+
]);
166+
167+
expect(edgerouteTransaction1.contexts?.trace?.op).toBe('http.server');
168+
expect(edgerouteTransaction2.contexts?.trace?.op).toBe('http.server');
169+
expect(edgerouteTransaction3.contexts?.trace?.op).toBe('http.server');
170+
171+
expect(edgerouteTransaction1.spans?.length).toBe(1);
172+
expect(edgerouteTransaction2.spans?.length).toBe(1);
173+
expect(edgerouteTransaction3.spans?.length).toBe(1);
174+
175+
expect(edgerouteTransaction1.spans?.[0].description).toBe('GET https://github.com');
176+
expect(edgerouteTransaction2.spans?.[0].description).toBe('GET https://github.com');
177+
expect(edgerouteTransaction3.spans?.[0].description).toBe('GET https://github.com');
178+
});
122179
});
123180

124181
test('should not crash route handlers that are configured with `export const dynamic = "error"`', async ({

‎packages/node/src/integrations/node-fetch/SentryNodeFetchInstrumentation.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import * as diagch from 'diagnostics_channel';
1717
import { NODE_MAJOR, NODE_MINOR } from '../../nodeVersion';
1818
import { mergeBaggageHeaders } from '../../utils/baggage';
1919
import type { UndiciRequest, UndiciResponse } from './types';
20+
import { isNextEdgeRuntime } from '../../utils/isNextEdgeRuntime';
2021

2122
const SENTRY_TRACE_HEADER = 'sentry-trace';
2223
const SENTRY_BAGGAGE_HEADER = 'baggage';
@@ -238,7 +239,10 @@ export class SentryNodeFetchInstrumentation extends InstrumentationBase<SentryNo
238239
* Check if the given outgoing request should be ignored.
239240
*/
240241
private _shouldIgnoreOutgoingRequest(request: UndiciRequest): boolean {
241-
if (isTracingSuppressed(context.active())) {
242+
// Never instrument outgoing requests in Edge Runtime
243+
// This can be a problem when running in Next.js Edge Runtime in dev,
244+
// as there edge is simulated but still uses Node under the hood, leaving to problems
245+
if (isTracingSuppressed(context.active()) || isNextEdgeRuntime()) {
242246
return true;
243247
}
244248

‎packages/node/src/integrations/node-fetch/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { generateInstrumentOnce } from '../../otel/instrument';
66
import type { NodeClient } from '../../sdk/client';
77
import type { NodeClientOptions } from '../../types';
88
import { SentryNodeFetchInstrumentation } from './SentryNodeFetchInstrumentation';
9+
import { isNextEdgeRuntime } from '../../utils/isNextEdgeRuntime';
910

1011
const INTEGRATION_NAME = 'NodeFetch';
1112

@@ -96,6 +97,13 @@ function getConfigWithDefaults(options: Partial<NodeFetchOptions> = {}): UndiciI
9697
const instrumentationConfig = {
9798
requireParentforSpans: false,
9899
ignoreRequestHook: request => {
100+
// Never instrument outgoing requests in Edge Runtime
101+
// This can be a problem when running in Next.js Edge Runtime in dev,
102+
// as there edge is simulated but still uses Node under the hood, leaving to problems
103+
if (isNextEdgeRuntime()) {
104+
return true;
105+
}
106+
99107
const url = getAbsoluteUrl(request.origin, request.path);
100108
const _ignoreOutgoingRequests = options.ignoreOutgoingRequests;
101109
const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url);

‎packages/node/src/utils/isNextEdgeRuntime.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/**
2+
* Returns true if the current runtime is Next.js Edge Runtime.
3+
*
4+
* @returns `true` if the current runtime is Next.js Edge Runtime, `false` otherwise.
5+
*/
6+
export function isNextEdgeRuntime(): boolean {
7+
return process.env.NEXT_RUNTIME === 'edge';
8+
}

‎yarn.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29253,7 +29253,7 @@ vite@^5.0.0, vite@^5.4.11, vite@^5.4.5:
2925329253
optionalDependencies:
2925429254
fsevents "~2.3.3"
2925529255

29256-
vitefu@^0.2.2, vitefu@^0.2.4, vitefu@^0.2.5:
29256+
vitefu@^0.2.2, vitefu@^0.2.4:
2925729257
version "0.2.5"
2925829258
resolved "https://registry.yarnpkg.com/vitefu/-/vitefu-0.2.5.tgz#c1b93c377fbdd3e5ddd69840ea3aa70b40d90969"
2925929259
integrity sha512-SgHtMLoqaeeGnd2evZ849ZbACbnwQCIwRH57t18FxcXoZop0uQu0uzlIhJBlF/eWVzuce0sHeqPcDo+evVcg8Q==

0 commit comments

Comments
 (0)