Merge pull request #595 from brancz/fix-send-resolved

notify: replace unfiltered with filtered alerts
prometheus · Jan 9, 2017 · 45bd4fd · 45bd4fd
2 parents cb4260d + c392ace
commit 45bd4fd
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 33 deletions.
diff --git a/notify/impl.go b/notify/impl.go
@@ -79,7 +79,7 @@ func (i *Integration) Notify(ctx context.Context, alerts ...*types.Alert) (bool,
 		return false, nil
 	}
 
-	return i.notifier.Notify(ctx, alerts...)
+	return i.notifier.Notify(ctx, res...)
 }
 
 // BuildReceiverIntegrations builds a list of integration notifiers off of a

diff --git a/notify/notify_test.go b/notify/notify_test.go
@@ -295,18 +295,44 @@ func TestRoutingStage(t *testing.T) {
 	}
 }
 
-func TestIntegration(t *testing.T) {
+func TestIntegrationNoResolved(t *testing.T) {
 	res := []*types.Alert{}
 	r := notifierFunc(func(ctx context.Context, alerts ...*types.Alert) (bool, error) {
 		res = append(res, alerts...)
 
 		return false, nil
 	})
-	i1 := Integration{
+	i := Integration{
 		notifier: r,
 		conf:     notifierConfigFunc(func() bool { return false }),
 	}
-	i2 := Integration{
+
+	alerts := []*types.Alert{
+		&types.Alert{
+			Alert: model.Alert{
+				EndsAt: time.Now().Add(-time.Hour),
+			},
+		},
+		&types.Alert{
+			Alert: model.Alert{
+				EndsAt: time.Now().Add(time.Hour),
+			},
+		},
+	}
+
+	i.Notify(nil, alerts...)
+
+	require.Equal(t, len(res), 1)
+}
+
+func TestIntegrationSendResolved(t *testing.T) {
+	res := []*types.Alert{}
+	r := notifierFunc(func(ctx context.Context, alerts ...*types.Alert) (bool, error) {
+		res = append(res, alerts...)
+
+		return false, nil
+	})
+	i := Integration{
 		notifier: r,
 		conf:     notifierConfigFunc(func() bool { return true }),
 	}
@@ -319,12 +345,9 @@ func TestIntegration(t *testing.T) {
 		},
 	}
 
-	i1.Notify(nil, alerts...)
-	i2.Notify(nil, alerts...)
+	i.Notify(nil, alerts...)
 
-	// Even though the alert is sent to both integrations, which end up being
-	// delivered to the same notifier, only one is actually delivered as the
-	// second integration filters the resolved notifications.
+	require.Equal(t, len(res), 1)
 	require.Equal(t, res, alerts)
 }
 

diff --git a/test/acceptance/send_test.go b/test/acceptance/send_test.go
@@ -373,47 +373,27 @@ receivers:
 			am.Push(At(1),
 				Alert("alertname", "test", "lbl", "v1"),
 				Alert("alertname", "test", "lbl", "v2"),
-				Alert("alertname", "test", "lbl", "v3"),
 			)
 
-			am.Push(At(16),
+			am.Push(At(7),
 				Alert("alertname", "test", "lbl", "v1"),
-				Alert("alertname", "test", "lbl", "v2"),
-				Alert("alertname", "test", "lbl", "v3"),
 			)
 
 			co1.Want(Between(2, 2.5),
 				Alert("alertname", "test", "lbl", "v1").Active(1),
 				Alert("alertname", "test", "lbl", "v2").Active(1),
-				Alert("alertname", "test", "lbl", "v3").Active(1),
 			)
 			co1.Want(Between(12, 13),
-				Alert("alertname", "test", "lbl", "v1").Active(1, 11),
+				Alert("alertname", "test", "lbl", "v1").Active(1),
 				Alert("alertname", "test", "lbl", "v2").Active(1, 11),
-				Alert("alertname", "test", "lbl", "v3").Active(1, 11),
-			)
-
-			co1.Want(Between(17, 17.5),
-				Alert("alertname", "test", "lbl", "v1").Active(16),
-				Alert("alertname", "test", "lbl", "v2").Active(16),
-				Alert("alertname", "test", "lbl", "v3").Active(16),
-			)
-			co1.Want(Between(27, 28),
-				Alert("alertname", "test", "lbl", "v1").Active(16, 26),
-				Alert("alertname", "test", "lbl", "v2").Active(16, 26),
-				Alert("alertname", "test", "lbl", "v3").Active(16, 26),
 			)
 
 			co2.Want(Between(2, 2.5),
 				Alert("alertname", "test", "lbl", "v1").Active(1),
 				Alert("alertname", "test", "lbl", "v2").Active(1),
-				Alert("alertname", "test", "lbl", "v3").Active(1),
 			)
-
-			co2.Want(Between(17, 17.5),
-				Alert("alertname", "test", "lbl", "v1").Active(16),
-				Alert("alertname", "test", "lbl", "v2").Active(16),
-				Alert("alertname", "test", "lbl", "v3").Active(16),
+			co2.Want(Between(12, 13),
+				Alert("alertname", "test", "lbl", "v1").Active(1),
 			)
 
 			at.Run()