Add a per-notifier ShouldNotify()

This way we are able to edit notification behavior per notifier.
This would be usefull to let some notifiers send notifications,
even when the state doesn't change, or with custom condition.

Signed-off-by: Thibault Chataigner <t.chataigner@criteo.com>

pkg/services/alerting/eval_context.go

@@ -75,14 +75,6 @@ func (c *EvalContext) ShouldUpdateAlertState() bool {
 	return c.Rule.State != c.PrevAlertState
 }
 
-func (c *EvalContext) ShouldSendNotification() bool {
-	if (c.PrevAlertState == m.AlertStatePending) && (c.Rule.State == m.AlertStateOK) {
-		return false
-	}
-
-	return true
-}
-
 func (a *EvalContext) GetDurationMs() float64 {
 	return float64(a.EndTime.Nanosecond()-a.StartTime.Nanosecond()) / float64(1000000)
 }

pkg/services/alerting/eval_context_test.go

@@ -28,21 +28,5 @@ func TestAlertingEvalContext(t *testing.T) {
 				So(ctx.ShouldUpdateAlertState(), ShouldBeFalse)
 			})
 		})
-
-		Convey("Should send notifications", func() {
-			Convey("pending -> ok", func() {
-				ctx.PrevAlertState = models.AlertStatePending
-				ctx.Rule.State = models.AlertStateOK
-
-				So(ctx.ShouldSendNotification(), ShouldBeFalse)
-			})
-
-			Convey("ok -> alerting", func() {
-				ctx.PrevAlertState = models.AlertStateOK
-				ctx.Rule.State = models.AlertStateAlerting
-
-				So(ctx.ShouldSendNotification(), ShouldBeTrue)
-			})
-		})
 	})
 }

pkg/services/alerting/interfaces.go

