From 30530a784546b1d4d1ea49d0bcd2e1f58d9797e0 Mon Sep 17 00:00:00 2001
From: Simon Pasquier <spasquie@redhat.com>
Date: Mon, 15 Jan 2018 09:59:00 +0100
Subject: [PATCH 1/2] Don't notify resolved alerts if none were firing

---
 notify/notify.go      |  9 +++++++++
 notify/notify_test.go | 17 +++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/notify/notify.go b/notify/notify.go
index 9ee2e2714c..9b78ccb33a 100644
--- a/notify/notify.go
+++ b/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
 	}
diff --git a/notify/notify_test.go b/notify/notify_test.go
index 8021619172..37e3396758 100644
--- a/notify/notify_test.go
+++ b/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 {

From ce2fbec9034b3304300236def7f3ef389bb413a8 Mon Sep 17 00:00:00 2001
From: Simon Pasquier <spasquie@redhat.com>
Date: Mon, 15 Jan 2018 09:59:49 +0100
Subject: [PATCH 2/2] Fix comments

---
 dispatch/dispatch.go | 3 +--
 nflog/nflog.go       | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/dispatch/dispatch.go b/dispatch/dispatch.go
index f8461e97e2..e66c4fbc7e 100644
--- a/dispatch/dispatch.go
+++ b/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()
diff --git a/nflog/nflog.go b/nflog/nflog.go
index 4eb67a327f..1d9e141de5 100644
--- a/nflog/nflog.go
+++ b/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?