Skip to content

feat(node): Drop 401-404 and 3xx status code spans by default #16972

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.

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 5 commits into from
Jul 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,3 @@ test('Should send a transaction and an error event for a faulty generateViewport

expect(errorEvent.transaction).toBe('Page.generateViewport (/generation-functions)');
});

Copy link
Member

Choose a reason for hiding this comment

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

Removed this test because the redirect is no longer sent.

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

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

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

expect((await transactionPromise).contexts?.trace?.status).toBe('ok');
});
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: 403 });
return NextResponse.json({ name: 'John Doe' }, { status: 400 });
}
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('permission_denied');
expect(routehandlerTransaction.contexts?.trace?.status).toBe('invalid_argument');
expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ export const appRouter = t.router({
.mutation(() => {
throw new Error('I crashed in a trpc handler');
}),
unauthorized: procedure.mutation(() => {
throw new TRPCError({ code: 'UNAUTHORIZED', cause: new Error('Unauthorized') });
badRequest: procedure.mutation(() => {
throw new TRPCError({ code: 'BAD_REQUEST', cause: new Error('Bad Request') });
}),
});

Expand Down
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/unauthorized')
!!transactionEvent.spans?.find(span => span.description === 'trpc/badRequest')
);
});

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

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.unauthorized.mutate()).rejects.toBeDefined();
await expect(trpcClient.badRequest.mutate()).rejects.toBeDefined();

await expect(transactionEventPromise).resolves.toBeDefined();
await expect(errorEventPromise).resolves.toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ app.get('/', (_req, res) => {
res.send({ response: 'response 0' });
});

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

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

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