@@ -15,7 +15,7 @@ type Notifier interface {
 	Notify(evalContext *EvalContext) error
 	GetType() string
 	NeedsImage() bool
-	PassesFilter(rule *Rule) bool
+	ShouldNotify(evalContext *EvalContext) bool
 
 	GetNotifierId() int64
 	GetIsDefault() bool

pkg/services/alerting/notifier.go

@@ -24,7 +24,7 @@ type NotifierPlugin struct {
 }
 
 type NotificationService interface {
-	Send(context *EvalContext) error
+	SendIfNeeded(context *EvalContext) error
 }
 
 func NewNotificationService() NotificationService {
@@ -41,8 +41,8 @@ func newNotificationService() *notificationService {
 	}
 }
 
-func (n *notificationService) Send(context *EvalContext) error {
-	notifiers, err := n.getNotifiers(context.Rule.OrgId, context.Rule.Notifications, context)
+func (n *notificationService) SendIfNeeded(context *EvalContext) error {
+	notifiers, err := n.getNeededNotifiers(context.Rule.OrgId, context.Rule.Notifications, context)
 	if err != nil {
 		return err
 	}
@@ -109,7 +109,7 @@ func (n *notificationService) uploadImage(context *EvalContext) (err error) {
 	return nil
 }
 
-func (n *notificationService) getNotifiers(orgId int64, notificationIds []int64, context *EvalContext) (NotifierSlice, error) {
+func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds []int64, context *EvalContext) (NotifierSlice, error) {
 	query := &m.GetAlertNotificationsToSendQuery{OrgId: orgId, Ids: notificationIds}
 
 	if err := bus.Dispatch(query); err != nil {
@@ -121,7 +121,7 @@ func (n *notificationService) getNotifiers(orgId int64, notificationIds []int64,
 		if not, err := n.createNotifierFor(notification); err != nil {
 			return nil, err
 		} else {
-			if shouldUseNotification(not, context) {
+			if not.ShouldNotify(context) {
 				result = append(result, not)
 			}
 		}
@@ -139,18 +139,6 @@ func (n *notificationService) createNotifierFor(model *m.AlertNotification) (Not
 	return notifierPlugin.Factory(model)
 }
 
-func shouldUseNotification(notifier Notifier, context *EvalContext) bool {
-	if !context.Firing {
-		return true
-	}
-
-	if context.Error != nil {
-		return true
-	}
-
-	return notifier.PassesFilter(context.Rule)
-}
-
 type NotifierFactory func(notification *m.AlertNotification) (Notifier, error)
 
 var notifierFactories map[string]*NotifierPlugin = make(map[string]*NotifierPlugin)

pkg/services/alerting/notifier_test.go

@@ -1,89 +0,0 @@
-package alerting
-
-import (
-	"testing"
-
-	"fmt"
-
-	"github.com/grafana/grafana/pkg/models"
-	m "github.com/grafana/grafana/pkg/models"
-	. "github.com/smartystreets/goconvey/convey"
-)
-
-type FakeNotifier struct {
-	FakeMatchResult bool
-}
-
-func (fn *FakeNotifier) GetType() string {
-	return "FakeNotifier"
-}
-
-func (fn *FakeNotifier) NeedsImage() bool {
-	return true
-}
-
-func (n *FakeNotifier) GetNotifierId() int64 {
-	return 0
-}
-
-func (n *FakeNotifier) GetIsDefault() bool {
-	return false
-}
-
-func (fn *FakeNotifier) Notify(alertResult *EvalContext) error { return nil }
-
-func (fn *FakeNotifier) PassesFilter(rule *Rule) bool {
-	return fn.FakeMatchResult
-}
-
-func TestAlertNotificationExtraction(t *testing.T) {
-
-	Convey("Notifier tests", t, func() {
-		Convey("none firing alerts", func() {
-			ctx := &EvalContext{
-				Firing: false,
-				Rule: &Rule{
-					State: m.AlertStateAlerting,
-				},
-			}
-			notifier := &FakeNotifier{FakeMatchResult: false}
-
-			So(shouldUseNotification(notifier, ctx), ShouldBeTrue)
-		})
-
-		Convey("execution error cannot be ignored", func() {
-			ctx := &EvalContext{
-				Firing: true,
-				Error:  fmt.Errorf("I used to be a programmer just like you"),
-				Rule: &Rule{
-					State: m.AlertStateOK,
-				},
-			}
-			notifier := &FakeNotifier{FakeMatchResult: false}
-
-			So(shouldUseNotification(notifier, ctx), ShouldBeTrue)
-		})
-
-		Convey("firing alert that match", func() {
-			ctx := &EvalContext{
-				Firing: true,
-				Rule: &Rule{
-					State: models.AlertStateAlerting,
-				},
-			}
-			notifier := &FakeNotifier{FakeMatchResult: true}
-
-			So(shouldUseNotification(notifier, ctx), ShouldBeTrue)
-		})
-
-		Convey("firing alert that dont match", func() {
-			ctx := &EvalContext{
-				Firing: true,
-				Rule:   &Rule{State: m.AlertStateOK},
-			}
-			notifier := &FakeNotifier{FakeMatchResult: false}
-
-			So(shouldUseNotification(notifier, ctx), ShouldBeFalse)
-		})
-	})
-}

pkg/services/alerting/notifiers/base.go

@@ -2,6 +2,7 @@ package notifiers
 
 import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
+	m "github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/alerting"
 )
 
@@ -25,7 +26,13 @@ func NewNotifierBase(id int64, isDefault bool, name, notifierType string, model
 	}
 }
 
-func (n *NotifierBase) PassesFilter(rule *alerting.Rule) bool {
+func defaultShouldNotify(context *alerting.EvalContext) bool {
+	if context.PrevAlertState == context.Rule.State {
+		return false
+	}
+	if (context.PrevAlertState == m.AlertStatePending) && (context.Rule.State == m.AlertStateOK) {
+		return false
+	}
 	return true
 }
 

pkg/services/alerting/notifiers/base_test.go

@@ -0,0 +1,32 @@
+package notifiers
+
+import (
+	"context"
+	"testing"
+
+	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/alerting"
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+func TestBaseNotifier(t *testing.T) {
+	Convey("Base notifier tests", t, func() {
+		Convey("should notify", func() {
+			Convey("pending -> ok", func() {
+				context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{
+					State: m.AlertStatePending,
+				})
+				context.Rule.State = m.AlertStateOK
+				So(defaultShouldNotify(context), ShouldBeFalse)
+			})
+
+			Convey("ok -> alerting", func() {
+				context := alerting.NewEvalContext(context.TODO(), &alerting.Rule{
+					State: m.AlertStateOK,
+				})
+				context.Rule.State = m.AlertStateAlerting
+				So(defaultShouldNotify(context), ShouldBeTrue)
+			})
+		})
+	})
+}

pkg/services/alerting/notifiers/dingding.go

@@ -38,6 +38,10 @@ func NewDingDingNotifier(model *m.AlertNotification) (alerting.Notifier, error)
 	}, nil
 }
 
+func (this *DingDingNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 type DingDingNotifier struct {
 	NotifierBase
 	Url string

pkg/services/alerting/notifiers/dingding_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func TestDingDingNotifier(t *testing.T) {
-	Convey("Line notifier tests", t, func() {
+	Convey("Dingding notifier tests", t, func() {
 		Convey("empty settings should return error", func() {
 			json := `{ }`
 

pkg/services/alerting/notifiers/email.go

@@ -58,6 +58,10 @@ func NewEmailNotifier(model *m.AlertNotification) (alerting.Notifier, error) {
 	}, nil
 }
 
+func (this *EmailNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *EmailNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Sending alert notification to", "addresses", this.Addresses)
 

pkg/services/alerting/notifiers/hipchat.go

@@ -75,6 +75,10 @@ type HipChatNotifier struct {
 	log    log.Logger
 }
 
+func (this *HipChatNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *HipChatNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Executing hipchat notification", "ruleId", evalContext.Rule.Id, "notification", this.Name)
 

pkg/services/alerting/notifiers/kafka.go

@@ -57,6 +57,10 @@ type KafkaNotifier struct {
 	log      log.Logger
 }
 
+func (this *KafkaNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *KafkaNotifier) Notify(evalContext *alerting.EvalContext) error {
 
 	state := evalContext.Rule.State

pkg/services/alerting/notifiers/line.go

@@ -51,6 +51,10 @@ type LineNotifier struct {
 	log   log.Logger
 }
 
+func (this *LineNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *LineNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Executing line notification", "ruleId", evalContext.Rule.Id, "notification", this.Name)
 

pkg/services/alerting/notifiers/opsgenie.go

@@ -62,6 +62,10 @@ type OpsGenieNotifier struct {
 	log       log.Logger
 }
 
+func (this *OpsGenieNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *OpsGenieNotifier) Notify(evalContext *alerting.EvalContext) error {
 
 	var err error

pkg/services/alerting/notifiers/pagerduty.go

@@ -63,6 +63,10 @@ type PagerdutyNotifier struct {
 	log         log.Logger
 }
 
+func (this *PagerdutyNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *PagerdutyNotifier) Notify(evalContext *alerting.EvalContext) error {
 
 	if evalContext.Rule.State == m.AlertStateOK && !this.AutoResolve {

pkg/services/alerting/notifiers/pushover.go

@@ -123,6 +123,10 @@ type PushoverNotifier struct {
 	log      log.Logger
 }
 
+func (this *PushoverNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *PushoverNotifier) Notify(evalContext *alerting.EvalContext) error {
 	ruleUrl, err := evalContext.GetRuleUrl()
 	if err != nil {

pkg/services/alerting/notifiers/sensu.go

@@ -71,6 +71,10 @@ type SensuNotifier struct {
 	log      log.Logger
 }
 
+func (this *SensuNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *SensuNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Sending sensu result")
 

pkg/services/alerting/notifiers/slack.go

@@ -98,6 +98,10 @@ type SlackNotifier struct {
 	log       log.Logger
 }
 
+func (this *SlackNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *SlackNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Executing slack notification", "ruleId", evalContext.Rule.Id, "notification", this.Name)
 

pkg/services/alerting/notifiers/slack_test.go

@@ -78,7 +78,6 @@ func TestSlackNotifier(t *testing.T) {
 				So(slackNotifier.Mention, ShouldEqual, "@carl")
 				So(slackNotifier.Token, ShouldEqual, "xoxb-XXXXXXXX-XXXXXXXX-XXXXXXXXXX")
 			})
-
 		})
 	})
 }

pkg/services/alerting/notifiers/teams.go

@@ -47,6 +47,10 @@ type TeamsNotifier struct {
 	log       log.Logger
 }
 
+func (this *TeamsNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *TeamsNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Executing teams notification", "ruleId", evalContext.Rule.Id, "notification", this.Name)
 

pkg/services/alerting/notifiers/telegram.go

@@ -76,6 +76,10 @@ func NewTelegramNotifier(model *m.AlertNotification) (alerting.Notifier, error)
 	}, nil
 }
 
+func (this *TelegramNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *TelegramNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Sending alert notification to", "bot_token", this.BotToken)
 	this.log.Info("Sending alert notification to", "chat_id", this.ChatID)

pkg/services/alerting/notifiers/threema.go

@@ -114,6 +114,10 @@ func NewThreemaNotifier(model *m.AlertNotification) (alerting.Notifier, error) {
 	}, nil
 }
 
+func (this *ThreemaNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (notifier *ThreemaNotifier) Notify(evalContext *alerting.EvalContext) error {
 	notifier.log.Info("Sending alert notification from", "threema_id", notifier.GatewayID)
 	notifier.log.Info("Sending alert notification to", "threema_id", notifier.RecipientID)

pkg/services/alerting/notifiers/victorops.go

@@ -68,6 +68,10 @@ type VictoropsNotifier struct {
 	log         log.Logger
 }
 
+func (this *VictoropsNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 // Notify sends notification to Victorops via POST to URL endpoint
 func (this *VictoropsNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Executing victorops notification", "ruleId", evalContext.Rule.Id, "notification", this.Name)

pkg/services/alerting/notifiers/webhook.go

@@ -65,6 +65,10 @@ type WebhookNotifier struct {
 	log        log.Logger
 }
 
+func (this *WebhookNotifier) ShouldNotify(context *alerting.EvalContext) bool {
+	return defaultShouldNotify(context)
+}
+
 func (this *WebhookNotifier) Notify(evalContext *alerting.EvalContext) error {
 	this.log.Info("Sending webhook")
 

pkg/services/alerting/notifiers/webhook_test.go

@@ -18,7 +18,7 @@ func TestWebhookNotifier(t *testing.T) {
 				settingsJSON, _ := simplejson.NewJson([]byte(json))
 				model := &m.AlertNotification{
 					Name:     "ops",
-					Type:     "email",
+					Type:     "webhook",
 					Settings: settingsJSON,
 				}
 
@@ -35,7 +35,7 @@ func TestWebhookNotifier(t *testing.T) {
 				settingsJSON, _ := simplejson.NewJson([]byte(json))
 				model := &m.AlertNotification{
 					Name:     "ops",
-					Type:     "email",
+					Type:     "webhook",
 					Settings: settingsJSON,
 				}
 
@@ -44,7 +44,7 @@ func TestWebhookNotifier(t *testing.T) {
 
 				So(err, ShouldBeNil)
 				So(webhookNotifier.Name, ShouldEqual, "ops")
-				So(webhookNotifier.Type, ShouldEqual, "email")
+				So(webhookNotifier.Type, ShouldEqual, "webhook")
 				So(webhookNotifier.Url, ShouldEqual, "http://google.com")
 			})
 		})

pkg/services/alerting/result_handler.go

@@ -85,11 +85,9 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error {
 		if err := annotationRepo.Save(&item); err != nil {
 			handler.log.Error("Failed to save annotation for new alert state", "error", err)
 		}
-
-		if evalContext.ShouldSendNotification() {
-			handler.notifier.Send(evalContext)
-		}
 	}
 
+	handler.notifier.SendIfNeeded(evalContext)
+
 	return nil
 }