-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
ref(alerts): fetch group_to_groupevent once per delayed processing task #91831
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
Conversation
267e04b
to
583eaa4
Compare
group = group_id_to_group.get(group_id) | ||
if not group: | ||
logger.info( | ||
"delayed_processing.group_not_found", |
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.
We should make it easier to understand what this situation means.
group_id_to_group
is derived from rules_to_fire
, but I think the subset of groups from rulegroup_to_event_data
that were in rules_to_fire
and we were able to find in the DB.
group_ids
is just the raw group ids from rules_to_fire
. rules_to_fire
groups are themselves a subset of the groups present in rulegroup_to_event_data
(i think).
So, my read is that not finding the group suggests something went wrong in fetching, which seems unexpected and probably worth escalating beyond logs.
If that's wrong, I think a comment clarifying what someone who sees this log message should be thinking is worthwhile.
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 can logger.error here, it will throw a sentry error
Codecov ReportAttention: Patch coverage is
|
Files with missing lines | Patch % | Lines |
---|---|---|
src/sentry/rules/processing/delayed_processing.py | 63.63% | 4 Missing |
Additional details and impacted files
@@ Coverage Diff @@
## master #91831 +/- ##
==========================================
- Coverage 82.60% 82.60% -0.01%
==========================================
Files 10350 10350
Lines 586633 586639 +6
Branches 22574 22574
==========================================
- Hits 484580 484579 -1
- Misses 100821 100828 +7
Partials 1232 1232
Because we fetch
group_to_groupevent
for every rule that we need to fire groups for, this quickly takes up space in the span tree when we are trying to debug whyfire_rules
is taking so long. Fetching it once should also improve performance a little.Rules can also fire for the same groups so this reduces some redundancy.