Fix repeated resolved alerts

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>

Author: damnever

dispatch/dispatch.go

@@ -15,6 +15,7 @@ package dispatch
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 	"sync"
@@ -474,8 +475,12 @@ func (ag *aggrGroup) stop() {
 
 // insert inserts the alert into the aggregation group.
 func (ag *aggrGroup) insert(alert *types.Alert) {
-	if err := ag.alerts.Set(alert); err != nil {
-		level.Error(ag.logger).Log("msg", "error on set alert", "err", err)
+	if err := ag.alerts.SetOrReplaceResolved(alert); err != nil {
+		if errors.Is(err, store.ErrNotFound) {
+			level.Warn(ag.logger).Log("msg", "ignore resolved alert since there is no corresponding record in the store")
+		} else {
+			level.Error(ag.logger).Log("msg", "error on set alert", "err", err)
+		}
 	}
 
 	// Immediately trigger a flush if the wait duration for this
@@ -516,17 +521,15 @@ func (ag *aggrGroup) flush(notify func(...*types.Alert) bool) {
 
 	if notify(alertsSlice...) {
 		for _, a := range alertsSlice {
-			// Only delete if the fingerprint has not been inserted
+			// Only delete the resolved alert if the fingerprint has not been active
 			// again since we notified about it.
-			fp := a.Fingerprint()
-			got, err := ag.alerts.Get(fp)
-			if err != nil {
-				// This should never happen.
-				level.Error(ag.logger).Log("msg", "failed to get alert", "err", err, "alert", a.String())
+			if !a.Resolved() {
 				continue
 			}
-			if a.Resolved() && got.UpdatedAt == a.UpdatedAt {
-				if err := ag.alerts.Delete(fp); err != nil {
+			if err := ag.alerts.DeleteIfResolved(a.Fingerprint()); err != nil {
+				if errors.Is(err, store.ErrNotResolved) {
+					level.Debug(ag.logger).Log("msg", "resolved alert has been active again", "alert", a.String())
+				} else {
 					level.Error(ag.logger).Log("msg", "error on delete alert", "err", err, "alert", a.String())
 				}
 			}

store/store.go

@@ -24,8 +24,12 @@ import (
 	"github.com/prometheus/alertmanager/types"
 )
 
-// ErrNotFound is returned if a Store cannot find the Alert.
-var ErrNotFound = errors.New("alert not found")
+var (
+	// ErrNotFound is returned if a Store cannot find the Alert.
+	ErrNotFound = errors.New("alert not found")
+	// ErrNotResolved is returned if the alert is not resolved.
+	ErrNotResolved = errors.New("alert is not resolved")
+)
 
 // Alerts provides lock-coordinated to an in-memory map of alerts, keyed by
 // their fingerprint. Resolved alerts are removed from the map based on
@@ -98,10 +102,26 @@ func (a *Alerts) Get(fp model.Fingerprint) (*types.Alert, error) {
 
 // Set unconditionally sets the alert in memory.
 func (a *Alerts) Set(alert *types.Alert) error {
+	fp := alert.Fingerprint()
+
 	a.Lock()
 	defer a.Unlock()
 
-	a.c[alert.Fingerprint()] = alert
+	a.c[fp] = alert
+	return nil
+}
+
+// SetOrReplaceResolved returns ErrNotFound if the alert is resolved and
+// there is no corresponding record in the store.
+func (a *Alerts) SetOrReplaceResolved(alert *types.Alert) error {
+	fp := alert.Fingerprint()
+
+	a.Lock()
+	defer a.Unlock()
+	if _, ok := a.c[fp]; !ok && alert.Resolved() {
+		return ErrNotFound
+	}
+	a.c[fp] = alert
 	return nil
 }
 
@@ -114,6 +134,20 @@ func (a *Alerts) Delete(fp model.Fingerprint) error {
 	return nil
 }
 
+// DeleteIfResolved removes the Alert if it is resolved.
+func (a *Alerts) DeleteIfResolved(fp model.Fingerprint) error {
+	a.Lock()
+	defer a.Unlock()
+
+	if exist, ok := a.c[fp]; !ok {
+		return ErrNotFound
+	} else if !exist.Resolved() {
+		return ErrNotResolved
+	}
+	delete(a.c, fp)
+	return nil
+}
+
 // List returns a slice of Alerts currently held in memory.
 func (a *Alerts) List() []*types.Alert {
 	a.Lock()

store/store_test.go

@@ -54,17 +54,33 @@ func TestDelete(t *testing.T) {
 	require.Equal(t, ErrNotFound, err)
 }
 
+func TestResolved(t *testing.T) {
+	a := NewAlerts()
+
+	now := time.Now()
+	require.NoError(t, a.SetOrReplaceResolved(makeAlert("a", now, -2, 10)))
+	resolved := makeAlert("a", now, -2, -1)
+	require.NoError(t, a.SetOrReplaceResolved(resolved))
+	require.NoError(t, a.SetOrReplaceResolved(resolved))
+	a.gc()
+	require.ErrorIs(t, a.SetOrReplaceResolved(resolved), ErrNotFound)
+
+	require.ErrorIs(t, a.DeleteIfResolved(resolved.Fingerprint()), ErrNotFound)
+	require.NoError(t, a.SetOrReplaceResolved(makeAlert("a", now, -2, 10)))
+	require.ErrorIs(t, a.DeleteIfResolved(resolved.Fingerprint()), ErrNotResolved)
+	require.NoError(t, a.SetOrReplaceResolved(resolved))
+	require.NoError(t, a.DeleteIfResolved(resolved.Fingerprint()))
+	_, err := a.Get(resolved.Fingerprint())
+	require.ErrorIs(t, err, ErrNotFound)
+}
+
 func TestGC(t *testing.T) {
 	now := time.Now()
+
 	newAlert := func(key string, start, end time.Duration) *types.Alert {
-		return &types.Alert{
-			Alert: model.Alert{
-				Labels:   model.LabelSet{model.LabelName(key): "b"},
-				StartsAt: now.Add(start * time.Minute),
-				EndsAt:   now.Add(end * time.Minute),
-			},
-		}
+		return makeAlert(key, now, start, end)
 	}
+
 	active := []*types.Alert{
 		newAlert("b", 10, 20),
 		newAlert("c", -10, 10),
@@ -111,3 +127,13 @@ func TestGC(t *testing.T) {
 	}
 	require.Equal(t, len(resolved), n)
 }
+
+func makeAlert(key string, now time.Time, start, end time.Duration) *types.Alert {
+	return &types.Alert{
+		Alert: model.Alert{
+			Labels:   model.LabelSet{model.LabelName(key): "b"},
+			StartsAt: now.Add(start * time.Minute),
+			EndsAt:   now.Add(end * time.Minute),
+		},
+	}
+}