diff --git a/pkg/models/alert_notifications.go b/pkg/models/alert_notifications.go index b90b3d36ced..419f5048af4 100644 --- a/pkg/models/alert_notifications.go +++ b/pkg/models/alert_notifications.go @@ -76,33 +76,36 @@ type GetAllAlertNotificationsQuery struct { Result []*AlertNotification } -type AlertNotificationJournal struct { +type AlertNotificationState struct { Id int64 OrgId int64 AlertId int64 NotifierId int64 SentAt int64 - Success bool + State string + Version int64 } -type RecordNotificationJournalCommand struct { +type UpdateAlertNotificationStateCommand struct { OrgId int64 AlertId int64 NotifierId int64 SentAt int64 - Success bool + State bool } -type GetLatestNotificationQuery struct { +type GetNotificationStateQuery struct { OrgId int64 AlertId int64 NotifierId int64 - Result []AlertNotificationJournal + Result *AlertNotificationState } -type CleanNotificationJournalCommand struct { +type InsertAlertNotificationCommand struct { OrgId int64 AlertId int64 NotifierId int64 + SentAt int64 + State string } diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index cbad5cbfdcf..ec448a8c1b0 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -4,12 +4,10 @@ import ( "context" "errors" "fmt" - "time" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/imguploader" "github.com/grafana/grafana/pkg/log" - "github.com/grafana/grafana/pkg/metrics" "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/setting" @@ -68,30 +66,31 @@ func (n *notificationService) sendNotifications(evalContext *EvalContext, notifi // Verify that we can send the notification again // but this time within the same transaction. - if !evalContext.IsTestRun && !not.ShouldNotify(ctx, evalContext) { - return nil - } + // if !evalContext.IsTestRun && !not.ShouldNotify(ctx, evalContext) { + // return nil + // } - n.log.Debug("Sending notification", "type", not.GetType(), "id", not.GetNotifierId(), "isDefault", not.GetIsDefault()) - metrics.M_Alerting_Notification_Sent.WithLabelValues(not.GetType()).Inc() + // n.log.Debug("Sending notification", "type", not.GetType(), "id", not.GetNotifierId(), "isDefault", not.GetIsDefault()) + // metrics.M_Alerting_Notification_Sent.WithLabelValues(not.GetType()).Inc() - //send notification - success := not.Notify(evalContext) == nil + // //send notification + // // success := not.Notify(evalContext) == nil - if evalContext.IsTestRun { - return nil - } + // if evalContext.IsTestRun { + // return nil + // } //write result to db. - cmd := &m.RecordNotificationJournalCommand{ - OrgId: evalContext.Rule.OrgId, - AlertId: evalContext.Rule.Id, - NotifierId: not.GetNotifierId(), - SentAt: time.Now().Unix(), - Success: success, - } + // cmd := &m.RecordNotificationJournalCommand{ + // OrgId: evalContext.Rule.OrgId, + // AlertId: evalContext.Rule.Id, + // NotifierId: not.GetNotifierId(), + // SentAt: time.Now().Unix(), + // Success: success, + // } - return bus.DispatchCtx(ctx, cmd) + // return bus.DispatchCtx(ctx, cmd) + return nil }) if err != nil { diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 24daa02bce8..f71a41235f7 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -42,22 +42,23 @@ func NewNotifierBase(model *models.AlertNotification) NotifierBase { } } -func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequency time.Duration, journals []models.AlertNotificationJournal) bool { +func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequency time.Duration, notificationState *models.AlertNotificationState) bool { // Only notify on state change. if context.PrevAlertState == context.Rule.State && !sendReminder { return false } // get last successfully sent notification - lastNotify := time.Time{} - for _, j := range journals { - if j.Success { - lastNotify = time.Unix(j.SentAt, 0) - break - } - } + // lastNotify := time.Time{} + // for _, j := range journals { + // if j.Success { + // lastNotify = time.Unix(j.SentAt, 0) + // break + // } + // } // Do not notify if interval has not elapsed + lastNotify := time.Unix(notificationState.SentAt, 0) if sendReminder && !lastNotify.IsZero() && lastNotify.Add(frequency).After(time.Now()) { return false } @@ -77,7 +78,7 @@ func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequ // ShouldNotify checks this evaluation should send an alert notification func (n *NotifierBase) ShouldNotify(ctx context.Context, c *alerting.EvalContext) bool { - cmd := &models.GetLatestNotificationQuery{ + cmd := &models.GetNotificationStateQuery{ OrgId: c.Rule.OrgId, AlertId: c.Rule.Id, NotifierId: n.Id, diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index 9ea4b82fd54..c14006637a4 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -23,7 +23,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState m.AlertStateType sendReminder bool frequency time.Duration - journals []m.AlertNotificationJournal + journals *m.AlertNotificationState expect bool }{ @@ -32,7 +32,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState: m.AlertStatePending, prevState: m.AlertStateOK, sendReminder: false, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: false, }, @@ -41,7 +41,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState: m.AlertStateOK, prevState: m.AlertStateAlerting, sendReminder: false, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: true, }, @@ -50,7 +50,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState: m.AlertStateOK, prevState: m.AlertStatePending, sendReminder: false, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: false, }, @@ -59,7 +59,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState: m.AlertStateOK, prevState: m.AlertStateOK, sendReminder: false, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: false, }, @@ -68,7 +68,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState: m.AlertStateOK, prevState: m.AlertStateAlerting, sendReminder: true, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: true, }, @@ -77,7 +77,7 @@ func TestShouldSendAlertNotification(t *testing.T) { newState: m.AlertStateOK, prevState: m.AlertStateOK, sendReminder: true, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: false, }, @@ -87,7 +87,7 @@ func TestShouldSendAlertNotification(t *testing.T) { prevState: m.AlertStateAlerting, frequency: time.Minute * 10, sendReminder: true, - journals: []m.AlertNotificationJournal{}, + journals: &m.AlertNotificationState{}, expect: true, }, @@ -97,9 +97,7 @@ func TestShouldSendAlertNotification(t *testing.T) { prevState: m.AlertStateAlerting, frequency: time.Minute * 10, sendReminder: true, - journals: []m.AlertNotificationJournal{ - {SentAt: tnow.Add(-time.Minute).Unix(), Success: true}, - }, + journals: &m.AlertNotificationState{SentAt: tnow.Add(-time.Minute).Unix()}, expect: false, }, @@ -110,10 +108,7 @@ func TestShouldSendAlertNotification(t *testing.T) { frequency: time.Minute * 10, sendReminder: true, expect: true, - journals: []m.AlertNotificationJournal{ - {SentAt: tnow.Add(-time.Minute).Unix(), Success: false}, // recent failed notification - {SentAt: tnow.Add(-time.Hour).Unix(), Success: true}, // old successful notification - }, + journals: &m.AlertNotificationState{SentAt: tnow.Add(-time.Hour).Unix()}, }, } @@ -142,7 +137,7 @@ func TestShouldNotifyWhenNoJournalingIsFound(t *testing.T) { evalContext := alerting.NewEvalContext(context.TODO(), &alerting.Rule{}) Convey("should not notify query returns error", func() { - bus.AddHandlerCtx("", func(ctx context.Context, q *m.GetLatestNotificationQuery) error { + bus.AddHandlerCtx("", func(ctx context.Context, q *m.GetNotificationStateQuery) error { return errors.New("some kind of error unknown error") }) diff --git a/pkg/services/alerting/result_handler.go b/pkg/services/alerting/result_handler.go index 893cca948f9..e2c70de0e28 100644 --- a/pkg/services/alerting/result_handler.go +++ b/pkg/services/alerting/result_handler.go @@ -88,19 +88,6 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error { } } - if evalContext.Rule.State == m.AlertStateOK && evalContext.PrevAlertState != m.AlertStateOK { - for _, notifierId := range evalContext.Rule.Notifications { - cmd := &m.CleanNotificationJournalCommand{ - AlertId: evalContext.Rule.Id, - NotifierId: notifierId, - OrgId: evalContext.Rule.OrgId, - } - if err := bus.DispatchCtx(evalContext.Ctx, cmd); err != nil { - handler.log.Error("Failed to clean up old notification records", "notifier", notifierId, "alert", evalContext.Rule.Id, "Error", err) - } - } - } - handler.notifier.SendIfNeeded(evalContext) return nil } diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index df247e6891d..ece06614d4a 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -20,7 +20,6 @@ func init() { bus.AddHandler("sql", GetAllAlertNotifications) bus.AddHandlerCtx("sql", RecordNotificationJournal) bus.AddHandlerCtx("sql", GetLatestNotification) - bus.AddHandlerCtx("sql", CleanNotificationJournal) } func DeleteAlertNotification(cmd *m.DeleteAlertNotificationCommand) error { @@ -229,14 +228,13 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error { }) } -func RecordNotificationJournal(ctx context.Context, cmd *m.RecordNotificationJournalCommand) error { +func RecordNotificationJournal(ctx context.Context, cmd *m.UpdateAlertNotificationStateCommand) error { return withDbSession(ctx, func(sess *DBSession) error { - journalEntry := &m.AlertNotificationJournal{ + journalEntry := &m.AlertNotificationState{ OrgId: cmd.OrgId, AlertId: cmd.AlertId, NotifierId: cmd.NotifierId, SentAt: cmd.SentAt, - Success: cmd.Success, } _, err := sess.Insert(journalEntry) @@ -244,9 +242,9 @@ func RecordNotificationJournal(ctx context.Context, cmd *m.RecordNotificationJou }) } -func GetLatestNotification(ctx context.Context, cmd *m.GetLatestNotificationQuery) error { +func GetLatestNotification(ctx context.Context, cmd *m.GetNotificationStateQuery) error { return withDbSession(ctx, func(sess *DBSession) error { - nj := []m.AlertNotificationJournal{} + nj := &m.AlertNotificationState{} err := sess.Desc("alert_notification_journal.sent_at"). Where("alert_notification_journal.org_id = ?", cmd.OrgId). @@ -262,11 +260,3 @@ func GetLatestNotification(ctx context.Context, cmd *m.GetLatestNotificationQuer return nil }) } - -func CleanNotificationJournal(ctx context.Context, cmd *m.CleanNotificationJournalCommand) error { - return inTransactionCtx(ctx, func(sess *DBSession) error { - sql := "DELETE FROM alert_notification_journal WHERE alert_notification_journal.org_id = ? AND alert_notification_journal.alert_id = ? AND alert_notification_journal.notifier_id = ?" - _, err := sess.Exec(sql, cmd.OrgId, cmd.AlertId, cmd.NotifierId) - return err - }) -} diff --git a/pkg/services/sqlstore/alert_notification_test.go b/pkg/services/sqlstore/alert_notification_test.go index 1e3df45b5cf..742d459e1ca 100644 --- a/pkg/services/sqlstore/alert_notification_test.go +++ b/pkg/services/sqlstore/alert_notification_test.go @@ -20,17 +20,17 @@ func TestAlertNotificationSQLAccess(t *testing.T) { var notifierId int64 = 10 Convey("Getting last journal should raise error if no one exists", func() { - query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} - GetLatestNotification(context.Background(), query) - So(len(query.Result), ShouldEqual, 0) + query := &m.GetNotificationStateQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} + err := GetLatestNotification(context.Background(), query) + So(err, ShouldNotBeNil) // recording an journal entry in another org to make sure org filter works as expected. - journalInOtherOrg := &m.RecordNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: 10, Success: true, SentAt: 1} - err := RecordNotificationJournal(context.Background(), journalInOtherOrg) + journalInOtherOrg := &m.UpdateAlertNotificationStateCommand{AlertId: alertId, NotifierId: notifierId, OrgId: 10, SentAt: 1} + err = RecordNotificationJournal(context.Background(), journalInOtherOrg) So(err, ShouldBeNil) Convey("should be able to record two journaling events", func() { - createCmd := &m.RecordNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId, Success: true, SentAt: 1} + createCmd := &m.UpdateAlertNotificationStateCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId, SentAt: 1} err := RecordNotificationJournal(context.Background(), createCmd) So(err, ShouldBeNil) @@ -39,27 +39,6 @@ func TestAlertNotificationSQLAccess(t *testing.T) { err = RecordNotificationJournal(context.Background(), createCmd) So(err, ShouldBeNil) - - Convey("get last journaling event", func() { - err := GetLatestNotification(context.Background(), query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 2) - last := query.Result[0] - So(last.SentAt, ShouldEqual, 1001) - - Convey("be able to clear all journaling for an notifier", func() { - cmd := &m.CleanNotificationJournalCommand{AlertId: alertId, NotifierId: notifierId, OrgId: orgId} - err := CleanNotificationJournal(context.Background(), cmd) - So(err, ShouldBeNil) - - Convey("querying for last journaling should return no journal entries", func() { - query := &m.GetLatestNotificationQuery{AlertId: alertId, OrgId: orgId, NotifierId: notifierId} - err := GetLatestNotification(context.Background(), query) - So(err, ShouldBeNil) - So(len(query.Result), ShouldEqual, 0) - }) - }) - }) }) }) }) diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index e27e64c6124..d6886f6ecae 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -107,4 +107,25 @@ func addAlertMigrations(mg *Migrator) { mg.AddMigration("create notification_journal table v1", NewAddTableMigration(notification_journal)) mg.AddMigration("add index notification_journal org_id & alert_id & notifier_id", NewAddIndexMigration(notification_journal, notification_journal.Indices[0])) + + mg.AddMigration("drop alert_notification_journal", NewDropTableMigration("alert_notification_journal")) + + alert_notification_state := Table{ + Name: "alert_notification_state", + Columns: []*Column{ + {Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true}, + {Name: "org_id", Type: DB_BigInt, Nullable: false}, + {Name: "alert_id", Type: DB_BigInt, Nullable: false}, + {Name: "notifier_id", Type: DB_BigInt, Nullable: false}, + {Name: "sent_at", Type: DB_BigInt, Nullable: false}, + {Name: "state", Type: DB_NVarchar, Length: 50, Nullable: false}, + {Name: "version", Type: DB_BigInt, Nullable: false}, + }, + Indices: []*Index{ + {Cols: []string{"org_id", "alert_id", "notifier_id"}, Type: IndexType}, + }, + } + + mg.AddMigration("create alert_notification_state table v1", NewAddTableMigration(alert_notification_state)) + mg.AddMigration("add index alert_notification_state org_id & alert_id & notifier_id", NewAddIndexMigration(alert_notification_state, notification_journal.Indices[0])) }