app.get('/499', (_req, res) => {
res.status(499).send({ response: 'response 499' });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ app.get('/', (_req, res) => {
res.send({ response: 'response 0' });
});

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

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

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

app.get('/test/express', (_req, res) => {
res.send({ response: 'response 1' });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,16 @@ describe('express v5 tracing', () => {
await runner.completed();
});

test('ignores 404 routes by default', async () => {
test.each(['/401', '/402', '/403', '/does-not-exist'])('ignores %s route by default', async (url: string) => {
const runner = createRunner()
.expect({
// No transaction is sent for the 404 route
// No transaction is sent for the 401, 402, 403, 404 routes
transaction: {
transaction: 'GET /',
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
runner.makeRequest('get', url, { expectError: true });
runner.makeRequest('get', '/');
await runner.completed();
});
Expand Down Expand Up @@ -284,33 +284,41 @@ describe('express v5 tracing', () => {
'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',
// We opt-out of the default [401, 404] fitering in order to test how these spans are handled
test.each([
{ status_code: 401, url: '/401', status: 'unauthenticated' },
{ status_code: 402, url: '/402', status: 'invalid_argument' },
{ status_code: 403, url: '/403', status: 'permission_denied' },
{ status_code: 404, url: '/does-not-exist', status: 'not_found' },
])(
'handles %s route correctly',
async ({ status_code, url, status }: { status_code: number; url: string; status: string }) => {
const runner = createRunner()
.expect({
transaction: {
transaction: `GET ${url}`,
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'http.response.status_code': status_code,
url: expect.stringMatching(url),
'http.method': 'GET',
'http.url': expect.stringMatching(url),
'http.target': url,
},
op: 'http.server',
status,
},
op: 'http.server',
status: 'not_found',
},
},
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
await runner.completed();
});
})
.start();
runner.makeRequest('get', url, { expectError: true });
await runner.completed();
},
);

test('filters defined status codes', async () => {
const runner = createRunner()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ app.get('/', (_req, res) => {
res.send({ response: 'response 0' });
});

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

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

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

app.get('/499', (_req, res) => {
res.status(499).send({ response: 'response 499' });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ app.get('/', (_req, res) => {
res.send({ response: 'response 0' });
});

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

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

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

app.get('/test/express', (_req, res) => {
res.send({ response: 'response 1' });
});
Expand Down
64 changes: 36 additions & 28 deletions dev-packages/node-integration-tests/suites/express/tracing/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,16 @@ describe('express tracing', () => {
await runner.completed();
});

test('ignores 404 routes by default', async () => {
test.each(['/401', '/402', '/403', '/does-not-exist'])('ignores %s route by default', async (url: string) => {
const runner = createRunner()
.expect({
// No transaction is sent for the 404 route
// No transaction is sent for the 401, 402, 403, 404 routes
transaction: {
transaction: 'GET /',
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
runner.makeRequest('get', url, { expectError: true });
runner.makeRequest('get', '/');
await runner.completed();
});
Expand Down Expand Up @@ -315,34 +315,42 @@ describe('express tracing', () => {
'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',
// We opt-out of the default [401, 404] filtering in order to test how these spans are handled
test.each([
{ status_code: 401, url: '/401', status: 'unauthenticated' },
{ status_code: 402, url: '/402', status: 'invalid_argument' },
{ status_code: 403, url: '/403', status: 'permission_denied' },
{ status_code: 404, url: '/does-not-exist', status: 'not_found' },
])(
'handles %s route correctly',
async ({ status_code, url, status }: { status_code: number; url: string; status: string }) => {
const runner = createRunner()
.expect({
transaction: {
// TODO(v10): This is incorrect on OpenTelemetry v1 but can be fixed in v2
transaction: `GET ${status_code === 404 ? '/' : url}`,
contexts: {
trace: {
span_id: expect.stringMatching(/[a-f0-9]{16}/),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
data: {
'http.response.status_code': status_code,
url: expect.stringMatching(url),
'http.method': 'GET',
'http.url': expect.stringMatching(url),
'http.target': url,
},
op: 'http.server',
status,
},
op: 'http.server',
status: 'not_found',
},
},
},
})
.start();
runner.makeRequest('get', '/does-not-exist', { expectError: true });
await runner.completed();
});
})
.start();
runner.makeRequest('get', url, { expectError: true });
await runner.completed();
},
);

test('filters defined status codes', async () => {
const runner = createRunner()
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/rollup-utils/npmHelpers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export function makeBaseNPMConfig(options = {}) {
}

return true;
}
},
},

plugins: [nodeResolvePlugin, sucrasePlugin, debugBuildStatementReplacePlugin, rrwebBuildPlugin, cleanupPlugin],
Expand Down
7 changes: 5 additions & 2 deletions packages/node-core/src/integrations/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ interface HttpOptions {
* By default, spans with 404 status code are ignored.
* Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes.
*
* @default `[404]`
* @default `[[401, 404], [300, 399]]`
*/
dropSpansForIncomingRequestStatusCodes?: (number | [number, number])[];

Expand Down Expand Up @@ -105,7 +105,10 @@ const instrumentSentryHttp = generateInstrumentOnce<SentryHttpInstrumentationOpt
* It creates breadcrumbs for outgoing HTTP requests which will be attached to the currently active span.
*/
export const httpIntegration = defineIntegration((options: HttpOptions = {}) => {
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [404];
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [
[401, 404],
[300, 399],
];

return {
name: INTEGRATION_NAME,
Expand Down
7 changes: 5 additions & 2 deletions packages/node/src/integrations/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ interface HttpOptions {
* By default, spans with 404 status code are ignored.
* Expects an array of status codes or a range of status codes, e.g. [[300,399], 404] would ignore 3xx and 404 status codes.
*
* @default `[404]`
* @default `[[401, 404], [300, 399]]`
*/
dropSpansForIncomingRequestStatusCodes?: (number | [number, number])[];

Expand Down Expand Up @@ -184,7 +184,10 @@ 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];
const dropSpansForIncomingRequestStatusCodes = options.dropSpansForIncomingRequestStatusCodes ?? [
[401, 404],
[300, 399],
];

return {
name: INTEGRATION_NAME,
Expand Down
Loading
Loading