don't notify if notification state pending

If notification state is pending and last update of state was made
less than a minute ago. In the case of a grafana instance is shut down/crashes
between setting pending state and before sending the notification/marks as complete
this logic should allow the notification to be sent after some time instead of
being left in an inconsistent state where no notifications are being sent.

pkg/models/alert_notifications.go

@@ -93,6 +93,7 @@ type AlertNotificationState struct {
 	SentAt     int64
 	State      AlertNotificationStateType
 	Version    int64
+	UpdatedAt  int64
 }
 
 type SetAlertNotificationStateToPendingCommand struct {
@@ -110,4 +111,3 @@ type GetNotificationStateQuery struct {
 
 	Result *AlertNotificationState
 }
-

pkg/services/alerting/notifiers/base.go

@@ -54,7 +54,7 @@ func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequ
 	if context.PrevAlertState == context.Rule.State && sendReminder {
 		// Do not notify if interval has not elapsed
 		lastNotify := time.Unix(notificationState.SentAt, 0)
-		if !lastNotify.IsZero() && lastNotify.Add(frequency).After(time.Now()) {
+		if notificationState.SentAt != 0 && lastNotify.Add(frequency).After(time.Now()) {
 			return false
 		}
 
@@ -74,6 +74,14 @@ func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequ
 		return false
 	}
 
+	// Do not notifu if state pending and it have been updated last minute
+	if notificationState.State == models.AlertNotificationStatePending {
+		lastUpdated := time.Unix(notificationState.UpdatedAt, 0)
+		if lastUpdated.Add(1 * time.Minute).After(time.Now()) {
+			return false
+		}
+	}
+
 	return true
 }
 

pkg/services/alerting/notifiers/base_test.go

@@ -116,6 +116,22 @@ func TestShouldSendAlertNotification(t *testing.T) {
 			sendReminder: true,
 			state:        &m.AlertNotificationState{SentAt: tnow.Add(-11 * time.Minute).Unix()},
 
+			expect: true,
+		},
+		{
+			name:      "OK -> alerting with notifciation state pending and updated 30 seconds ago should not trigger",
+			newState:  m.AlertStateAlerting,
+			prevState: m.AlertStateOK,
+			state:     &m.AlertNotificationState{State: m.AlertNotificationStatePending, UpdatedAt: tnow.Add(-30 * time.Second).Unix()},
+
+			expect: false,
+		},
+		{
+			name:      "OK -> alerting with notifciation state pending and updated 2 minutes ago should trigger",
+			newState:  m.AlertStateAlerting,
+			prevState: m.AlertStateOK,
+			state:     &m.AlertNotificationState{State: m.AlertNotificationStatePending, UpdatedAt: tnow.Add(-2 * time.Minute).Unix()},
+
 			expect: true,
 		},
 	}

pkg/services/sqlstore/alert_notification.go

@@ -242,11 +242,12 @@ func SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *m.SetA
 		sql := `UPDATE alert_notification_state SET
 			state = ?,
 			version = ?,
-			sent_at = ?
+			sent_at = ?,
+			updated_at = ?
 		WHERE
 			id = ?`
 
