Skip to content

Conversation

mydea
Copy link
Member

@mydea mydea commented Sep 17, 2025

Looking at https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#redirection_messages, it seems that we should not filter out 300, 302 and 304 status codes by default:

300 Multiple Choices
In agent-driven content negotiation, the request has more than one possible response and the user agent or user should choose one of them. There is no standardized way for clients to automatically choose one of the responses, so this is rarely used.
304 Not Modified
This is used for caching purposes. It tells the client that the response has not been modified, so the client can continue to use the same cached version of the response.

In contrast, the others seem safe to continue to filter:

301 Moved Permanently
The URL of the requested resource has been changed permanently. The new URL is given in the response.

302 Found
This response code means that the URI of requested resource has been changed temporarily. Further changes in the URI might be made in the future, so the same URI should be used by the client in future requests.

303 See Other
The server sent this response to direct the client to get the requested resource at another URI with a GET request.

305 Use Proxy Deprecated
Defined in a previous version of the HTTP specification to indicate that a requested response must be accessed by a proxy. It has been deprecated due to security concerns regarding in-band configuration of a proxy.

306 unused
This response code is no longer used; but is reserved. It was used in a previous version of the HTTP/1.1 specification.

307 Temporary Redirect
The server sends this response to direct the client to get the requested resource at another URI with the same method that was used in the prior request. This has the same semantics as the 302 Found response code, with the exception that the user agent must not change the HTTP method used: if a POST was used in the first request, a POST must be used in the redirected request.

308 Permanent Redirect
This means that the resource is now permanently located at another URI, specified by the Location response header. This has the same semantics as the 301 Moved Permanently HTTP response code, with the exception that the user agent must not change the HTTP method used: if a POST was used in the first request, a POST must be used in the second request.

eventually it makes sense to unify this between node/node-core, but this should happen when we merge the httpIntegration split PR.

@mydea mydea self-assigned this Sep 17, 2025
/**
* Do not capture spans for incoming HTTP requests with the given status codes.
* By default, spans with 404 status code are ignored.
* By default, spans with some 3xx and 4xx status codes are ignored.
Copy link
Member

Choose a reason for hiding this comment

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

super-l: Just to clarify?

Suggested change
* By default, spans with some 3xx and 4xx status codes are ignored.
* By default, spans with some 3xx and 4xx status codes are ignored (see below).

Copy link
Contributor

github-actions bot commented Sep 17, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.16 kB - -
@sentry/browser - with treeshaking flags 22.72 kB - -
@sentry/browser (incl. Tracing) 40.16 kB - -
@sentry/browser (incl. Tracing, Replay) 78.53 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 68.24 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 83.19 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 95.4 kB - -
@sentry/browser (incl. Feedback) 40.87 kB - -
@sentry/browser (incl. sendFeedback) 28.81 kB - -
@sentry/browser (incl. FeedbackAsync) 33.75 kB - -
@sentry/react 25.88 kB - -
@sentry/react (incl. Tracing) 42.17 kB - -
@sentry/vue 28.64 kB - -
@sentry/vue (incl. Tracing) 41.98 kB - -
@sentry/svelte 24.19 kB - -
CDN Bundle 25.74 kB - -
CDN Bundle (incl. Tracing) 40.05 kB - -
CDN Bundle (incl. Tracing, Replay) 76.27 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 81.78 kB - -
CDN Bundle - uncompressed 75.18 kB - -
CDN Bundle (incl. Tracing) - uncompressed 118.5 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 233.6 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 246.37 kB - -
@sentry/nextjs (client) 44.15 kB - -
@sentry/sveltekit (client) 40.6 kB - -
@sentry/node-core 49.89 kB +0.07% +30 B 🔺
@sentry/node 151.23 kB +0.06% +78 B 🔺
@sentry/node - without tracing 91.81 kB +0.07% +58 B 🔺
@sentry/aws-serverless 105.26 kB +0.07% +67 B 🔺

View base workflow run

Copy link
Contributor

github-actions bot commented Sep 17, 2025

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 8,635 - 8,818 -2%
GET With Sentry 1,288 15% 1,431 -10%
GET With Sentry (error only) 5,913 68% 6,176 -4%
POST Baseline 1,183 - 1,203 -2%
POST With Sentry 504 43% 530 -5%
POST With Sentry (error only) 1,030 87% 1,056 -2%
MYSQL Baseline 3,242 - 3,353 -3%
MYSQL With Sentry 373 12% 499 -25%
MYSQL With Sentry (error only) 2,613 81% 2,711 -4%

View base workflow run

@mydea mydea changed the title feat(node): Do not drop 300, 302 and 304 status codes by default feat(node): Do not drop 300 and 304 status codes by default Sep 17, 2025
@mydea mydea merged commit 2cde2a4 into develop Sep 17, 2025
187 of 188 checks passed
@mydea mydea deleted the fn/no-304-filter branch September 17, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants