feat(react-router): Drop low-quality transactions via ignoreSpans#20514
Open
nicohrubec wants to merge 7 commits intodevelopfrom
Open
feat(react-router): Drop low-quality transactions via ignoreSpans#20514nicohrubec wants to merge 7 commits intodevelopfrom
ignoreSpans#20514nicohrubec wants to merge 7 commits intodevelopfrom
Conversation
Contributor
size-limit report 📦
|
nicohrubec
changed the title
ignoreSpans
mydea
reviewed
Apr 27, 2026
mydea
reviewed
Apr 27, 2026
| return [...getNodeDefaultIntegrations(options), reactRouterServerIntegration()]; | ||
| } | ||
|
|
||
| const LOW_QUALITY_TRANSACTIONS_REGEXES = [ |
Member
There was a problem hiding this comment.
m: Let's not delete the integration, but let's do this inside of the integration, in beforeAllSetup or something like this! This way, users can also opt-out of this behavior, for example. It is also technically breaking to remove an integration like this as users may import it (theoretically...)
Member
Author
There was a problem hiding this comment.
updated to keep the integration and set ignoreSpans in the beforeSetup hook
nicohrubec
force-pushed
the
nh/span-streaming-reactrouter-lowqualitytransaction
branch
2 times, most recently
from
2d26ee8 to
d4e8438
Compare
nicohrubec
force-pushed
the
nh/span-streaming-reactrouter-lowqualitytransaction
branch
from
d4e8438 to
828583d
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3cf652b. Configure here.
mydea
reviewed
Apr 27, 2026
mydea
reviewed
Apr 27, 2026
mydea
reviewed
Apr 27, 2026
mydea
approved these changes
Apr 27, 2026
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| /GET \/node_modules\//, | ||
| /GET \/favicon\.ico/, | ||
| /GET \/@id\//, | ||
| /GET \/__manifest\?/, |
There was a problem hiding this comment.
Bug: The regex /GET \/__manifest\?/ will not match in the streaming path because query strings are stripped from the URL before matching, rendering the filter ineffective.
Severity: MEDIUM
Suggested Fix
Update the regular expression to not expect a query string. Change /GET \/__manifest\?/ to /GET \/__manifest/ to ensure it matches the span description after the query string has been removed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/react-router/src/server/integration/lowQualityTransactionsFilterIntegration.ts#L9
Potential issue: The regular expression `/GET \/__manifest\?/` is used to filter
low-quality transactions. This expression requires a literal `?` character to match.
However, the logic for the streaming path uses `inferSpanData()`, which strips query
strings and fragments from URLs before the regex is applied. For a request like `GET
/__manifest?p=...`, the description becomes `GET /__manifest`, which will not be matched
by the regex. This causes the filter to silently fail for all transactions processed via
the streaming path, defeating one of the primary goals of the change.
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the React Router low-quality transactions filter from a dedicated event-processor integration (
lowQualityTransactionsFilterIntegration) toignoreSpansso it also works in the streaming path. Adds some unit tests plus a new e2e test inreact-router-7-frameworkthat asserts no server transaction is sent for filtered/__manifest?requests during client-side navigation (no e2e covered this filter before).Closes #20362