Skip to content

Commit 3a81041

Browse files
authored
feat(node): Drop http.server spans with 404 status by default (#16205)
This can be configured like this: ```js httpIntegration({ dropSpansForIncomingRequestStatusCodes: [404, [300,399]] }) ``` It defaults to `[404]`. Closes #16193
1 parent e0c0d9d commit 3a81041

File tree

14 files changed

+232
-63
lines changed

14 files changed

+232
-63
lines changed

dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -125,20 +125,3 @@ test('Should send a transaction event with correct status for a generateMetadata
125125

126126
expect((await transactionPromise).contexts?.trace?.status).toBe('ok');
127127
});
128-
129-
test('Should send a transaction event with correct status for a generateMetadata() function invocation with notfound()', async ({
130-
page,
131-
}) => {
132-
const testTitle = 'notfound-foobar';
133-
134-
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
135-
return (
136-
transactionEvent.contexts?.trace?.data?.['http.target'] ===
137-
`/generation-functions/with-notfound?metadataTitle=${testTitle}`
138-
);
139-
});
140-
141-
await page.goto(`/generation-functions/with-notfound?metadataTitle=${testTitle}`);
142-
143-
expect((await transactionPromise).contexts?.trace?.status).toBe('not_found');
144-
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ export async function GET() {
55
}
66

77
export async function POST() {
8-
return NextResponse.json({ name: 'John Doe' }, { status: 404 });
8+
return NextResponse.json({ name: 'John Doe' }, { status: 403 });
99
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ test('Should create a transaction for route handlers and correctly set span stat
2828

2929
const routehandlerTransaction = await routehandlerTransactionPromise;
3030

31-
expect(routehandlerTransaction.contexts?.trace?.status).toBe('not_found');
31+
expect(routehandlerTransaction.contexts?.trace?.status).toBe('permission_denied');
3232
expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server');
3333
});
3434

