From 396f8e6464a38e6ac925ecb8465a71e5118b79c5 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 29 Jun 2018 15:15:31 +0200 Subject: [PATCH] notifications: read without tran, write with tran --- pkg/services/alerting/interfaces.go | 7 +++++-- pkg/services/alerting/notifier.go | 6 +++--- pkg/services/alerting/notifiers/alertmanager.go | 3 ++- pkg/services/alerting/notifiers/base.go | 5 +++-- pkg/services/alerting/notifiers/base_test.go | 4 ++-- pkg/services/sqlstore/alert_notification.go | 1 + 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/services/alerting/interfaces.go b/pkg/services/alerting/interfaces.go index b4376191df0..46f8b3c769c 100644 --- a/pkg/services/alerting/interfaces.go +++ b/pkg/services/alerting/interfaces.go @@ -1,6 +1,9 @@ package alerting -import "time" +import ( + "context" + "time" +) type EvalHandler interface { Eval(evalContext *EvalContext) @@ -17,7 +20,7 @@ type Notifier interface { NeedsImage() bool // ShouldNotify checks this evaluation should send an alert notification - ShouldNotify(evalContext *EvalContext) bool + ShouldNotify(ctx context.Context, evalContext *EvalContext) bool GetNotifierId() int64 GetIsDefault() bool diff --git a/pkg/services/alerting/notifier.go b/pkg/services/alerting/notifier.go index 61526ed642c..a19e44a8f99 100644 --- a/pkg/services/alerting/notifier.go +++ b/pkg/services/alerting/notifier.go @@ -72,7 +72,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(evalContext) { + if !evalContext.IsTestRun && !not.ShouldNotify(context.Background(), evalContext) { return nil } @@ -91,7 +91,7 @@ func (n *notificationService) sendNotifications(evalContext *EvalContext, notifi Success: success, } - return bus.DispatchCtx(evalContext.Ctx, cmd) + return bus.DispatchCtx(ctx, cmd) }) }) } @@ -149,7 +149,7 @@ func (n *notificationService) getNeededNotifiers(orgId int64, notificationIds [] return nil, err } - if not.ShouldNotify(evalContext) { + if not.ShouldNotify(evalContext.Ctx, evalContext) { result = append(result, not) } } diff --git a/pkg/services/alerting/notifiers/alertmanager.go b/pkg/services/alerting/notifiers/alertmanager.go index 42ffa9b2d6e..9826dd1dffb 100644 --- a/pkg/services/alerting/notifiers/alertmanager.go +++ b/pkg/services/alerting/notifiers/alertmanager.go @@ -1,6 +1,7 @@ package notifiers import ( + "context" "time" "github.com/grafana/grafana/pkg/bus" @@ -45,7 +46,7 @@ type AlertmanagerNotifier struct { log log.Logger } -func (this *AlertmanagerNotifier) ShouldNotify(evalContext *alerting.EvalContext) bool { +func (this *AlertmanagerNotifier) ShouldNotify(ctx context.Context, evalContext *alerting.EvalContext) bool { this.log.Debug("Should notify", "ruleId", evalContext.Rule.Id, "state", evalContext.Rule.State, "previousState", evalContext.PrevAlertState) // Do not notify when we become OK for the first time. diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 31ec77cbf25..ca011356247 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -1,6 +1,7 @@ package notifiers import ( + "context" "time" "github.com/grafana/grafana/pkg/bus" @@ -66,14 +67,14 @@ func defaultShouldNotify(context *alerting.EvalContext, sendReminder bool, frequ } // ShouldNotify checks this evaluation should send an alert notification -func (n *NotifierBase) ShouldNotify(c *alerting.EvalContext) bool { +func (n *NotifierBase) ShouldNotify(ctx context.Context, c *alerting.EvalContext) bool { cmd := &models.GetLatestNotificationQuery{ OrgId: c.Rule.OrgId, AlertId: c.Rule.Id, NotifierId: n.Id, } - err := bus.DispatchCtx(c.Ctx, cmd) + err := bus.DispatchCtx(ctx, cmd) if err == models.ErrJournalingNotFound { return true } diff --git a/pkg/services/alerting/notifiers/base_test.go b/pkg/services/alerting/notifiers/base_test.go index 3fd23b69c6e..57b82f32466 100644 --- a/pkg/services/alerting/notifiers/base_test.go +++ b/pkg/services/alerting/notifiers/base_test.go @@ -92,7 +92,7 @@ func TestShouldNotifyWhenNoJournalingIsFound(t *testing.T) { return m.ErrJournalingNotFound }) - if !notifier.ShouldNotify(evalContext) { + if !notifier.ShouldNotify(context.Background(), evalContext) { t.Errorf("should send notifications when ErrJournalingNotFound is returned") } }) @@ -102,7 +102,7 @@ func TestShouldNotifyWhenNoJournalingIsFound(t *testing.T) { return errors.New("some kind of error unknown error") }) - if notifier.ShouldNotify(evalContext) { + if notifier.ShouldNotify(context.Background(), evalContext) { t.Errorf("should not send notifications when query returns error") } }) diff --git a/pkg/services/sqlstore/alert_notification.go b/pkg/services/sqlstore/alert_notification.go index 3f2ca109c1a..8fb1e2212a9 100644 --- a/pkg/services/sqlstore/alert_notification.go +++ b/pkg/services/sqlstore/alert_notification.go @@ -250,6 +250,7 @@ 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{} + _, 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)