-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ref(aci): optional tag filters on event frequency conditions #85215
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
❌ Your patch check has failed because the patch coverage (42.16%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #85215 +/- ##
==========================================
- Coverage 87.94% 87.93% -0.02%
==========================================
Files 9638 9634 -4
Lines 544341 544223 -118
Branches 21124 21124
==========================================
- Hits 478738 478567 -171
- Misses 65298 65351 +53
Partials 305 305 |
|
||
with self.disable_consistent_snuba_mode(duration): | ||
result = self.batch_query( | ||
group_ids=group_ids, | ||
start=start, | ||
end=end, | ||
environment_id=environment_id, | ||
conditions=conditions or None, |
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 start using the name conditions
for the function arguments after the filters
have been converted to snuba conditions. other naming suggestions would be appreciated
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.
name makes sense to me
@@ -134,9 +140,68 @@ def get_value_from_groups( | |||
result = group.get(value) | |||
return result | |||
|
|||
@staticmethod | |||
def convert_filter_to_snuba_condition( |
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.
taken from
sentry/src/sentry/rules/conditions/event_frequency.py
Lines 724 to 770 in 8fddfa8
@staticmethod | |
def convert_rule_condition_to_snuba_condition( | |
condition: dict[str, Any] | |
) -> tuple[str, str, str | list[str]] | None: | |
if condition["id"] != "sentry.rules.filters.tagged_event.TaggedEventFilter": | |
return None | |
lhs = f"tags[{condition['key']}]" | |
rhs = condition["value"] | |
match condition["match"]: | |
case MatchType.EQUAL: | |
operator = Op.EQ | |
case MatchType.NOT_EQUAL: | |
operator = Op.NEQ | |
case MatchType.STARTS_WITH: | |
operator = Op.LIKE | |
rhs = f"{rhs}%" | |
case MatchType.NOT_STARTS_WITH: | |
operator = Op.NOT_LIKE | |
rhs = f"{rhs}%" | |
case MatchType.ENDS_WITH: | |
operator = Op.LIKE | |
rhs = f"%{rhs}" | |
case MatchType.NOT_ENDS_WITH: | |
operator = Op.NOT_LIKE | |
rhs = f"%{rhs}" | |
case MatchType.CONTAINS: | |
operator = Op.LIKE | |
rhs = f"%{rhs}%" | |
case MatchType.NOT_CONTAINS: | |
operator = Op.NOT_LIKE | |
rhs = f"%{rhs}%" | |
case MatchType.IS_SET: | |
operator = Op.IS_NOT_NULL | |
rhs = None | |
case MatchType.NOT_SET: | |
operator = Op.IS_NULL | |
rhs = None | |
case MatchType.IS_IN: | |
operator = Op.IN | |
rhs = rhs.split(",") | |
case MatchType.NOT_IN: | |
operator = Op.NOT_IN | |
rhs = rhs.split(",") | |
case _: | |
raise ValueError(f"Unsupported match type: {condition['match']}") | |
return (lhs, operator.value, rhs) |
We will be allowing event frequency conditions with additional tag filters in workflow engine. This PR modifies the existing slow condition handlers to:
filters
in the handlers'comparison_json_schema
, which have the same shape asTaggedEventConditionHandler.comparison_json_schema