Skip to content

Commit 466f076

Browse files
mydeamydea
committed
feat(node): Drop http.server spans with 404 status by default
This can be configured via `dropSpansForIncomingRequestStatusCodes` option in `httpIntegration()`.
1 parent e0c0d9d commit 466f076

File tree

7 files changed

+217
-37
lines changed

7 files changed

+217
-37
lines changed

‎dev-packages/node-integration-tests/suites/express-v5/tracing/instrument-filterStatusCode.mjs‎

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, /3\d{2}/],
12+
}),
13+
],
14+
});

‎dev-packages/node-integration-tests/suites/express-v5/tracing/scenario-filterStatusCode.mjs‎

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
});

‎dev-packages/node-integration-tests/suites/express/tracing/instrument-filterStatusCode.mjs‎

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, /3\d{2}/],
12+
}),
13+
],
14+
});

‎dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs‎

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
});

‎packages/node/src/integrations/http/index.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ interface HttpOptions {
7373
*/
7474
ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean;
7575

76+
/**
77+
* Do not capture spans for incoming HTTP requests with the given status codes.
78+
* By default, spans with 404 status code are ignored.
79+
*
80+
* @default `[404]`
81+
*/
82+
dropSpansForIncomingRequestStatusCodes?: (number | RegExp)[];
83+
7684
/**
7785
* Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`.
7886
* This can be useful for long running requests where the body is not needed and we want to avoid capturing it.
@@ -148,6 +156,8 @@ export function _shouldInstrumentSpans(options: HttpOptions, clientOptions: Part
148156
* It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span.
149157
*/
150158
export const httpIntegration = defineIntegration((options: HttpOptions = {}) => {
159+
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [404];
160+
151161
return {
152162
name: INTEGRATION_NAME,
153163
setupOnce() {
@@ -180,6 +190,22 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
180190
instrumentOtelHttp(instrumentationConfig);
181191
}
182192
},
193+
processEvent(event) {
194+
// Drop transaction if it has a status code that should be ignored
195+
if (event.type === 'transaction') {
196+
const statusCode = event.contexts?.trace?.data?.['http.response.status_code'];
197+
if (
198+
typeof statusCode === 'number' &&
199+
dropSpansForIncomingRequestStatusCodes.some(code =>
200+
typeof code === 'number' ? code === statusCode : code.test(statusCode.toString()),
201+
)
202+
) {
203+
return null;
204+
}
205+
}
206+
207+
return event;
208+
},
183209
};
184210
});
185211

0 commit comments

Comments
 (0)