Skip to content
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

feat(inbound-filters): Move filters to generic filters #73840

Merged
merged 16 commits into from
Jul 9, 2024

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Jul 4, 2024

This PR ports the first two inbound filters to the new generic filters implementation, namely chunk load errors and hydration errors.

The definition of the new generic filter is very close to the previous hardcoded definition in Relay (for the errorMessages inbound filter); however, it differs in the matching of the ty and value. In Relay, there is an additional step in which, if both ty and value are set, Relay will try to match the string in the form "{ty}: {value}", which is currently not expressible with RuleCondition constructs.

Our plan is to roll out this implementation and check how many occurrences of the error messages are still going through the old filter and act accordingly.

Closes: getsentry/relay#2850

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 4, 2024
@iambriccardo iambriccardo marked this pull request as ready for review July 8, 2024 09:42
@iambriccardo iambriccardo requested a review from a team as a code owner July 8, 2024 09:42
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 78.43137% with 11 lines in your changes missing coverage. Please review.

Project coverage is 78.10%. Comparing base (5ac86dc) to head (dd4c9af).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #73840    +/-   ##
========================================
  Coverage   78.10%   78.10%            
========================================
  Files        6660     6667     +7     
  Lines      297718   297609   -109     
  Branches    51232    51204    -28     
========================================
- Hits       232546   232461    -85     
+ Misses      58902    58871    -31     
- Partials     6270     6277     +7     
Files Coverage Δ
src/sentry/options/defaults.py 100.00% <100.00%> (ø)
src/sentry/relay/globalconfig.py 96.55% <100.00%> (ø)
src/sentry/relay/types/__init__.py 100.00% <100.00%> (ø)
src/sentry/relay/types/generic_filters.py 100.00% <100.00%> (ø)
src/sentry/relay/config/__init__.py 90.88% <55.55%> (-0.79%) ⬇️
src/sentry/relay/config/generic_filters.py 74.07% <74.07%> (ø)

... and 77 files with indirect coverage changes


generic_filter = generic_filter_fn()
if generic_filter is not None:
generic_filter["id"] = generic_filter_id
Copy link
Member Author

Choose a reason for hiding this comment

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

Decided to put the id here. A bit risky but reduces duplication and arguments of the closures.

Comment on lines 358 to 359
("*", "*https://reactjs.org/docs/error-decoder.html?invariant={418,419,422,423,425}*"),
("*", "*https://react.dev/errors/{418,419,422,423,425}*"),
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, in this case we omit the check for event.exceptions.ty.

@iambriccardo iambriccardo merged commit f3af800 into master Jul 9, 2024
49 checks passed
@iambriccardo iambriccardo deleted the riccardo/feat/generic-inbound-filters branch July 9, 2024 09:25
@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inbound Filter Outcome Reason
2 participants