Skip to content

feat(node): Drop http.server spans with 404 status by default #16205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 7, 2025
Merged

feat(node): Drop http.server spans with 404 status by default #16205

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
466f076
feat(node): Drop `http.server` spans with 404 status by default
mydea May 6, 2025
5ae3c07
use ranges instead of regex
mydea May 7, 2025
94ec0f2
fix tests
mydea May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions ...packages/node-integration-tests/suites/express-v5/tracing/instrument-filterStatusCode.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://[email protected]/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
integrations: [
Sentry.httpIntegration({
dropSpansForIncomingRequestStatusCodes: [499, /3\d{2}/],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex is an interesting API choice. I can kindof see the reason, just wanna suggest other options that might be a bit more intuitive to write and don't require googling what the regex for numbers is, like:

  • Providing intervals to drop for ranges like [from, to] e.g. [400, 499].
  • Providing strings with x as a wildcard e.g. "4xx"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I know what you mean. The drop ranges seem fine to me too, I'll adjust it to that instead!

}),
],
});
25 changes: 25 additions & 0 deletions dev-packages/node-integration-tests/suites/express-v5/tracing/scenario-filterStatusCode.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as Sentry from '@sentry/node';
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
import express from 'express';

const app = express();

app.get('/', (_req, res) => {
res.send({ response: 'response 0' });
});

app.get('/499', (_req, res) => {
res.status(499).send({ response: 'response 499' });
});

app.get('/300', (_req, res) => {
res.status(300).send({ response: 'response 300' });
});

app.get('/399', (_req, res) => {
res.status(399).send({ response: 'response 399' });
});

Sentry.setupExpressErrorHandler(app);

startExpressServerAndSendPortToRunner(app);
75 changes: 57 additions & 18 deletions dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('express v5 tracing', () => {
await runner.completed();
});

test('handles root page correctly', async () => {
test('handles root route correctly', async () => {
const runner = createRunner()
.expect({
transaction: {
Expand All @@ -89,30 +89,17 @@ describe('express v5 tracing', () => {
await runner.completed();
});

test('handles 404 page correctly', async () => {
test('ignores 404 routes by default', async () => {
const runner = createRunner()
.expect({
// No transaction is sent for the 404 route
transaction: {
transaction: 'GET /does-not-exist',
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'http.response.status_code': 404,
url: expect.stringMatching(/\/does-not-exist$/),
'http.method': 'GET',
'http.url': expect.stringMatching(/\/does-not-exist$/),
'http.target': '/does-not-exist',
},
op: 'http.server',
status: 'not_found',
},
},
transaction: 'GET /',
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
runner.makeRequest('get', '/');
await runner.completed();
});

Expand Down Expand Up @@ -290,4 +277,56 @@ describe('express v5 tracing', () => {
});
});
});

describe('filter status codes', () => {
createEsmAndCjsTests(
__dirname,
'scenario-filterStatusCode.mjs',
'instrument-filterStatusCode.mjs',
(createRunner, test) => {
// We opt-out of the default 404 filtering in order to test how 404 spans are handled
test('handles 404 route correctly', async () => {
const runner = createRunner()
.expect({
transaction: {
transaction: 'GET /does-not-exist',
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'http.response.status_code': 404,
url: expect.stringMatching(/\/does-not-exist$/),
'http.method': 'GET',
'http.url': expect.stringMatching(/\/does-not-exist$/),
'http.target': '/does-not-exist',
},
op: 'http.server',
status: 'not_found',
},
},
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
await runner.completed();
});

test('filters defined status codes', async () => {
const runner = createRunner()
.expect({
transaction: {
transaction: 'GET /',
},
})
.start();
await runner.makeRequest('get', '/499', { expectError: true });
await runner.makeRequest('get', '/300', { expectError: true });
await runner.makeRequest('get', '/399', { expectError: true });
await runner.makeRequest('get', '/');
await runner.completed();
});
},
);
});
});
14 changes: 14 additions & 0 deletions dev-packages/node-integration-tests/suites/express/tracing/instrument-filterStatusCode.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://[email protected]/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
integrations: [
Sentry.httpIntegration({
dropSpansForIncomingRequestStatusCodes: [499, /3\d{2}/],
}),
],
});
25 changes: 25 additions & 0 deletions dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as Sentry from '@sentry/node';
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
import express from 'express';

const app = express();

app.get('/', (_req, res) => {
res.send({ response: 'response 0' });
});

app.get('/499', (_req, res) => {
res.status(499).send({ response: 'response 499' });
});

app.get('/300', (_req, res) => {
res.status(300).send({ response: 'response 300' });
});

app.get('/399', (_req, res) => {
res.status(399).send({ response: 'response 399' });
});

Sentry.setupExpressErrorHandler(app);

startExpressServerAndSendPortToRunner(app);
75 changes: 56 additions & 19 deletions dev-packages/node-integration-tests/suites/express/tracing/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,33 +90,17 @@ describe('express tracing', () => {
await runner.completed();
});

test('handles 404 page correctly', async () => {
test('ignores 404 routes by default', async () => {
const runner = createRunner()
.expect({
// No transaction is sent for the 404 route
transaction: {
// FIXME: This is wrong :(
transaction: 'GET /',
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'http.response.status_code': 404,
url: expect.stringMatching(/\/does-not-exist$/),
'http.method': 'GET',
// FIXME: This is wrong :(
'http.route': '/',
'http.url': expect.stringMatching(/\/does-not-exist$/),
'http.target': '/does-not-exist',
},
op: 'http.server',
status: 'not_found',
},
},
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
runner.makeRequest('get', '/');
await runner.completed();
});

Expand Down Expand Up @@ -324,4 +308,57 @@ describe('express tracing', () => {
});
});
});

