-
Notifications
You must be signed in to change notification settings - Fork 103
feat(relay-pattern): Add negation pattern matching #5116
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
base: master
Are you sure you want to change the base?
Conversation
fn parse(&mut self) -> Result<(), ErrorKind> { | ||
if self.advance_if(|c| c == '!') { | ||
self.push_token(Token::Negated); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Negated Token Incorrectly Affects Wildmatch
The Token::Negated
is incorrectly passed into the wildmatch
algorithm. This meta-token should be stripped before wildmatch
evaluation, as its inclusion causes incorrect matching, especially for patterns like !
which incorrectly match non-empty strings but not empty ones.
Additional Locations (2)
@cmanallen have you considered using Generic Inbound Filters for this? We consider the other inbound filters as deprecated and would prefer if new features are implemented through generic filters instead. |
|
||
- Add InstallableBuild and SizeAnalysis data categories. ([#5084](https://github.com/getsentry/relay/pull/5084)) | ||
- Add dynamic PII derivation to `metastructure`. ([#5107](https://github.com/getsentry/relay/pull/5107)) | ||
- Add negation pattern matching. ([#5116](https://github.com/getsentry/relay/pull/5116)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong section now.
/// The pattern is complex and needs to be evaluated using [`wildmatch`]. | ||
NegatedWildmatch(Tokens), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem correct, a Static
literal can also be inverted, we now have this implicit behavior of negated is always a wild match, which also removes a lot of the performance benefits we have by factoring out simple patterns.
You can instead track this property on the Pattern
itself and just invert the is_match
there.
if self.advance_if(|c| c == '!') { | ||
self.push_token(Token::Negated); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should make this behavior configurable via Options
and only enable it for releases for now.
We can then just use a TypedPattern
with special options for that one glob.
Prefixing
!
before a pattern will negate the pattern that follows. This is designed for use within inbound-filters (specifically release filters) where a customer wants to define an allow-list (as opposed to the current deny-list).