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 all commits
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
17 changes: 0 additions & 17 deletions dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,3 @@ test('Should send a transaction event with correct status for a generateMetadata

expect((await transactionPromise).contexts?.trace?.status).toBe('ok');
});

Copy link
Member Author

Choose a reason for hiding this comment

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

@lforst removed this test as this is no longer sent

test('Should send a transaction event with correct status for a generateMetadata() function invocation with notfound()', async ({
page,
}) => {
const testTitle = 'notfound-foobar';

const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
return (
transactionEvent.contexts?.trace?.data?.['http.target'] ===
`/generation-functions/with-notfound?metadataTitle=${testTitle}`
);
});

await page.goto(`/generation-functions/with-notfound?metadataTitle=${testTitle}`);

expect((await transactionPromise).contexts?.trace?.status).toBe('not_found');
});
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/route-handlers/[param]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ export async function GET() {
}

export async function POST() {
return NextResponse.json({ name: 'John Doe' }, { status: 404 });
return NextResponse.json({ name: 'John Doe' }, { status: 403 });
}
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test('Should create a transaction for route handlers and correctly set span stat

const routehandlerTransaction = await routehandlerTransactionPromise;

expect(routehandlerTransaction.contexts?.trace?.status).toBe('not_found');
expect(routehandlerTransaction.contexts?.trace?.status).toBe('permission_denied');
expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server');
});

Expand Down
4 changes: 2 additions & 2 deletions dev-packages/e2e-tests/test-applications/node-express/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ export const appRouter = t.router({
.mutation(() => {
throw new Error('I crashed in a trpc handler');
}),
dontFindSomething: procedure.mutation(() => {
throw new TRPCError({ code: 'NOT_FOUND', cause: new Error('Page not found') });
unauthorized: procedure.mutation(() => {
throw new TRPCError({ code: 'UNAUTHORIZED', cause: new Error('Unauthorized') });
}),
});

Expand Down
6 changes: 3 additions & 3 deletions dev-packages/e2e-tests/test-applications/node-express/tests/trpc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ test('Should record transaction and error for a trpc handler that returns a stat
const transactionEventPromise = waitForTransaction('node-express', transactionEvent => {
return (
transactionEvent.transaction === 'POST /trpc' &&
!!transactionEvent.spans?.find(span => span.description === 'trpc/dontFindSomething')
!!transactionEvent.spans?.find(span => span.description === 'trpc/unauthorized')
);
});

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

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

await expect(trpcClient.dontFindSomething.mutate()).rejects.toBeDefined();
await expect(trpcClient.unauthorized.mutate()).rejects.toBeDefined();

await expect(transactionEventPromise).resolves.toBeDefined();
await expect(errorEventPromise).resolves.toBeDefined();
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/node-hapi/src/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ const init = async () => {
const path = request.route.path;

if (path.includes('boom-4xx')) {
throw Boom.notFound('4xx not found (onPreResponse)');
throw Boom.badRequest('4xx bad request (onPreResponse)');
} else if (path.includes('boom-5xx')) {
throw Boom.gatewayTimeout('5xx not implemented (onPreResponse)');
} else if (path.includes('JS-error-onPreResponse')) {
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/node-hapi/tests/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ test('Does not send errors to Sentry if boom throws in "onPreResponse" after JS
const response4xx = await fetch(`${baseURL}/test-failure-boom-4xx`);
const response5xx = await fetch(`${baseURL}/test-failure-boom-5xx`);

expect(response4xx.status).toBe(404);
expect(response4xx.status).toBe(400);
expect(response5xx.status).toBe(504);

const transactionEvent4xx = await transactionEventPromise4xx;
Expand Down
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://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
integrations: [
Sentry.httpIntegration({
dropSpansForIncomingRequestStatusCodes: [499, [300, 399]],
}),
],
});
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://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
integrations: [
Sentry.httpIntegration({
dropSpansForIncomingRequestStatusCodes: [499, [300, 399]],
}),
],
});
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();
});
},
);
});
});
Loading
Loading