Skip to content

Commit f997961

Browse files
committed
feat(nextjs): remove tracing from pages router API routes
1 parent 20c7c8f commit f997961

File tree

6 files changed

+52
-155
lines changed

6 files changed

+52
-155
lines changed

dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/sessions.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ test('should report healthy sessions', async ({ page }) => {
55
test.skip(process.env.TEST_ENV === 'development', 'test is flakey in dev mode');
66

77
const sessionPromise = waitForSession('nextjs-13', session => {
8+
console.log('session', session);
89
return session.init === true && session.status === 'ok' && session.errors === 0;
910
});
1011

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/cjs-api-endpoints.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ test('should create a transaction for a CJS pages router API endpoint', async ({
3939
data: {
4040
'http.response.status_code': 200,
4141
'sentry.op': 'http.server',
42-
'sentry.origin': 'auto.http.nextjs',
42+
'sentry.origin': 'auto',
4343
'sentry.sample_rate': 1,
4444
'sentry.source': 'route',
4545
},
4646
op: 'http.server',
47-
origin: 'auto.http.nextjs',
47+
origin: 'auto',
4848
span_id: expect.stringMatching(/[a-f0-9]{16}/),
4949
status: 'ok',
5050
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
@@ -57,7 +57,7 @@ test('should create a transaction for a CJS pages router API endpoint', async ({
5757
cookies: expect.any(Object),
5858
headers: expect.any(Object),
5959
method: 'GET',
60-
url: expect.stringMatching(/^http.*\/api\/cjs-api-endpoint$/),
60+
url: expect.stringMatching(/\/api\/cjs-api-endpoint$/),
6161
},
6262
spans: expect.arrayContaining([]),
6363
start_timestamp: expect.any(Number),
@@ -102,12 +102,12 @@ test('should not mess up require statements in CJS API endpoints', async ({ requ
102102
data: {
103103
'http.response.status_code': 200,
104104
'sentry.op': 'http.server',
105-
'sentry.origin': 'auto.http.nextjs',
105+
'sentry.origin': 'auto',
106106
'sentry.sample_rate': 1,
107107
'sentry.source': 'route',
108108
},
109109
op: 'http.server',
110-
origin: 'auto.http.nextjs',
110+
origin: 'auto',
111111
span_id: expect.stringMatching(/[a-f0-9]{16}/),
112112
status: 'ok',
113113
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
@@ -120,7 +120,7 @@ test('should not mess up require statements in CJS API endpoints', async ({ requ
120120
cookies: expect.any(Object),
121121
headers: expect.any(Object),
122122
method: 'GET',
123-
url: expect.stringMatching(/^http.*\/api\/cjs-api-endpoint-with-require$/),
123+
url: expect.stringMatching(/\/api\/cjs-api-endpoint-with-require$/),
124124
},
125125
spans: expect.arrayContaining([]),
126126
start_timestamp: expect.any(Number),

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/pages-router-api-endpoints.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ test('Should report an error event for errors thrown in pages router api routes'
5555
data: {
5656
'http.response.status_code': 500,
5757
'sentry.op': 'http.server',
58-
'sentry.origin': 'auto.http.nextjs',
58+
'sentry.origin': 'auto',
5959
'sentry.source': 'route',
6060
},
6161
op: 'http.server',
62-
origin: 'auto.http.nextjs',
62+
origin: 'auto',
6363
span_id: expect.stringMatching(/[a-f0-9]{16}/),
6464
status: 'internal_error',
6565
trace_id: (await errorEventPromise).contexts?.trace?.trace_id,
@@ -69,7 +69,7 @@ test('Should report an error event for errors thrown in pages router api routes'
6969
request: {
7070
headers: expect.any(Object),
7171
method: 'GET',
72-
url: expect.stringMatching(/^http.*\/api\/foo\/failure-api-route$/),
72+
url: expect.stringMatching(/^\/api\/foo\/failure-api-route$/),
7373
},
7474
start_timestamp: expect.any(Number),
7575
timestamp: expect.any(Number),
@@ -98,11 +98,11 @@ test('Should report a transaction event for a successful pages router api route'
9898
data: {
9999
'http.response.status_code': 200,
100100
'sentry.op': 'http.server',
101-
'sentry.origin': 'auto.http.nextjs',
101+
'sentry.origin': 'auto',
102102
'sentry.source': 'route',
103103
},
104104
op: 'http.server',
105-
origin: 'auto.http.nextjs',
105+
origin: 'auto',
106106
span_id: expect.stringMatching(/[a-f0-9]{16}/),
107107
status: 'ok',
108108
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
@@ -112,7 +112,7 @@ test('Should report a transaction event for a successful pages router api route'
112112
request: {
113113
headers: expect.any(Object),
114114
method: 'GET',
115-
url: expect.stringMatching(/^http.*\/api\/foo\/success-api-route$/),
115+
url: expect.stringMatching(/^\/api\/foo\/success-api-route$/),
116116
},
117117
start_timestamp: expect.any(Number),
118118
timestamp: expect.any(Number),

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/wrapApiHandlerWithSentry.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ cases.forEach(({ name, url, transactionName }) => {
3939
data: {
4040
'http.response.status_code': 200,
4141
'sentry.op': 'http.server',
42-
'sentry.origin': 'auto.http.nextjs',
42+
'sentry.origin': 'auto',
4343
'sentry.source': 'route',
4444
},
4545
op: 'http.server',
46-
origin: 'auto.http.nextjs',
46+
origin: 'auto',
4747
span_id: expect.stringMatching(/[a-f0-9]{16}/),
4848
status: 'ok',
4949
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
Lines changed: 37 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,14 @@
11
import {
22
captureException,
3-
continueTrace,
43
debug,
5-
getActiveSpan,
4+
getCurrentScope,
5+
getIsolationScope,
66
httpRequestToRequestData,
7-
isString,
87
objectify,
9-
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
10-
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
11-
setHttpStatus,
12-
startSpanManual,
13-
withIsolationScope,
148
} from '@sentry/core';
159
import type { NextApiRequest } from 'next';
1610
import type { AugmentedNextApiResponse, NextApiHandler } from '../types';
17-
import { flushSafelyWithTimeout, waitUntil } from '../utils/responseEnd';
18-
import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils';
11+
import { flushSafelyWithTimeout } from '../utils/responseEnd';
1912

2013
export type AugmentedNextApiRequest = NextApiRequest & {
2114
__withSentry_applied__?: boolean;
@@ -31,15 +24,13 @@ export type AugmentedNextApiRequest = NextApiRequest & {
3124
*/
3225
export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameterizedRoute: string): NextApiHandler {
3326
return new Proxy(apiHandler, {
34-
apply: (
27+
apply: async (
3528
wrappingTarget,
3629
thisArg,
3730
args: [AugmentedNextApiRequest | undefined, AugmentedNextApiResponse | undefined],
3831
) => {
39-
dropNextjsRootContext();
40-
return escapeNextjsTracing(() => {
32+
try {
4133
const [req, res] = args;
42-
4334
if (!req) {
4435
debug.log(
4536
`Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`,
@@ -56,86 +47,45 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz
5647
if (req.__withSentry_applied__) {
5748
return wrappingTarget.apply(thisArg, args);
5849
}
59-
req.__withSentry_applied__ = true;
60-
61-
return withIsolationScope(isolationScope => {
62-
// Normally, there is an active span here (from Next.js OTEL) and we just use that as parent
63-
// Else, we manually continueTrace from the incoming headers
64-
const continueTraceIfNoActiveSpan = getActiveSpan()
65-
? <T>(_opts: unknown, callback: () => T) => callback()
66-
: continueTrace;
67-
68-
return continueTraceIfNoActiveSpan(
69-
{
70-
sentryTrace:
71-
req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined,
72-
baggage: req.headers?.baggage,
73-
},
74-
() => {
75-
const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
76-
const normalizedRequest = httpRequestToRequestData(req);
7750

78-
isolationScope.setSDKProcessingMetadata({ normalizedRequest });
79-
isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`);
80-
81-
return startSpanManual(
82-
{
83-
name: `${reqMethod}${parameterizedRoute}`,
84-
op: 'http.server',
85-
forceTransaction: true,
86-
attributes: {
87-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
88-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs',
89-
},
90-
},
91-
async span => {
92-
// eslint-disable-next-line @typescript-eslint/unbound-method
93-
res.end = new Proxy(res.end, {
94-
apply(target, thisArg, argArray) {
95-
setHttpStatus(span, res.statusCode);
96-
span.end();
97-
waitUntil(flushSafelyWithTimeout());
98-
return target.apply(thisArg, argArray);
99-
},
100-
});
101-
try {
102-
return await wrappingTarget.apply(thisArg, args);
103-
} catch (e) {
104-
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
105-
// store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced
106-
// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
107-
// way to prevent it from actually being reported twice.)
108-
const objectifiedErr = objectify(e);
51+
req.__withSentry_applied__ = true;
10952

110-
captureException(objectifiedErr, {
111-
mechanism: {
112-
type: 'auto.http.nextjs.api_handler',
113-
handled: false,
114-
data: {
115-
wrapped_handler: wrappingTarget.name,
116-
function: 'withSentry',
117-
},
118-
},
119-
});
53+
// Set transaction name even without tracing to ensure parameterized routes are used
54+
const method = req.method || 'GET';
55+
getCurrentScope().setTransactionName(`${method} ${parameterizedRoute}`);
12056

121-
setHttpStatus(span, 500);
122-
span.end();
57+
// Set SDK processing metadata for session tracking (needed even without tracing)
58+
const normalizedRequest = httpRequestToRequestData(req);
59+
getIsolationScope().setSDKProcessingMetadata({ normalizedRequest });
12360

124-
// we need to await the flush here to ensure that the error is captured
125-
// as the runtime freezes as soon as the error is thrown below
126-
await flushSafelyWithTimeout();
61+
return await wrappingTarget.apply(thisArg, args);
62+
} catch (e) {
63+
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
64+
// store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced
65+
// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
66+
// way to prevent it from actually being reported twice.)
67+
const objectifiedErr = objectify(e);
12768

128-
// We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
129-
// would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
130-
// the error as already having been captured.)
131-
throw objectifiedErr;
132-
}
133-
},
134-
);
69+
captureException(objectifiedErr, {
70+
mechanism: {
71+
type: 'auto.http.nextjs.api_handler',
72+
handled: false,
73+
data: {
74+
wrapped_handler: wrappingTarget.name,
75+
function: 'withSentry',
13576
},
136-
);
77+
},
13778
});
138-
});
79+
80+
// we need to await the flush here to ensure that the error is captured
81+
// as the runtime freezes as soon as the error is thrown below
82+
await flushSafelyWithTimeout();
83+
84+
// We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
85+
// would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
86+
// the error as already having been captured.)
87+
throw objectifiedErr;
88+
}
13989
},
14090
});
14191
}

packages/nextjs/test/config/withSentry.test.ts

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

0 commit comments

Comments
 (0)