-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(aci milestone 3): migrate edge case alerts #93749
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
This PR has a migration; here is the generated SQL for for --
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL |
AlertRuleTriggerActionType.SLACK.value: "Slack", | ||
AlertRuleTriggerActionType.MSTEAMS.value: "Microsoft Teams", | ||
AlertRuleTriggerActionType.OPSGENIE.value: "Opsgenie", | ||
AlertRuleTriggerActionType.DISCORD.value: "Discord", |
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.
Made sure I didn't miss this this time 😅
# resolve action filter | ||
DataCondition.objects.create( | ||
comparison=PRIORITY_MAP.get(trigger.label, DetectorPriorityLevel.HIGH), | ||
condition_result=True, | ||
type=Condition.ISSUE_PRIORITY_DEESCALATING, | ||
condition_group=data_condition_group, | ||
) |
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.
This was introduced after the initial migration, so added it here to avoid the need for a backfill
alert_rules = ( | ||
AlertRule.objects_with_snapshots.filter( | ||
status__in=[ | ||
AlertRuleStatus.PENDING.value, | ||
AlertRuleStatus.DISABLED.value, # we don't need to worry about NOT_ENOUGH_DATA here, because all anomaly detection alerts have been migrated | ||
] | ||
) | ||
.filter(~Exists(AlertRuleDetector.objects.filter(alert_rule_id=OuterRef("id")))) | ||
.filter(Exists(AlertRuleProjects.objects.filter(alert_rule_id=OuterRef("id")))) | ||
) |
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're fetching all non-deleted alert rules that 1) have not been migrated and 2) have associated projects
enabled = ( | ||
True | ||
if snoozed is None and alert_rule.status == AlertRuleStatus.PENDING.value | ||
else False | ||
) |
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.
Disable the detector if the alert rule is disabled
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #93749 +/- ##
===========================================
+ Coverage 46.09% 88.04% +41.95%
===========================================
Files 10309 10330 +21
Lines 594907 596553 +1646
Branches 23134 23134
===========================================
+ Hits 274198 525230 +251032
+ Misses 320215 70829 -249386
Partials 494 494 |
This migration creates ACI objects for the following alerts: - Alerts that were disabled during the initial migration but are now enabled - Alerts that were anomaly detection alerts and created before anomaly detection dual write was enabled, but became static/percent alerts before the anomaly detection alert migration - All disabled alert rules with projects to prevent future instances of case 1. These detectors will be set to disabled.
This migration creates ACI objects for the following alerts: