Merge branch 'Thib17-master'

* Thib17-master:
  removes verbose logging
  Add a per-notifier ShouldNotify()

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,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)

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
 }