describe('filter status codes', () => {
createEsmAndCjsTests(
__dirname,
'scenario-filterStatusCode.mjs',
'instrument-filterStatusCode.mjs',
(createRunner, test) => {
// We opt-out of the default 404 filtering in order to test how 404 spans are handled
test('handles 404 route correctly', async () => {
const runner = createRunner()
.expect({
transaction: {
// FIXME: This is incorrect, sadly :(
transaction: 'GET /',
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'http.response.status_code': 404,
url: expect.stringMatching(/\/does-not-exist$/),
'http.method': 'GET',
'http.url': expect.stringMatching(/\/does-not-exist$/),
'http.target': '/does-not-exist',
},
op: 'http.server',
status: 'not_found',
},
},
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
await runner.completed();
});

test('filters defined status codes', async () => {
const runner = createRunner()
.expect({
transaction: {
transaction: 'GET /',
},
})
.start();
await runner.makeRequest('get', '/499', { expectError: true });
await runner.makeRequest('get', '/300', { expectError: true });
await runner.makeRequest('get', '/399', { expectError: true });
await runner.makeRequest('get', '/');
await runner.completed();
});
},
);
});
});
26 changes: 26 additions & 0 deletions packages/node/src/integrations/http/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ interface HttpOptions {
*/
ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean;

/**
* Do not capture spans for incoming HTTP requests with the given status codes.
* By default, spans with 404 status code are ignored.
*
* @default `[404]`
*/
dropSpansForIncomingRequestStatusCodes?: (number | RegExp)[];

/**
* Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`.
* This can be useful for long running requests where the body is not needed and we want to avoid capturing it.
Expand Down Expand Up @@ -148,6 +156,8 @@ export function _shouldInstrumentSpans(options: HttpOptions, clientOptions: Part
* It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span.
*/
export const httpIntegration = defineIntegration((options: HttpOptions = {}) => {
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [404];

return {
name: INTEGRATION_NAME,
setupOnce() {
Expand Down Expand Up @@ -180,6 +190,22 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
instrumentOtelHttp(instrumentationConfig);
}
},
processEvent(event) {
// Drop transaction if it has a status code that should be ignored
if (event.type === 'transaction') {
const statusCode = event.contexts?.trace?.data?.['http.response.status_code'];
if (
typeof statusCode === 'number' &&
dropSpansForIncomingRequestStatusCodes.some(code =>
typeof code === 'number' ? code === statusCode : code.test(statusCode.toString()),
)
) {
return null;
}
}

return event;
},
};
});

Expand Down
Loading