dev-packages/e2e-tests/test-applications/node-express/src/app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ export const appRouter = t.router({
123123
.mutation(() => {
124124
throw new Error('I crashed in a trpc handler');
125125
}),
126-
dontFindSomething: procedure.mutation(() => {
127-
throw new TRPCError({ code: 'NOT_FOUND', cause: new Error('Page not found') });
126+
unauthorized: procedure.mutation(() => {
127+
throw new TRPCError({ code: 'UNAUTHORIZED', cause: new Error('Unauthorized') });
128128
}),
129129
});
130130

dev-packages/e2e-tests/test-applications/node-express/tests/trpc.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,12 @@ test('Should record transaction and error for a trpc handler that returns a stat
109109
const transactionEventPromise = waitForTransaction('node-express', transactionEvent => {
110110
return (
111111
transactionEvent.transaction === 'POST /trpc' &&
112-
!!transactionEvent.spans?.find(span => span.description === 'trpc/dontFindSomething')
112+
!!transactionEvent.spans?.find(span => span.description === 'trpc/unauthorized')
113113
);
114114
});
115115

116116
const errorEventPromise = waitForError('node-express', errorEvent => {
117-
return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('Page not found'));
117+
return !!errorEvent?.exception?.values?.some(exception => exception.value?.includes('Unauthorized'));
118118
});
119119

120120
const trpcClient = createTRPCProxyClient<AppRouter>({
@@ -125,7 +125,7 @@ test('Should record transaction and error for a trpc handler that returns a stat
125125
],
126126
});
127127

128-
await expect(trpcClient.dontFindSomething.mutate()).rejects.toBeDefined();
128+
await expect(trpcClient.unauthorized.mutate()).rejects.toBeDefined();
129129

130130
await expect(transactionEventPromise).resolves.toBeDefined();
131131
await expect(errorEventPromise).resolves.toBeDefined();

dev-packages/e2e-tests/test-applications/node-hapi/src/app.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ const init = async () => {
102102
const path = request.route.path;
103103

104104
if (path.includes('boom-4xx')) {
105-
throw Boom.notFound('4xx not found (onPreResponse)');
105+
throw Boom.badRequest('4xx bad request (onPreResponse)');
106106
} else if (path.includes('boom-5xx')) {
107107
throw Boom.gatewayTimeout('5xx not implemented (onPreResponse)');
108108
} else if (path.includes('JS-error-onPreResponse')) {

dev-packages/e2e-tests/test-applications/node-hapi/tests/errors.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ test('Does not send errors to Sentry if boom throws in "onPreResponse" after JS
7777
const response4xx = await fetch(`${baseURL}/test-failure-boom-4xx`);
7878
const response5xx = await fetch(`${baseURL}/test-failure-boom-5xx`);
7979

80-
expect(response4xx.status).toBe(404);
80+
expect(response4xx.status).toBe(400);
8181
expect(response5xx.status).toBe(504);
8282

8383
const transactionEvent4xx = await transactionEventPromise4xx;
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
integrations: [
10+
Sentry.httpIntegration({
11+
dropSpansForIncomingRequestStatusCodes: [499, [300, 399]],
12+
}),
13+
],
14+
});
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import express from 'express';
4+
5+
const app = express();
6+
7+
app.get('/', (_req, res) => {
8+
res.send({ response: 'response 0' });
9+
});
10+
11+
app.get('/499', (_req, res) => {
12+
res.status(499).send({ response: 'response 499' });
13+
});
14+
15+
app.get('/300', (_req, res) => {
16+
res.status(300).send({ response: 'response 300' });
17+
});
18+
19+
app.get('/399', (_req, res) => {
20+
res.status(399).send({ response: 'response 399' });
21+
});
22+
23+
Sentry.setupExpressErrorHandler(app);
24+
25+
startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('express v5 tracing', () => {
7777
await runner.completed();
7878
});
7979

80-
test('handles root page correctly', async () => {
80+
test('handles root route correctly', async () => {
8181
const runner = createRunner()
8282
.expect({
8383
transaction: {
@@ -89,30 +89,17 @@ describe('express v5 tracing', () => {
8989
await runner.completed();
9090
});
9191

92-
test('handles 404 page correctly', async () => {
92+
test('ignores 404 routes by default', async () => {
9393
const runner = createRunner()
9494
.expect({
95+
// No transaction is sent for the 404 route
9596
transaction: {
96-
transaction: 'GET /does-not-exist',
97-
contexts: {
98-
trace: {
99-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
100-
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
101-
data: {
102-
'http.response.status_code': 404,
103-
url: expect.stringMatching(/\/does-not-exist$/),
104-
'http.method': 'GET',
105-
'http.url': expect.stringMatching(/\/does-not-exist$/),
106-
'http.target': '/does-not-exist',
107-
},
108-
op: 'http.server',
109-
status: 'not_found',
110-
},
111-
},
97+
transaction: 'GET /',
11298
},
11399
})
114100
.start();
115101
runner.makeRequest('get', '/does-not-exist', { expectError: true });
102+
runner.makeRequest('get', '/');
116103
await runner.completed();
117104
});
118105

@@ -290,4 +277,56 @@ describe('express v5 tracing', () => {
290277
});
291278
});
292279
});
280+
281+
describe('filter status codes', () => {
282+
createEsmAndCjsTests(
283+
__dirname,
284+
'scenario-filterStatusCode.mjs',
285+
'instrument-filterStatusCode.mjs',
286+
(createRunner, test) => {
287+
// We opt-out of the default 404 filtering in order to test how 404 spans are handled
288+
test('handles 404 route correctly', async () => {
289+
const runner = createRunner()
290+
.expect({
291+
transaction: {
292+
transaction: 'GET /does-not-exist',
293+
contexts: {
294+
trace: {
295+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
296+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
297+
data: {
298+
'http.response.status_code': 404,
299+
url: expect.stringMatching(/\/does-not-exist$/),
300+
'http.method': 'GET',
301+
'http.url': expect.stringMatching(/\/does-not-exist$/),
302+
'http.target': '/does-not-exist',
303+
},
304+
op: 'http.server',
305+
status: 'not_found',
306+
},
307+
},
308+
},
309+
})
310+
.start();
311+
runner.makeRequest('get', '/does-not-exist', { expectError: true });
312+
await runner.completed();
313+
});
314+
315+
test('filters defined status codes', async () => {
316+
const runner = createRunner()
317+
.expect({
318+
transaction: {
319+
transaction: 'GET /',
320+
},
321+
})
322+
.start();
323+
await runner.makeRequest('get', '/499', { expectError: true });
324+
await runner.makeRequest('get', '/300', { expectError: true });
325+
await runner.makeRequest('get', '/399', { expectError: true });
326+
await runner.makeRequest('get', '/');
327+
await runner.completed();
328+
});
329+
},
330+
);
331+
});
293332
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
integrations: [
10+
Sentry.httpIntegration({
11+
dropSpansForIncomingRequestStatusCodes: [499, [300, 399]],
12+
}),
13+
],
14+
});
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import express from 'express';
4+
5+
const app = express();
6+
7+
app.get('/', (_req, res) => {
8+
res.send({ response: 'response 0' });
9+
});
10+
11+
app.get('/499', (_req, res) => {
12+
res.status(499).send({ response: 'response 499' });
13+
});
14+
15+
app.get('/300', (_req, res) => {
16+
res.status(300).send({ response: 'response 300' });
17+
});
18+
19+
app.get('/399', (_req, res) => {
20+
res.status(399).send({ response: 'response 399' });
21+
});
22+
23+
Sentry.setupExpressErrorHandler(app);
24+
25+
startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/tracing/test.ts

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -90,33 +90,17 @@ describe('express tracing', () => {
9090
await runner.completed();
9191
});
9292

93-
test('handles 404 page correctly', async () => {
93+
test('ignores 404 routes by default', async () => {
9494
const runner = createRunner()
9595
.expect({
96+
// No transaction is sent for the 404 route
9697
transaction: {
97-
// FIXME: This is wrong :(
9898
transaction: 'GET /',
99-
contexts: {
100-
trace: {
101-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
102-
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
103-
data: {
104-
'http.response.status_code': 404,
105-
url: expect.stringMatching(/\/does-not-exist$/),
106-
'http.method': 'GET',
107-
// FIXME: This is wrong :(
108-
'http.route': '/',
109-
'http.url': expect.stringMatching(/\/does-not-exist$/),
110-
'http.target': '/does-not-exist',
111-
},
112-
op: 'http.server',
113-
status: 'not_found',
114-
},
115-
},
11699
},
117100
})
118101
.start();
119102
runner.makeRequest('get', '/does-not-exist', { expectError: true });
103+
runner.makeRequest('get', '/');
120104
await runner.completed();
121105
});
122106

@@ -324,4 +308,57 @@ describe('express tracing', () => {
324308
});
325309
});
326310
});
311+
312+
describe('filter status codes', () => {
313+
createEsmAndCjsTests(
314+
__dirname,
315+
'scenario-filterStatusCode.mjs',
316+
'instrument-filterStatusCode.mjs',
317+
(createRunner, test) => {
318+
// We opt-out of the default 404 filtering in order to test how 404 spans are handled
319+
test('handles 404 route correctly', async () => {
320+
const runner = createRunner()
321+
.expect({
322+
transaction: {
323+
// FIXME: This is incorrect, sadly :(
324+
transaction: 'GET /',
325+
contexts: {
326+
trace: {
327+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
328+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
329+
data: {
330+
'http.response.status_code': 404,
331+
url: expect.stringMatching(/\/does-not-exist$/),
332+
'http.method': 'GET',
333+
'http.url': expect.stringMatching(/\/does-not-exist$/),
334+
'http.target': '/does-not-exist',
335+
},
336+
op: 'http.server',
337+
status: 'not_found',
338+
},
339+
},
340+
},
341+
})
342+
.start();
343+
runner.makeRequest('get', '/does-not-exist', { expectError: true });
344+
await runner.completed();
345+
});
346+
347+
test('filters defined status codes', async () => {
348+
const runner = createRunner()
349+
.expect({
350+
transaction: {
351+
transaction: 'GET /',
352+
},
353+
})
354+
.start();
355+
await runner.makeRequest('get', '/499', { expectError: true });
356+
await runner.makeRequest('get', '/300', { expectError: true });
357+
await runner.makeRequest('get', '/399', { expectError: true });
358+
await runner.makeRequest('get', '/');
359+
await runner.completed();
360+
});
361+
},
362+
);
363+
});
327364
});

0 commit comments

Comments
 (0)