-		_, err := sess.Exec(sql, cmd.State.State, cmd.State.Version, cmd.State.SentAt, cmd.State.Id)
+		_, err := sess.Exec(sql, cmd.State.State, cmd.State.Version, cmd.State.SentAt, timeNow().Unix(), cmd.State.Id)
 
 		if err != nil {
 			return err
@@ -268,12 +269,13 @@ func SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *m.SetAl
 
 		sql := `UPDATE alert_notification_state SET
 			state = ?,
-			version = ?
+			version = ?,
+			updated_at = ?
 		WHERE
 			id = ? AND
 			version = ?`
 
-		res, err := sess.Exec(sql, cmd.State.State, cmd.State.Version, cmd.State.Id, currentVersion)
+		res, err := sess.Exec(sql, cmd.State.State, cmd.State.Version, timeNow().Unix(), cmd.State.Id, currentVersion)
 
 		if err != nil {
 			return err
@@ -310,6 +312,7 @@ func GetAlertNotificationState(ctx context.Context, cmd *m.GetNotificationStateQ
 			AlertId:    cmd.AlertId,
 			NotifierId: cmd.NotifierId,
 			State:      "unknown",
+			UpdatedAt:  timeNow().Unix(),
 		}
 
 		if _, err := sess.Insert(notificationState); err != nil {
@@ -337,7 +340,7 @@ func GetAlertNotificationState(ctx context.Context, cmd *m.GetNotificationStateQ
 }
 
 func getAlertNotificationState(sess *DBSession, cmd *m.GetNotificationStateQuery, nj *m.AlertNotificationState) (bool, error) {
-	exist, err := sess.Desc("alert_notification_state.sent_at").
+	exist, err := sess.
 		Where("alert_notification_state.org_id = ?", cmd.OrgId).
 		Where("alert_notification_state.alert_id = ?", cmd.AlertId).
 		Where("alert_notification_state.notifier_id = ?", cmd.NotifierId).

pkg/services/sqlstore/alert_notification_test.go

@@ -18,6 +18,9 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 			var alertID int64 = 7
 			var orgID int64 = 5
 			var notifierID int64 = 10
+			oldTimeNow := timeNow
+			now := time.Date(2018, 9, 30, 0, 0, 0, 0, time.UTC)
+			timeNow = func() time.Time { return now }
 
 			Convey("Get no existing state should create a new state", func() {
 				query := &models.GetNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID}
@@ -26,6 +29,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 				So(query.Result, ShouldNotBeNil)
 				So(query.Result.State, ShouldEqual, "unknown")
 				So(query.Result.Version, ShouldEqual, 0)
+				So(query.Result.UpdatedAt, ShouldEqual, now.Unix())
 
 				Convey("Get existing state should not create a new state", func() {
 					query2 := &models.GetNotificationStateQuery{AlertId: alertID, OrgId: orgID, NotifierId: notifierID}
@@ -33,6 +37,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(query2.Result, ShouldNotBeNil)
 					So(query2.Result.Id, ShouldEqual, query.Result.Id)
+					So(query2.Result.UpdatedAt, ShouldEqual, now.Unix())
 				})
 
 				Convey("Update existing state to pending with correct version should update database", func() {
@@ -50,6 +55,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 					So(err, ShouldBeNil)
 					So(query2.Result.Version, ShouldEqual, 1)
 					So(query2.Result.State, ShouldEqual, models.AlertNotificationStatePending)
+					So(query2.Result.UpdatedAt, ShouldEqual, now.Unix())
 
 					Convey("Update existing state to completed should update database", func() {
 						s := *cmd.State
@@ -64,6 +70,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 						So(err, ShouldBeNil)
 						So(query3.Result.Version, ShouldEqual, 2)
 						So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted)
+						So(query3.Result.UpdatedAt, ShouldEqual, now.Unix())
 					})
 
 					Convey("Update existing state to completed should update database, but return version mismatch", func() {
@@ -80,6 +87,7 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 						So(err, ShouldBeNil)
 						So(query3.Result.Version, ShouldEqual, 1001)
 						So(query3.Result.State, ShouldEqual, models.AlertNotificationStateCompleted)
+						So(query3.Result.UpdatedAt, ShouldEqual, now.Unix())
 					})
 				})
 
@@ -93,6 +101,10 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
 					So(err, ShouldEqual, models.ErrAlertNotificationStateVersionConflict)
 				})
 			})
+
+			Reset(func() {
+				timeNow = oldTimeNow
+			})
 		})
 
 		Convey("Alert notifications should be empty", func() {

pkg/services/sqlstore/migrations/alert_mig.go

@@ -120,6 +120,7 @@ func addAlertMigrations(mg *Migrator) {
 			{Name: "sent_at", Type: DB_BigInt, Nullable: false},
 			{Name: "state", Type: DB_NVarchar, Length: 50, Nullable: false},
 			{Name: "version", Type: DB_BigInt, Nullable: false},
+			{Name: "updated_at", Type: DB_BigInt, Nullable: false},
 		},
 		Indices: []*Index{
 			{Cols: []string{"org_id", "alert_id", "notifier_id"}, Type: UniqueIndex},