Fix notifications for flapping alerts #1071
Conversation
| if !r.integration.conf.SendResolved() { | ||
| firing, ok := FiringAlerts(ctx) | ||
| if !ok { | ||
| return ctx, alerts, fmt.Errorf("firing alerts missing") |
There was a problem hiding this comment.
If we error out here the MultiStage will abort the further execution, therefore the resolved subset of this set of alerts will not be gossiped. My understanding is that if there are no firing alerts in this set of alerts, but there are resolved alerts, we still want to gossip that.
There was a problem hiding this comment.
Yep. OTOH isn't this the same thing we are doing here? https://github.com/prometheus/alertmanager/blob/master/notify/notify.go#L627
AFAICS if this errors it means there's a bug in the code that should be fixed, as the context key should always be set earlier in the pipeline?
There was a problem hiding this comment.
Looks good to me. For anyone looking at the code, the logic for filtering resolved alerts is handled at https://github.com/prometheus/alertmanager/blob/master/notify/impl.go#L73-L81
Fixes #1063