-
Notifications
You must be signed in to change notification settings - Fork 910
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
[draft] fix: running workflows for suppressed alerts #4188
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details:
|
@@ -183,6 +183,19 @@ def insert_events(self, tenant_id, events: typing.List[AlertDto | IncidentDto]): | |||
if not trigger.get("type") == "alert": | |||
self.logger.debug("trigger type is not alert, skipping") | |||
continue | |||
# Skipping suppressed alerts unless it's explicitly mentioned in the filter | |||
if event.status == "suppressed": |
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.
AlertStatus.SUPPRESSED
if trigger.get("filters", []) == [ | ||
{"key": "status", "value": "suppressed"} | ||
]: | ||
self.logger.debug( |
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.
should be info
@shahargl in general, how do you think if we should avoid running workflows for suppressed alerts? I wonder if we shouldn't run them by default also for |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4188 +/- ##
===========================================
+ Coverage 30.95% 46.74% +15.78%
===========================================
Files 92 164 +72
Lines 10257 16538 +6281
===========================================
+ Hits 3175 7730 +4555
- Misses 7082 8808 +1726 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@Matvey-Kuk idk - is there any context for this? did someone ask for it? because if not I would probably keep things simple |
i think that in general it should be configurable, but I would take the approach where you can specify which statuses you ignore, instead of marking those you want to execute for. triggers:
- type: alert
filters:
- key: status
value: r"(firing|acknowledged)" |
It's pretty confusing when we run Workflows for suppressed alerts: #4005
I don't like idea of just not allowing to run workflows for suppressed alerts at all, so I allowed to run workflows for them if suppressed status is mentioned in the filter.
close #4005
close #3858