test(alerting): add tests for when to send notifcations

pkg/services/alerting/notifier.go

@@ -12,27 +12,33 @@ import (
 )
 
 type NotifierImpl struct {
-	log log.Logger
+	log              log.Logger
+	getNotifications func(orgId int64, notificationGroups []int64) []*Notification
 }
 
 func NewNotifier() *NotifierImpl {
+	log := log.New("alerting.notifier")
 	return &NotifierImpl{
-		log: log.New("alerting.notifier"),
+		log:              log,
+		getNotifications: buildGetNotifiers(log),
 	}
 }
 
+func (n NotifierImpl) ShouldDispath(alertResult *AlertResult, notifier *Notification) bool {
+	warn := alertResult.State == alertstates.Warn && notifier.SendWarning
+	crit := alertResult.State == alertstates.Critical && notifier.SendCritical
+	return (warn || crit) || alertResult.State == alertstates.Ok
+}
+
 func (n *NotifierImpl) Notify(alertResult *AlertResult) {
-	notifiers := n.getNotifiers(alertResult.AlertJob.Rule.OrgId, alertResult.AlertJob.Rule.NotificationGroups)
+	notifiers := n.getNotifications(alertResult.AlertJob.Rule.OrgId, alertResult.AlertJob.Rule.NotificationGroups)
 
 	for _, notifier := range notifiers {
-		warn := alertResult.State == alertstates.Warn && notifier.SendWarning
+		if n.ShouldDispath(alertResult, notifier) {
-		crit := alertResult.State == alertstates.Critical && notifier.SendCritical
-		if (warn || crit) || alertResult.State == alertstates.Ok {
 			n.log.Info("Sending notification", "state", alertResult.State, "type", notifier.Type)
 			go notifier.Notifierr.Dispatch(alertResult)
 		}
 	}
-
 }
 
 type Notification struct {
@@ -107,29 +113,31 @@ type NotificationDispatcher interface {
 	Dispatch(alertResult *AlertResult)
 }
 
-func (n *NotifierImpl) getNotifiers(orgId int64, notificationGroups []int64) []*Notification {
+func buildGetNotifiers(log log.Logger) func(orgId int64, notificationGroups []int64) []*Notification {
-	query := &m.GetAlertNotificationQuery{
+	return func(orgId int64, notificationGroups []int64) []*Notification {
-		OrgID:                orgId,
+		query := &m.GetAlertNotificationQuery{
-		Ids:                  notificationGroups,
+			OrgID:                orgId,
-		IncludeAlwaysExecute: true,
+			Ids:                  notificationGroups,
-	}
+			IncludeAlwaysExecute: true,
-	err := bus.Dispatch(query)
+		}
-	if err != nil {
+		err := bus.Dispatch(query)
-		n.log.Error("Failed to read notifications", "error", err)
+		if err != nil {
-	}
+			log.Error("Failed to read notifications", "error", err)
-
-	var result []*Notification
-	n.log.Info("notifiriring", "count", len(query.Result), "groups", notificationGroups)
-	for _, notification := range query.Result {
-		not, err := NewNotificationFromDBModel(notification)
-		if err == nil {
-			result = append(result, not)
-		} else {
-			n.log.Error("Failed to read notification model", "error", err)
 		}
-	}
 
-	return result
+		var result []*Notification
+		log.Info("notifiriring", "count", len(query.Result), "groups", notificationGroups)
+		for _, notification := range query.Result {
+			not, err := NewNotificationFromDBModel(notification)
+			if err == nil {
+				result = append(result, not)
+			} else {
+				log.Error("Failed to read notification model", "error", err)
+			}
+		}
+
+		return result
+	}
 }
 
 func NewNotificationFromDBModel(model *m.AlertNotification) (*Notification, error) {

pkg/services/alerting/notifier_test.go

@@ -7,93 +7,119 @@ import (
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	m "github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/alerting/alertstates"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestAlertNotificationExtraction(t *testing.T) {
+	Convey("Notifier tests", t, func() {
+		Convey("rules for sending notifications", func() {
+			dummieNotifier := NotifierImpl{}
 
-	Convey("Parsing alert notification from settings", t, func() {
+			result := &AlertResult{
-		Convey("Parsing email", func() {
+				State: alertstates.Critical,
-			Convey("empty settings should return error", func() {
+			}
-				json := `{ }`
 
-				settingsJSON, _ := simplejson.NewJson([]byte(json))
+			notifier := &Notification{
-				model := &m.AlertNotification{
+				Name:         "Test Notifier",
-					Name:     "ops",
+				Type:         "TestType",
-					Type:     "email",
+				SendCritical: true,
-					Settings: settingsJSON,
+				SendWarning:  true,
-				}
+			}
 
-				_, err := NewNotificationFromDBModel(model)
+			Convey("Should send notification", func() {
-				So(err, ShouldNotBeNil)
+				So(dummieNotifier.ShouldDispath(result, notifier), ShouldBeTrue)
 			})
 
-			Convey("from settings", func() {
+			Convey("warn:false and state:warn should not send", func() {
-				json := `
+				result.State = alertstates.Warn
+				notifier.SendWarning = false
+				So(dummieNotifier.ShouldDispath(result, notifier), ShouldBeFalse)
+			})
+		})
+
+		Convey("Parsing alert notification from settings", func() {
+			Convey("Parsing email", func() {
+				Convey("empty settings should return error", func() {
+					json := `{ }`
+
+					settingsJSON, _ := simplejson.NewJson([]byte(json))
+					model := &m.AlertNotification{
+						Name:     "ops",
+						Type:     "email",
+						Settings: settingsJSON,
+					}
+
+					_, err := NewNotificationFromDBModel(model)
+					So(err, ShouldNotBeNil)
+				})
+
+				Convey("from settings", func() {
+					json := `
 				{
 					"to": "ops@grafana.org"
 				}`
 
-				settingsJSON, _ := simplejson.NewJson([]byte(json))
+					settingsJSON, _ := simplejson.NewJson([]byte(json))
-				model := &m.AlertNotification{
+					model := &m.AlertNotification{
-					Name:     "ops",
+						Name:     "ops",
-					Type:     "email",
+						Type:     "email",
-					Settings: settingsJSON,
+						Settings: settingsJSON,
-				}
+					}
 
-				not, err := NewNotificationFromDBModel(model)
+					not, err := NewNotificationFromDBModel(model)
 
-				So(err, ShouldBeNil)
+					So(err, ShouldBeNil)
-				So(not.Name, ShouldEqual, "ops")
+					So(not.Name, ShouldEqual, "ops")
-				So(not.Type, ShouldEqual, "email")
+					So(not.Type, ShouldEqual, "email")
-				So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.EmailNotifier")
+					So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.EmailNotifier")
 
-				email := not.Notifierr.(*EmailNotifier)
+					email := not.Notifierr.(*EmailNotifier)
-				So(email.To, ShouldEqual, "ops@grafana.org")
+					So(email.To, ShouldEqual, "ops@grafana.org")
-			})
+				})
-		})
-
-		Convey("Parsing webhook", func() {
-			Convey("empty settings should return error", func() {
-				json := `{ }`
-
-				settingsJSON, _ := simplejson.NewJson([]byte(json))
-				model := &m.AlertNotification{
-					Name:     "ops",
-					Type:     "webhook",
-					Settings: settingsJSON,
-				}
-
-				_, err := NewNotificationFromDBModel(model)
-				So(err, ShouldNotBeNil)
 			})
 
-			Convey("from settings", func() {
+			Convey("Parsing webhook", func() {
-				json := `
+				Convey("empty settings should return error", func() {
+					json := `{ }`
+
+					settingsJSON, _ := simplejson.NewJson([]byte(json))
+					model := &m.AlertNotification{
+						Name:     "ops",
+						Type:     "webhook",
+						Settings: settingsJSON,
+					}
+
+					_, err := NewNotificationFromDBModel(model)
+					So(err, ShouldNotBeNil)
+				})
+
+				Convey("from settings", func() {
+					json := `
 				{
 					"url": "http://localhost:3000",
 					"username": "username",
 					"password": "password"
 				}`
 
-				settingsJSON, _ := simplejson.NewJson([]byte(json))
+					settingsJSON, _ := simplejson.NewJson([]byte(json))
-				model := &m.AlertNotification{
+					model := &m.AlertNotification{
-					Name:     "slack",
+						Name:     "slack",
-					Type:     "webhook",
+						Type:     "webhook",
-					Settings: settingsJSON,
+						Settings: settingsJSON,
-				}
+					}
 
-				not, err := NewNotificationFromDBModel(model)
+					not, err := NewNotificationFromDBModel(model)
 
-				So(err, ShouldBeNil)
+					So(err, ShouldBeNil)
-				So(not.Name, ShouldEqual, "slack")
+					So(not.Name, ShouldEqual, "slack")
-				So(not.Type, ShouldEqual, "webhook")
+					So(not.Type, ShouldEqual, "webhook")
-				So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.WebhookNotifier")
+					So(reflect.TypeOf(not.Notifierr).Elem().String(), ShouldEqual, "alerting.WebhookNotifier")
 
-				webhook := not.Notifierr.(*WebhookNotifier)
+					webhook := not.Notifierr.(*WebhookNotifier)
-				So(webhook.Url, ShouldEqual, "http://localhost:3000")
+					So(webhook.Url, ShouldEqual, "http://localhost:3000")
+				})
 			})
 		})
-
 	})
 }