diff --git a/pkg/services/alerting/eval_context.go b/pkg/services/alerting/eval_context.go index e92edd2af12..f5663deb8ca 100644 --- a/pkg/services/alerting/eval_context.go +++ b/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) } diff --git a/pkg/services/alerting/eval_context_test.go b/pkg/services/alerting/eval_context_test.go index 5cff2996ca6..019ca1ed01f 100644 --- a/pkg/services/alerting/eval_context_test.go +++ b/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) - }) - }) }) } diff --git a/pkg/services/alerting/interfaces.go b/pkg/services/alerting/interfaces.go index 0955155575c..18f969ba1b9 100644 --- a/pkg/services/alerting/interfaces.go +++ b/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 diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index be74d41cc6c..32bece31820 100644 --- a/pkg/services/alerting/notifier.go +++ b/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,14 +41,12 @@ 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 } - n.log.Info("Sending notifications for", "ruleId", context.Rule.Id, "sent count", len(notifiers)) - if len(notifiers) == 0 { return nil } @@ -109,7 +107,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 +119,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 +137,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) diff --git a/pkg/services/alerting/notifier_test.go b/pkg/services/alerting/notifier_test.go deleted file mode 100644 index 5e378dec890..00000000000 --- a/pkg/services/alerting/notifier_test.go +++ /dev/null @@ -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) - }) - }) -} diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index dc22e1acaa0..2308a5943d6 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/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 } diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go new file mode 100644 index 00000000000..4225e203a3d --- /dev/null +++ b/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) + }) + }) + }) +} diff --git a/pkg/services/alerting/notifiers/dingding.go b/pkg/services/alerting/notifiers/dingding.go index e32b9d34f91..c2029b1173c 100644 --- a/pkg/services/alerting/notifiers/dingding.go +++ b/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 diff --git a/pkg/services/alerting/notifiers/dingding_test.go b/pkg/services/alerting/notifiers/dingding_test.go index 3ca267dbf5b..b21815f95af 100644 --- a/pkg/services/alerting/notifiers/dingding_test.go +++ b/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 := `{ }` diff --git a/pkg/services/alerting/notifiers/email.go b/pkg/services/alerting/notifiers/email.go index 7e8c4b33c0c..f84dc886d83 100644 --- a/pkg/services/alerting/notifiers/email.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/hipchat.go b/pkg/services/alerting/notifiers/hipchat.go index f1f63d42a04..b65f25b1422 100644 --- a/pkg/services/alerting/notifiers/hipchat.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/kafka.go b/pkg/services/alerting/notifiers/kafka.go index 92f6489106b..e885d44405d 100644 --- a/pkg/services/alerting/notifiers/kafka.go +++ b/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 diff --git a/pkg/services/alerting/notifiers/line.go b/pkg/services/alerting/notifiers/line.go index 4fbaa2d543e..bc0b0c984a4 100644 --- a/pkg/services/alerting/notifiers/line.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/opsgenie.go b/pkg/services/alerting/notifiers/opsgenie.go index aea02465177..1a812f49ca3 100644 --- a/pkg/services/alerting/notifiers/opsgenie.go +++ b/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 diff --git a/pkg/services/alerting/notifiers/pagerduty.go b/pkg/services/alerting/notifiers/pagerduty.go index c36dde63943..35f90a9e4b7 100644 --- a/pkg/services/alerting/notifiers/pagerduty.go +++ b/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 { diff --git a/pkg/services/alerting/notifiers/pushover.go b/pkg/services/alerting/notifiers/pushover.go index ecb4ed42e3e..6f8d0fb99e6 100644 --- a/pkg/services/alerting/notifiers/pushover.go +++ b/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 { diff --git a/pkg/services/alerting/notifiers/sensu.go b/pkg/services/alerting/notifiers/sensu.go index 9f77801d458..7a34d51b493 100644 --- a/pkg/services/alerting/notifiers/sensu.go +++ b/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") diff --git a/pkg/services/alerting/notifiers/slack.go b/pkg/services/alerting/notifiers/slack.go index e051a71740a..c5bb9344e30 100644 --- a/pkg/services/alerting/notifiers/slack.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/slack_test.go b/pkg/services/alerting/notifiers/slack_test.go index 13f8c7b48b7..dd82973bc95 100644 --- a/pkg/services/alerting/notifiers/slack_test.go +++ b/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") }) - }) }) } diff --git a/pkg/services/alerting/notifiers/teams.go b/pkg/services/alerting/notifiers/teams.go index 200a8594428..605b2742325 100644 --- a/pkg/services/alerting/notifiers/teams.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/telegram.go b/pkg/services/alerting/notifiers/telegram.go index 7fb029e57c8..62da584d019 100644 --- a/pkg/services/alerting/notifiers/telegram.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/threema.go b/pkg/services/alerting/notifiers/threema.go index e4ffffc9108..b8455dcfbfd 100644 --- a/pkg/services/alerting/notifiers/threema.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/victorops.go b/pkg/services/alerting/notifiers/victorops.go index 4b4db553cde..a2b770dfd02 100644 --- a/pkg/services/alerting/notifiers/victorops.go +++ b/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) diff --git a/pkg/services/alerting/notifiers/webhook.go b/pkg/services/alerting/notifiers/webhook.go index 4c97ed2b75e..d2d6ec636b7 100644 --- a/pkg/services/alerting/notifiers/webhook.go +++ b/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") diff --git a/pkg/services/alerting/notifiers/webhook_test.go b/pkg/services/alerting/notifiers/webhook_test.go index eb25130e4e1..b2d944eb6e9 100644 --- a/pkg/services/alerting/notifiers/webhook_test.go +++ b/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") }) }) diff --git a/pkg/services/alerting/result_handler.go b/pkg/services/alerting/result_handler.go index 448b4ace5bb..8f9deb758a6 100644 --- a/pkg/services/alerting/result_handler.go +++ b/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 }