From acfd998fa615fb8b06069b1bc14e3c2889f748ef Mon Sep 17 00:00:00 2001
From: Alexander Akhmetov <me@alx.cx>
Date: Mon, 14 Apr 2025 21:40:44 +0200
Subject: [PATCH] Alerting: Send resolved notifications immediately for state
 deleted states (#103996)

What is this feature?

Send resolved notifications not only when an alert state becomes stale (series is missing) and transitions from Alerting to Normal, but also from Error, NoData and Recovering.

Why do we need this feature?

Previously, when an alert state became stale or was deleted, it would transition to Normal but wouldn't trigger resolved notifications to the Alertmanager. This meant we relied on the Alertmanager to send resolved notifications when the alert expires. However, if the Alertmanager state is lost, these resolved notifications would never be sent, leaving users with firing alerts in their notification channels. This PR ensures that any transition from a firing state (Alerting, Error, NoData, Recovering) to Normal triggers a resolved notification.
---
 pkg/services/ngalert/state/manager.go              |  8 ++++----
 pkg/services/ngalert/state/manager_private_test.go |  6 ++++--
 pkg/services/ngalert/state/state.go                | 14 ++++++++++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go
index e4bac66ab93..65af2e40255 100644
--- a/pkg/services/ngalert/state/manager.go
+++ b/pkg/services/ngalert/state/manager.go
@@ -247,9 +247,8 @@ func (st *Manager) DeleteStateByRuleUID(ctx context.Context, ruleKey ngModels.Al
 			startsAt = now
 		}
 		s.SetNormal(reason, startsAt, now)
-		// Set Resolved property so the scheduler knows to send a postable alert
-		// to Alertmanager.
-		if oldState == eval.Alerting || oldState == eval.Error || oldState == eval.NoData || oldState == eval.Recovering {
+		// By setting ResolvedAt we trigger the scheduler to send a resolved notification to the Alertmanager.
+		if s.ShouldBeResolved(oldState) {
 			s.ResolvedAt = &now
 		} else {
 			s.ResolvedAt = nil
@@ -535,7 +534,8 @@ func (st *Manager) deleteStaleStatesFromCache(logger log.Logger, evaluatedAt tim
 		s.EndsAt = evaluatedAt
 		s.LastEvaluationTime = evaluatedAt
 
-		if oldState == eval.Alerting {
+		// By setting ResolvedAt we trigger the scheduler to send a resolved notification to the Alertmanager.
+		if s.ShouldBeResolved(oldState) {
 			s.ResolvedAt = &evaluatedAt
 			image := takeImageFn("stale state")
 			if image != nil {
diff --git a/pkg/services/ngalert/state/manager_private_test.go b/pkg/services/ngalert/state/manager_private_test.go
index e440ac2a1c3..7c41ee4a859 100644
--- a/pkg/services/ngalert/state/manager_private_test.go
+++ b/pkg/services/ngalert/state/manager_private_test.go
@@ -2628,7 +2628,8 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) {
 									StartsAt:           t1,
 									EndsAt:             t3,
 									LastEvaluationTime: t3,
-									LastSentAt:         &t1, // We don't bother updating LastSentAt for StateReasonMissingSeries since it's deleted from state.
+									ResolvedAt:         &t3,
+									LastSentAt:         &t3,
 								},
 							},
 						},
@@ -3539,7 +3540,8 @@ func TestProcessEvalResults_StateTransitions(t *testing.T) {
 									StartsAt:           t1,
 									EndsAt:             t3,
 									LastEvaluationTime: t3,
-									LastSentAt:         &t1, // We don't bother updating LastSentAt for StateReasonMissingSeries since it's deleted from state.
+									LastSentAt:         &t3,
+									ResolvedAt:         &t3,
 									Annotations: mergeLabels(baseRule.Annotations, data.Labels{
 										"Error": datasourceError.Error(),
 									}),
diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go
index 53d19cbf982..47957428c19 100644
--- a/pkg/services/ngalert/state/state.go
+++ b/pkg/services/ngalert/state/state.go
@@ -651,6 +651,20 @@ func (a *State) IsStale() bool {
 	return a.StateReason == models.StateReasonMissingSeries
 }
 
+// If the state is Normal, and the previous state was Alerting, Error, NoData, or Recovering,
+// we can consider the state to be resolved. This is used to determine if we should send a resolved notification.
+func (a *State) ShouldBeResolved(oldState eval.State) bool {
+	if a.State != eval.Normal {
+		return false
+	}
+
+	if oldState != eval.Alerting && oldState != eval.Error && oldState != eval.NoData && oldState != eval.Recovering {
+		return false
+	}
+
+	return true
+}
+
 // shouldTakeImage determines whether a new image should be taken for a given transition. This should return true when
 // newly transitioning to an alerting state, when no valid image exists, or when the alert has been resolved.
 func shouldTakeImage(state, previousState eval.State, previousImage *models.Image, resolved bool) string {