fix(notifier): incorrect sending alerts to subscriptions (#1109)

notifier/scheduler.go

@@ -50,10 +50,11 @@ func (scheduler *StandardScheduler) ScheduleNotification(params moira.SchedulerP
 		next      time.Time
 		throttled bool
 	)
+
 	now := scheduler.clock.NowUTC()
 	if params.SendFail > 0 {
 		next = now.Add(scheduler.config.ReschedulingDelay)
-		throttled = params.ThrottledOld
+		next, throttled = scheduler.calculateNextDelivery(next, &params.Event, logger)
 	} else {
 		if params.Event.State == moira.StateTEST {
 			next = now
@@ -62,6 +63,7 @@ func (scheduler *StandardScheduler) ScheduleNotification(params moira.SchedulerP
 			next, throttled = scheduler.calculateNextDelivery(now, &params.Event, logger)
 		}
 	}
+
 	notification := &moira.ScheduledNotification{
 		Event:     params.Event,
 		Trigger:   params.Trigger,
@@ -78,6 +80,7 @@ func (scheduler *StandardScheduler) ScheduleNotification(params moira.SchedulerP
 		Int64("notification_timestamp_unix", next.Unix()).
 		Int64("notification_created_at_unix", now.Unix()).
 		Msg("Scheduled notification")
+
 	return notification
 }
 

notifier/scheduler_test.go

@@ -1,6 +1,7 @@
 package notifier
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 	"time"
@@ -45,13 +46,23 @@ func TestThrottling(t *testing.T) {
 		SubscriptionID: &subID,
 	}
 
+	subscription := moira.SubscriptionData{
+		ID:                "SubscriptionID-000000000000001",
+		Enabled:           true,
+		Tags:              []string{"test-tag"},
+		Contacts:          []string{"ContactID-000000000000001"},
+		ThrottlingEnabled: true,
+		Schedule:          schedule5,
+	}
+
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 	dataBase := mock_moira_alert.NewMockDatabase(mockCtrl)
 	logger, _ := logging.GetLogger("Scheduler")
 	metrics2 := metrics.ConfigureNotifierMetrics(metrics.NewDummyRegistry(), "notifier")
 
 	now := time.Now()
+	next := now.Add(10 * time.Minute)
 	systemClock := mock_clock.NewMockClock(mockCtrl)
 	scheduler := NewScheduler(dataBase, logger, metrics2, SchedulerConfig{ReschedulingDelay: time.Minute}, systemClock)
 
@@ -84,6 +95,51 @@ func TestThrottling(t *testing.T) {
 		expected2.SendFail = 1
 		expected2.Timestamp = now.Add(time.Minute).Unix()
 		systemClock.EXPECT().NowUTC().Return(now).Times(1)
+		dataBase.EXPECT().GetTriggerThrottling(params2.Event.TriggerID).Return(now, now)
+		dataBase.EXPECT().GetSubscription(*params2.Event.SubscriptionID).Return(subscription, nil)
+		dataBase.EXPECT().GetNotificationEventCount(event.TriggerID, now.Unix()).Return(int64(0))
+		dataBase.EXPECT().GetNotificationEventCount(event.TriggerID, now.Unix()).Return(int64(0))
+
+		notification := scheduler.ScheduleNotification(params2, logger)
+		So(notification, ShouldResemble, &expected2)
+	})
+
+	Convey("Test sendFail more that 0, and no throttling, but subscription doesn't exists, should send message in one minute", t, func() {
+		params2 := params
+		params2.ThrottledOld = false
+		params2.SendFail = 1
+		testErr := errors.New("subscription doesn't exist")
+
+		expected2 := expected
+		expected2.SendFail = 1
+		expected2.Timestamp = now.Add(time.Minute).Unix()
+		systemClock.EXPECT().NowUTC().Return(now).Times(1)
+		dataBase.EXPECT().GetTriggerThrottling(params2.Event.TriggerID).Return(now, now)
+		dataBase.EXPECT().GetSubscription(*params2.Event.SubscriptionID).Return(moira.SubscriptionData{}, testErr)
+
+		notification := scheduler.ScheduleNotification(params2, logger)
+		So(notification, ShouldResemble, &expected2)
+	})
+
+	Convey("Test sendFail more that 0, and no throttling, but the subscription schedule postpones the dispatch time, should send message in one minute", t, func() {
+		params2 := params
+		params2.ThrottledOld = false
+		params2.SendFail = 1
+
+		// 2015-09-02, 01:00:00 GMT+03:00
+		testNow := time.Unix(1441144800, 0)
+		testSubscription := subscription
+		testSubscription.ThrottlingEnabled = false
+		testSubscription.Schedule = schedule3
+
+		expected2 := expected
+		expected2.SendFail = 1
+		// 2015-09-02, 02:00:00 GMT+03:00
+		expected2.Timestamp = time.Unix(1441148400, 0).Unix()
+		expected2.CreatedAt = testNow.Unix()
+		systemClock.EXPECT().NowUTC().Return(testNow).Times(1)
+		dataBase.EXPECT().GetTriggerThrottling(params2.Event.TriggerID).Return(testNow, testNow)
+		dataBase.EXPECT().GetSubscription(*params2.Event.SubscriptionID).Return(testSubscription, nil)
 
 		notification := scheduler.ScheduleNotification(params2, logger)
 		So(notification, ShouldResemble, &expected2)
@@ -96,9 +152,11 @@ func TestThrottling(t *testing.T) {
 
 		expected2 := expected
 		expected2.SendFail = 3
-		expected2.Timestamp = now.Add(time.Minute).Unix()
+		expected2.Timestamp = now.Add(10 * time.Minute).Unix()
 		expected2.Throttled = true
 		systemClock.EXPECT().NowUTC().Return(now).Times(1)
+		dataBase.EXPECT().GetTriggerThrottling(params2.Event.TriggerID).Return(next, now)
+		dataBase.EXPECT().GetSubscription(*params2.Event.SubscriptionID).Return(subscription, nil)
 
 		notification := scheduler.ScheduleNotification(params2, logger)
 		So(notification, ShouldResemble, &expected2)
@@ -504,3 +562,18 @@ var schedule4 = moira.ScheduleData{
 		{Enabled: true},
 	},
 }
+
+var schedule5 = moira.ScheduleData{
+	StartOffset:    0,    // 00:00
+	EndOffset:      1440, // 24:00
+	TimezoneOffset: -180, // (GMT +3)
+	Days: []moira.ScheduleDataDay{
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+		{Enabled: true},
+	},
+}