Don't notify resolved alerts if none were firing (#1198)

* Don't notify resolved alerts if none were firing

* Fix comments

dispatch/dispatch.go

@@ -387,8 +387,7 @@ func (ag *aggrGroup) stop() {
 	<-ag.done
 }
 
-// insert inserts the alert into the aggregation group. If the aggregation group
-// is empty afterwards, it returns true.
+// insert inserts the alert into the aggregation group.
 func (ag *aggrGroup) insert(alert *types.Alert) {
 	ag.mtx.Lock()
 	defer ag.mtx.Unlock()

nflog/nflog.go

@@ -46,7 +46,7 @@ type Log interface {
 	// alert object.
 	Log(r *pb.Receiver, key string, firing, resolved []uint64) error
 
-	// Query the log along the given Paramteres.
+	// Query the log along the given Parameters.
 	//
 	// TODO(fabxc):
 	// - extend the interface by a `QueryOne` method?

notify/notify.go

@@ -481,6 +481,15 @@ func (n *DedupStage) needsUpdate(entry *nflogpb.Entry, firing, resolved map[uint
 		return true, nil
 	}
 
+	// If the current alert group and last notification contain no firing alert
+	// and the resolved alerts are different, it means that some alerts have
+	// been fired and resolved during the last group_wait interval. In this
+	// case, there is no need to notify the receiver since it doesn't know
+	// about them.
+	if len(firing) == 0 && len(entry.FiringAlerts) == 0 {
+		return false, nil
+	}
+
 	if !entry.IsResolvedSubset(resolved) {
 		return true, nil
 	}

notify/notify_test.go

@@ -97,9 +97,10 @@ func TestDedupStageNeedsUpdate(t *testing.T) {
 	now := utcNow()
 
 	cases := []struct {
-		entry        *nflogpb.Entry
-		firingAlerts map[uint64]struct{}
-		repeat       time.Duration
+		entry          *nflogpb.Entry
+		firingAlerts   map[uint64]struct{}
+		resolvedAlerts map[uint64]struct{}
+		repeat         time.Duration
 
 		res    bool
 		resErr bool
@@ -135,6 +136,14 @@ func TestDedupStageNeedsUpdate(t *testing.T) {
 			repeat:       10 * time.Minute,
 			firingAlerts: alertHashSet(1, 2, 3),
 			res:          true,
+		}, {
+			entry: &nflogpb.Entry{
+				ResolvedAlerts: []uint64{1, 2, 3},
+				Timestamp:      now.Add(-11 * time.Minute),
+			},
+			repeat:         10 * time.Minute,
+			resolvedAlerts: alertHashSet(3, 4, 5),
+			res:            false,
 		},
 	}
 	for i, c := range cases {
@@ -143,7 +152,7 @@ func TestDedupStageNeedsUpdate(t *testing.T) {
 		s := &DedupStage{
 			now: func() time.Time { return now },
 		}
-		ok, err := s.needsUpdate(c.entry, c.firingAlerts, nil, c.repeat)
+		ok, err := s.needsUpdate(c.entry, c.firingAlerts, c.resolvedAlerts, c.repeat)
 		if c.resErr {
 			require.Error(t, err)
 		} else {