Alert store not calling callback when an alert is deleted

The garbage collection process within the store is in charge of
determining if an alert is resolved, deleting it, and then communicating
this back to the callback set.

When an alert was explicitly deleted, these were not being communicated
back to the callback and caused the metric to report incorrect results.

Fixes prometheus#2619

Signed-off-by: gotjosh <josue@grafana.com>

provider/mem/mem.go

@@ -77,7 +77,7 @@ func NewAlerts(ctx context.Context, m types.Marker, intervalGC time.Duration, al
 		logger:    log.With(l, "component", "provider"),
 		callback:  alertCallback,
 	}
-	a.alerts.SetGCCallback(func(alerts []*types.Alert) {
+	a.alerts.SetResolvedCallback(func(alerts []*types.Alert) {
 		for _, alert := range alerts {
 			// As we don't persist alerts, we no longer consider them after
 			// they are resolved. Alerts waiting for resolved notifications are

store/store.go

@@ -48,8 +48,9 @@ func NewAlerts() *Alerts {
 	return a
 }
 
-// SetGCCallback sets a GC callback to be executed after each GC.
-func (a *Alerts) SetGCCallback(cb func([]*types.Alert)) {
+// SetResolvedCallback sets a GC callback to be executed after each GC.
+// The callback receives resolved alerts as an argument.
+func (a *Alerts) SetResolvedCallback(cb func([]*types.Alert)) {
 	a.Lock()
 	defer a.Unlock()
 
@@ -111,7 +112,13 @@ func (a *Alerts) Delete(fp model.Fingerprint) error {
 	a.Lock()
 	defer a.Unlock()
 
+	alert, ok := a.c[fp]
+	if !ok {
+		return nil
+	}
+
 	delete(a.c, fp)
+	a.cb([]*types.Alert{alert})
 	return nil
 }
 

store/store_test.go

@@ -15,6 +15,7 @@ package store
 
 import (
 	"context"
+	"sync"
 	"testing"
 	"time"
 
@@ -42,7 +43,6 @@ func TestDelete(t *testing.T) {
 		UpdatedAt: time.Now(),
 	}
 	require.NoError(t, a.Set(alert))
-
 	fp := alert.Fingerprint()
 
 	err := a.Delete(fp)
@@ -53,7 +53,8 @@ func TestDelete(t *testing.T) {
 	require.Equal(t, ErrNotFound, err)
 }
 
-func TestGC(t *testing.T) {
+func TestGCAndResolvedAlerts(t *testing.T) {
+	ac := &alertsCounter{}
 	now := time.Now()
 	newAlert := func(key string, start, end time.Duration) *types.Alert {
 		return &types.Alert{
@@ -72,21 +73,34 @@ func TestGC(t *testing.T) {
 		newAlert("a", -10, -5),
 		newAlert("d", -10, -1),
 	}
+	deleted := []*types.Alert{
+		newAlert("e", -10, 30),
+	}
 	s := NewAlerts()
 	var (
-		n           int
 		done        = make(chan struct{})
 		ctx, cancel = context.WithCancel(context.Background())
 	)
-	s.SetGCCallback(func(a []*types.Alert) {
-		n += len(a)
-		if n >= len(resolved) {
+	s.SetResolvedCallback(func(a []*types.Alert) {
+		ac.Inc(len(a))
+		if ac.Count() >= len(resolved) {
 			cancel()
 		}
 	})
-	for _, alert := range append(active, resolved...) {
+	// Merge all of alerts into a single slice.
+	all := []*types.Alert{}
+	all = append(all, active...)
+	all = append(all, resolved...)
+	all = append(all, deleted...)
+
+	for _, alert := range all {
 		require.NoError(t, s.Set(alert))
 	}
+
+	for _, alert := range deleted {
+		require.NoError(t, s.Delete(alert.Fingerprint()))
+	}
+
 	go func() {
 		s.Run(ctx, 10*time.Millisecond)
 		close(done)
@@ -108,5 +122,24 @@ func TestGC(t *testing.T) {
 			t.Errorf("alert %v should have been gc'd", alert)
 		}
 	}
-	require.Equal(t, len(resolved), n)
+	// If we manually delete an alert it should be sent back to the callback as it should be considered resolved.
+	// So in total, the callback should have received 3 alerts. 2 that were GC and 1 that was manually deleted.
+	require.Equal(t, 3, ac.Count())
+}
+
+type alertsCounter struct {
+	mtx     sync.Mutex
+	counter int
+}
+
+func (c *alertsCounter) Inc(n int) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	c.counter += n
+}
+
+func (c *alertsCounter) Count() int {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	return c.counter
 }