mirror of
https://github.com/grafana/grafana.git
synced 2025-02-25 18:55:37 -06:00
alerting: move all notification conditions to defaultShouldNotify
This commit is contained in:
parent
8b5aefae26
commit
b04052f515
@ -98,7 +98,7 @@ type GetLatestNotificationQuery struct {
|
||||
AlertId int64
|
||||
NotifierId int64
|
||||
|
||||
Result *AlertNotificationJournal
|
||||
Result []AlertNotificationJournal
|
||||
}
|
||||
|
||||
type CleanNotificationJournalCommand struct {
|
||||
|
@ -68,7 +68,7 @@ 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(context.Background(), evalContext) {
|
||||
if !evalContext.IsTestRun && !not.ShouldNotify(ctx, evalContext) {
|
||||
return nil
|
||||
}
|
||||
|
||||
|
@ -42,12 +42,21 @@ func NewNotifierBase(model *models.AlertNotification) NotifierBase {
|
||||
}
|
||||
}
|
||||
|
||||
func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequency time.Duration, lastNotify time.Time) bool {
|
||||
func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequency time.Duration, journals []models.AlertNotificationJournal) 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
|
||||
}
|
||||
}
|
||||
|
||||
// Do not notify if interval has not elapsed
|
||||
if sendReminder && !lastNotify.IsZero() && lastNotify.Add(frequency).After(time.Now()) {
|
||||
return false
|
||||
@ -75,20 +84,12 @@ func (n *NotifierBase) ShouldNotify(ctx context.Context, c *alerting.EvalContext
|
||||
}
|
||||
|
||||
err := bus.DispatchCtx(ctx, cmd)
|
||||
if err == models.ErrJournalingNotFound {
|
||||
return true
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
n.log.Error("Could not determine last time alert notifier fired", "Alert name", c.Rule.Name, "Error", err)
|
||||
return false
|
||||
}
|
||||
|
||||
if !cmd.Result.Success {
|
||||
return true
|
||||
}
|
||||
|
||||
return defaultShouldNotify(c, n.SendReminder, n.Frequency, time.Unix(cmd.Result.SentAt, 0))
|
||||
return defaultShouldNotify(c, n.SendReminder, n.Frequency, cmd.Result)
|
||||
}
|
||||
|
||||
func (n *NotifierBase) GetType() string {
|
||||
|
@ -15,51 +15,105 @@ import (
|
||||
)
|
||||
|
||||
func TestShouldSendAlertNotification(t *testing.T) {
|
||||
tnow := time.Now()
|
||||
|
||||
tcs := []struct {
|
||||
name string
|
||||
prevState m.AlertStateType
|
||||
newState m.AlertStateType
|
||||
expected bool
|
||||
sendReminder bool
|
||||
frequency time.Duration
|
||||
journals []m.AlertNotificationJournal
|
||||
|
||||
expect bool
|
||||
}{
|
||||
{
|
||||
name: "pending -> ok should not trigger an notification",
|
||||
newState: m.AlertStatePending,
|
||||
prevState: m.AlertStateOK,
|
||||
expected: false,
|
||||
name: "pending -> ok should not trigger an notification",
|
||||
newState: m.AlertStatePending,
|
||||
prevState: m.AlertStateOK,
|
||||
sendReminder: false,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: false,
|
||||
},
|
||||
{
|
||||
name: "ok -> alerting should trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStateAlerting,
|
||||
expected: true,
|
||||
name: "ok -> alerting should trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStateAlerting,
|
||||
sendReminder: false,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: true,
|
||||
},
|
||||
{
|
||||
name: "ok -> pending should not trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStatePending,
|
||||
expected: false,
|
||||
name: "ok -> pending should not trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStatePending,
|
||||
sendReminder: false,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: false,
|
||||
},
|
||||
{
|
||||
name: "ok -> ok should not trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStateOK,
|
||||
expected: false,
|
||||
sendReminder: false,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: false,
|
||||
},
|
||||
{
|
||||
name: "ok -> alerting should not trigger an notification",
|
||||
name: "ok -> alerting should trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStateAlerting,
|
||||
expected: true,
|
||||
sendReminder: true,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: true,
|
||||
},
|
||||
{
|
||||
name: "ok -> ok with reminder should not trigger an notification",
|
||||
newState: m.AlertStateOK,
|
||||
prevState: m.AlertStateOK,
|
||||
expected: false,
|
||||
sendReminder: true,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: false,
|
||||
},
|
||||
{
|
||||
name: "alerting -> alerting with reminder and no journaling should trigger",
|
||||
newState: m.AlertStateAlerting,
|
||||
prevState: m.AlertStateAlerting,
|
||||
frequency: time.Minute * 10,
|
||||
sendReminder: true,
|
||||
journals: []m.AlertNotificationJournal{},
|
||||
|
||||
expect: true,
|
||||
},
|
||||
{
|
||||
name: "alerting -> alerting with reminder and successful recent journal event should not trigger",
|
||||
newState: m.AlertStateAlerting,
|
||||
prevState: m.AlertStateAlerting,
|
||||
frequency: time.Minute * 10,
|
||||
sendReminder: true,
|
||||
journals: []m.AlertNotificationJournal{
|
||||
{SentAt: tnow.Add(-time.Minute).Unix(), Success: true},
|
||||
},
|
||||
|
||||
expect: false,
|
||||
},
|
||||
{
|
||||
name: "alerting -> alerting with reminder and failed recent journal event should trigger",
|
||||
newState: m.AlertStateAlerting,
|
||||
prevState: m.AlertStateAlerting,
|
||||
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
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
@ -69,8 +123,8 @@ func TestShouldSendAlertNotification(t *testing.T) {
|
||||
})
|
||||
|
||||
evalContext.Rule.State = tc.prevState
|
||||
if defaultShouldNotify(evalContext, true, 0, time.Now()) != tc.expected {
|
||||
t.Errorf("failed %s. expected %+v to return %v", tc.name, tc, tc.expected)
|
||||
if defaultShouldNotify(evalContext, true, tc.frequency, tc.journals) != tc.expect {
|
||||
t.Errorf("failed test %s.\n expected \n%+v \nto return: %v", tc.name, tc, tc.expect)
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -87,16 +141,6 @@ func TestShouldNotifyWhenNoJournalingIsFound(t *testing.T) {
|
||||
})
|
||||
evalContext := alerting.NewEvalContext(context.TODO(), &alerting.Rule{})
|
||||
|
||||
Convey("should notify if no journaling is found", func() {
|
||||
bus.AddHandlerCtx("", func(ctx context.Context, q *m.GetLatestNotificationQuery) error {
|
||||
return m.ErrJournalingNotFound
|
||||
})
|
||||
|
||||
if !notifier.ShouldNotify(context.Background(), evalContext) {
|
||||
t.Errorf("should send notifications when ErrJournalingNotFound is returned")
|
||||
}
|
||||
})
|
||||
|
||||
Convey("should not notify query returns error", func() {
|
||||
bus.AddHandlerCtx("", func(ctx context.Context, q *m.GetLatestNotificationQuery) error {
|
||||
return errors.New("some kind of error unknown error")
|
||||
|
@ -230,7 +230,7 @@ func UpdateAlertNotification(cmd *m.UpdateAlertNotificationCommand) error {
|
||||
}
|
||||
|
||||
func RecordNotificationJournal(ctx context.Context, cmd *m.RecordNotificationJournalCommand) error {
|
||||
return inTransactionCtx(ctx, func(sess *DBSession) error {
|
||||
return withDbSession(ctx, func(sess *DBSession) error {
|
||||
journalEntry := &m.AlertNotificationJournal{
|
||||
OrgId: cmd.OrgId,
|
||||
AlertId: cmd.AlertId,
|
||||
@ -245,21 +245,19 @@ func RecordNotificationJournal(ctx context.Context, cmd *m.RecordNotificationJou
|
||||
}
|
||||
|
||||
func GetLatestNotification(ctx context.Context, cmd *m.GetLatestNotificationQuery) error {
|
||||
return inTransactionCtx(ctx, func(sess *DBSession) error {
|
||||
nj := &m.AlertNotificationJournal{}
|
||||
return withDbSession(ctx, func(sess *DBSession) error {
|
||||
nj := []m.AlertNotificationJournal{}
|
||||
|
||||
_, err := sess.Desc("alert_notification_journal.sent_at").
|
||||
Limit(1).
|
||||
Where("alert_notification_journal.org_id = ? AND alert_notification_journal.alert_id = ? AND alert_notification_journal.notifier_id = ?", cmd.OrgId, cmd.AlertId, cmd.NotifierId).Get(nj)
|
||||
err := sess.Desc("alert_notification_journal.sent_at").
|
||||
Where("alert_notification_journal.org_id = ?", cmd.OrgId).
|
||||
Where("alert_notification_journal.alert_id = ?", cmd.AlertId).
|
||||
Where("alert_notification_journal.notifier_id = ?", cmd.NotifierId).
|
||||
Find(&nj)
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if nj.AlertId == 0 && nj.Id == 0 && nj.NotifierId == 0 && nj.OrgId == 0 {
|
||||
return m.ErrJournalingNotFound
|
||||
}
|
||||
|
||||
cmd.Result = nj
|
||||
return nil
|
||||
})
|
||||
|
@ -15,16 +15,21 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
|
||||
InitTestDB(t)
|
||||
|
||||
Convey("Alert notification journal", func() {
|
||||
var alertId int64 = 5
|
||||
var alertId int64 = 7
|
||||
var orgId int64 = 5
|
||||
var notifierId int64 = 5
|
||||
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}
|
||||
err := GetLatestNotification(context.Background(), query)
|
||||
So(err, ShouldEqual, m.ErrJournalingNotFound)
|
||||
GetLatestNotification(context.Background(), query)
|
||||
So(len(query.Result), ShouldEqual, 0)
|
||||
|
||||
Convey("shoulbe be able to record two journaling events", func() {
|
||||
// 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)
|
||||
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}
|
||||
|
||||
err := RecordNotificationJournal(context.Background(), createCmd)
|
||||
@ -38,17 +43,20 @@ func TestAlertNotificationSQLAccess(t *testing.T) {
|
||||
Convey("get last journaling event", func() {
|
||||
err := GetLatestNotification(context.Background(), query)
|
||||
So(err, ShouldBeNil)
|
||||
So(query.Result.SentAt, ShouldEqual, 1001)
|
||||
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 junaling should raise error", func() {
|
||||
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, ShouldEqual, m.ErrJournalingNotFound)
|
||||
So(err, ShouldBeNil)
|
||||
So(len(query.Result), ShouldEqual, 0)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
Loading…
Reference in New Issue
Block a user