From 78b22935060c9a66ab904b8839becfd7a6f820b7 Mon Sep 17 00:00:00 2001 From: Danil Tarasov <87192879+almostinf@users.noreply.github.com> Date: Mon, 2 Sep 2024 11:40:07 +0300 Subject: [PATCH] fix(checker): fix skip first metric value (#1007) --- checker/check.go | 35 +++--- checker/check_test.go | 179 ++++++++++++++++++---------- checker/trigger_checker.go | 12 +- checker/trigger_checker_test.go | 25 ++-- clock/clock.go | 9 +- database/redis/last_check_test.go | 14 +++ database/redis/metric.go | 2 +- database/redis/metric_test.go | 4 +- database/redis/notification_test.go | 6 +- database/redis/reply/check.go | 2 + database/redis/trigger.go | 2 +- database/redis/trigger_test.go | 8 +- datatypes.go | 41 ++++--- datatypes_test.go | 173 ++++++++++++++++++++------- filter/patterns_storage.go | 2 +- filter/patterns_storage_test.go | 2 +- go.mod | 1 + go.sum | 1 + interfaces.go | 3 +- mock/clock/clock.go | 26 +++- notifier/events/event_test.go | 8 +- notifier/scheduler.go | 2 +- notifier/scheduler_test.go | 8 +- 23 files changed, 388 insertions(+), 177 deletions(-) diff --git a/checker/check.go b/checker/check.go index 7c295792e..698869cb8 100644 --- a/checker/check.go +++ b/checker/check.go @@ -12,7 +12,6 @@ import ( ) const ( - secondsInHour int64 = 3600 checkPointGap int64 = 120 ) @@ -222,6 +221,7 @@ func newCheckData(lastCheck *moira.CheckData, checkTimeStamp int64) moira.CheckD newCheckData.Timestamp = checkTimeStamp newCheckData.MetricsToTargetRelation = metricsToTargetRelation newCheckData.Message = "" + return newCheckData } @@ -465,34 +465,33 @@ func (triggerChecker *TriggerChecker) getMetricStepsStates( metrics map[string]metricSource.MetricData, logger moira.Logger, ) ( - last moira.MetricState, - current []moira.MetricState, + lastMetricState moira.MetricState, + newMetricStates []moira.MetricState, err error, ) { var startTime int64 var stepTime int64 for _, metric := range metrics { // Taking values from any metric - last = triggerChecker.lastCheck.GetOrCreateMetricState( + lastMetricState = triggerChecker.lastCheck.GetOrCreateMetricState( metricName, - metric.StartTime-secondsInHour, triggerChecker.trigger.MuteNewMetrics, + checkPointGap, ) + startTime = metric.StartTime stepTime = metric.StepTime break } - checkPoint := last.GetCheckPoint(checkPointGap) + checkPoint := lastMetricState.GetCheckPoint(checkPointGap) logger.Debug(). Int64(moira.LogFieldNameCheckpoint, checkPoint). Msg("Checkpoint got") - current = make([]moira.MetricState, 0) - // DO NOT CHANGE // Specific optimization magic - previousState := last + previousMetricState := lastMetricState difference := moira.MaxInt64(checkPoint-startTime, 0) stepsDifference := difference / stepTime if (difference % stepTime) > 0 { @@ -500,18 +499,24 @@ func (triggerChecker *TriggerChecker) getMetricStepsStates( } valueTimestamp := startTime + stepTime*stepsDifference endTimestamp := triggerChecker.until + stepTime + + newMetricStates = make([]moira.MetricState, 0) + for ; valueTimestamp < endTimestamp; valueTimestamp += stepTime { - metricNewState, err := triggerChecker.getMetricDataState(metrics, &previousState, &valueTimestamp, &checkPoint, logger) + newMetricState, err := triggerChecker.getMetricDataState(metrics, &previousMetricState, &valueTimestamp, &checkPoint, logger) if err != nil { - return last, current, err + return lastMetricState, newMetricStates, err } - if metricNewState == nil { + + if newMetricState == nil { continue } - previousState = *metricNewState - current = append(current, *metricNewState) + + previousMetricState = *newMetricState + newMetricStates = append(newMetricStates, *newMetricState) } - return last, current, nil + + return lastMetricState, newMetricStates, nil } func (triggerChecker *TriggerChecker) getMetricDataState( diff --git a/checker/check_test.go b/checker/check_test.go index 772f1eb0b..8a5ae0b1e 100644 --- a/checker/check_test.go +++ b/checker/check_test.go @@ -15,6 +15,7 @@ import ( "go.uber.org/mock/gomock" "github.com/moira-alert/moira/metrics" + mock_clock "github.com/moira-alert/moira/mock/clock" mock_metric_source "github.com/moira-alert/moira/mock/metric_source" mock_moira_alert "github.com/moira-alert/moira/mock/moira-alert" . "github.com/smartystreets/goconvey/convey" @@ -63,9 +64,9 @@ func TestGetMetricDataState(t *testing.T) { Suppressed: true, } - var valueTimestamp int64 = 37 - var checkPoint int64 = 47 Convey("Checkpoint more than valueTimestamp", t, func() { + var valueTimestamp int64 = 37 + var checkPoint int64 = 47 metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err, ShouldBeNil) So(metricState, ShouldBeNil) @@ -136,6 +137,7 @@ func TestTriggerChecker_PrepareMetrics(t *testing.T) { AloneMetrics: map[string]bool{}, }, } + Convey("last check has no metrics", func() { Convey("fetched metrics is empty", func() { prepared, alone, err := triggerChecker.prepareMetrics(map[string][]metricSource.MetricData{}) @@ -251,6 +253,7 @@ func TestTriggerChecker_PrepareMetrics(t *testing.T) { So(err, ShouldBeNil) }) }) + Convey("fetched metrics is empty", func() { prepared, alone, err := triggerChecker.prepareMetrics(map[string][]metricSource.MetricData{}) So(prepared, ShouldHaveLength, 3) @@ -263,6 +266,7 @@ func TestTriggerChecker_PrepareMetrics(t *testing.T) { So(alone, ShouldBeEmpty) So(err, ShouldBeNil) }) + Convey("fetched metrics has only wildcards, step is 0", func() { prepared, alone, err := triggerChecker.prepareMetrics( map[string][]metricSource.MetricData{ @@ -283,6 +287,7 @@ func TestTriggerChecker_PrepareMetrics(t *testing.T) { So(alone, ShouldBeEmpty) So(err, ShouldBeNil) }) + Convey("fetched metrics has only wildcards, step is 10", func() { prepared, alone, err := triggerChecker.prepareMetrics( map[string][]metricSource.MetricData{ @@ -304,6 +309,7 @@ func TestTriggerChecker_PrepareMetrics(t *testing.T) { So(alone, ShouldBeEmpty) So(err, ShouldBeNil) }) + Convey("fetched metrics has one of last check metrics", func() { prepared, alone, err := triggerChecker.prepareMetrics( map[string][]metricSource.MetricData{ @@ -321,6 +327,7 @@ func TestTriggerChecker_PrepareMetrics(t *testing.T) { So(alone, ShouldBeEmpty) So(err, ShouldBeNil) }) + Convey("fetched metrics has one of last check metrics and one new", func() { prepared, alone, err := triggerChecker.prepareMetrics( map[string][]metricSource.MetricData{ @@ -443,6 +450,7 @@ func TestGetMetricStepsStates(t *testing.T) { EventTimestamp: 11, }, } + Convey("Metric has all valid values", func() { _, metricStates, err := triggerChecker.getMetricStepsStates("main.metric", map[string]metricSource.MetricData{"t1": metricData2, "t2": addMetricData}, logger) So(err, ShouldBeNil) @@ -532,6 +540,7 @@ func TestCheckForNODATA(t *testing.T) { Maintenance: 11111, Suppressed: true, } + Convey("No TTL", t, func() { triggerChecker := TriggerChecker{} needToDeleteMetric, currentState := triggerChecker.checkForNoData(metricLastState, logger) @@ -560,6 +569,7 @@ func TestCheckForNODATA(t *testing.T) { So(needToDeleteMetric, ShouldBeFalse) So(currentState, ShouldBeNil) }) + Convey("2", func() { metricLastState.Timestamp = 401 needToDeleteMetric, currentState := triggerChecker.checkForNoData(metricLastState, logger) @@ -662,6 +672,8 @@ func TestCheck(t *testing.T) { messageException := `Unknown graphite function: "WrongFunction"` unknownFunctionExc := local.ErrorUnknownFunction(fmt.Errorf(messageException)) + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + var ttl int64 = 30 checkerMetrics, _ := metrics. @@ -674,8 +686,8 @@ func TestCheck(t *testing.T) { logger: logger, config: &Config{}, metrics: checkerMetrics, - from: 17, - until: 67, + from: testTime - 5*retention, + until: testTime, ttl: ttl, ttlState: moira.TTLStateNODATA, trigger: &moira.Trigger{ @@ -689,11 +701,11 @@ func TestCheck(t *testing.T) { }, lastCheck: &moira.CheckData{ State: moira.StateOK, - Timestamp: 57, + Timestamp: testTime - retention, Metrics: map[string]moira.MetricState{ metric: { State: moira.StateOK, - Timestamp: 26, + Timestamp: testTime - 4*retention - 1, }, }, }, @@ -717,7 +729,7 @@ func TestCheck(t *testing.T) { TriggerID: triggerChecker.triggerID, State: moira.StateEXCEPTION, OldState: moira.StateOK, - Timestamp: int64(67), + Timestamp: testTime, Metric: triggerChecker.trigger.Name, }, true).Return(nil), dataBase.EXPECT().SetTriggerLastCheck( @@ -737,7 +749,7 @@ func TestCheck(t *testing.T) { TriggerID: triggerChecker.triggerID, State: moira.StateEXCEPTION, OldState: moira.StateOK, - Timestamp: 67, + Timestamp: testTime, Metric: triggerChecker.trigger.Name, } @@ -767,14 +779,14 @@ func TestCheck(t *testing.T) { Convey("Switch state to OK. Event should be created", func() { triggerChecker.lastCheck.State = moira.StateEXCEPTION - triggerChecker.lastCheck.EventTimestamp = 67 + triggerChecker.lastCheck.EventTimestamp = testTime triggerChecker.lastCheck.LastSuccessfulCheckTimestamp = triggerChecker.until eventMetrics := map[string]moira.MetricState{ metric: { - EventTimestamp: 17, + EventTimestamp: testTime - 5*retention, State: moira.StateOK, Suppressed: false, - Timestamp: 57, + Timestamp: testTime - retention, Values: map[string]float64{"t1": 4}, }, } @@ -784,7 +796,7 @@ func TestCheck(t *testing.T) { TriggerID: triggerChecker.triggerID, State: moira.StateOK, OldState: moira.StateEXCEPTION, - Timestamp: 67, + Timestamp: testTime, Metric: triggerChecker.trigger.Name, } @@ -797,6 +809,7 @@ func TestCheck(t *testing.T) { LastSuccessfulCheckTimestamp: triggerChecker.until, MetricsToTargetRelation: map[string]string{"t1": "super.puper.metric"}, } + gomock.InOrder( source.EXPECT().Fetch(pattern, triggerChecker.from, triggerChecker.until, true).Return(fetchResult, nil), fetchResult.EXPECT().GetMetricsData().Return([]metricSource.MetricData{*metricSource.MakeMetricData(metric, []float64{0, 1, 2, 3, 4}, retention, triggerChecker.from)}), @@ -819,9 +832,9 @@ func TestCheck(t *testing.T) { lastCheck := moira.CheckData{ Metrics: map[string]moira.MetricState{ metric: { - EventTimestamp: 57, + EventTimestamp: testTime - retention, State: moira.StateERROR, - Timestamp: 57, + Timestamp: testTime - retention, MaintenanceInfo: moira.MaintenanceInfo{}, Values: map[string]float64{"t1": 25}, }, @@ -838,7 +851,7 @@ func TestCheck(t *testing.T) { TriggerID: triggerChecker.triggerID, State: moira.StateERROR, OldState: moira.StateOK, - Timestamp: 57, + Timestamp: testTime - retention, Metric: metric, Values: map[string]float64{"t1": 25}, } @@ -861,13 +874,14 @@ func TestCheck(t *testing.T) { err := triggerChecker.Check() So(err, ShouldBeNil) }) + Convey("Duplicate error", func() { lastCheck := moira.CheckData{ Metrics: map[string]moira.MetricState{ metric: { - EventTimestamp: 17, + EventTimestamp: testTime - 5*retention, State: moira.StateOK, - Timestamp: 57, + Timestamp: testTime - retention, MaintenanceInfo: moira.MaintenanceInfo{}, Values: map[string]float64{"t1": 4}, }, @@ -885,7 +899,7 @@ func TestCheck(t *testing.T) { TriggerID: triggerChecker.triggerID, State: moira.StateEXCEPTION, OldState: moira.StateOK, - Timestamp: 67, + Timestamp: testTime, Metric: triggerChecker.trigger.Name, } @@ -908,6 +922,7 @@ func TestCheck(t *testing.T) { }) Convey("Alone metrics error", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) metricName1 := "test.metric.1" metricName2 := "test.metric.2" metricNameAlone := "test.metric.alone" @@ -917,18 +932,16 @@ func TestCheck(t *testing.T) { lastCheck := moira.CheckData{ Metrics: map[string]moira.MetricState{ metricName1: { - EventTimestamp: -3533, + EventTimestamp: testTime - checkPointGap, State: moira.StateNODATA, - Timestamp: -3533, + Timestamp: testTime, MaintenanceInfo: moira.MaintenanceInfo{}, - Values: map[string]float64{}, }, metricName2: { - EventTimestamp: -3533, + EventTimestamp: testTime - checkPointGap, State: moira.StateNODATA, - Timestamp: -3533, + Timestamp: testTime, MaintenanceInfo: moira.MaintenanceInfo{}, - Values: map[string]float64{}, }, }, MetricsToTargetRelation: map[string]string{"t2": metricNameAlone}, @@ -938,6 +951,7 @@ func TestCheck(t *testing.T) { EventTimestamp: triggerChecker.until, LastSuccessfulCheckTimestamp: triggerChecker.until, Message: "", + Clock: mockTime, } expression := "OK" triggerChecker.trigger.AloneMetrics = map[string]bool{"t2": true} @@ -947,7 +961,8 @@ func TestCheck(t *testing.T) { triggerChecker.lastCheck = &moira.CheckData{ Metrics: map[string]moira.MetricState{}, State: moira.StateOK, - Timestamp: triggerChecker.until - 3600, + Timestamp: triggerChecker.until - metricsTTL, + Clock: mockTime, } gomock.InOrder( @@ -972,6 +987,8 @@ func TestCheck(t *testing.T) { dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL), dataBase.EXPECT().RemoveMetricsValues([]string{metricName1, metricNameAlone, metricName2}, triggerChecker.until-metricsTTL).Return(nil), + mockTime.EXPECT().NowUnix().Return(testTime).Times(4), + dataBase.EXPECT().SetTriggerLastCheck( triggerChecker.triggerID, &lastCheck, @@ -1038,6 +1055,10 @@ func TestIgnoreNodataToOk(t *testing.T) { logger.Level("info") // nolint: errcheck defer mockCtrl.Finish() + mockTime := mock_clock.NewMockClock(mockCtrl) + + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + var retention int64 = 10 var warnValue float64 = 10 var errValue float64 = 20 @@ -1048,13 +1069,14 @@ func TestIgnoreNodataToOk(t *testing.T) { Metrics: make(map[string]moira.MetricState), State: moira.StateNODATA, Timestamp: 66, + Clock: mockTime, } triggerChecker := TriggerChecker{ triggerID: "SuperId", logger: logger, config: &Config{}, - from: 3617, - until: 3667, + from: testTime - ttl, + until: testTime, ttl: ttl, ttlState: moira.TTLStateNODATA, trigger: &moira.Trigger{ @@ -1073,14 +1095,16 @@ func TestIgnoreNodataToOk(t *testing.T) { checkData := newCheckData(&lastCheck, triggerChecker.until) Convey("First Event, NODATA - OK is ignored", t, func() { + mockTime.EXPECT().NowUnix().Return(testTime).Times(2) + triggerChecker.trigger.MuteNewMetrics = true newCheckData, err := triggerChecker.check(metricsToCheck, aloneMetrics, checkData, logger) So(err, ShouldBeNil) So(newCheckData, ShouldResemble, moira.CheckData{ Metrics: map[string]moira.MetricState{ metric: { - Timestamp: time.Now().Unix(), - EventTimestamp: time.Now().Unix(), + Timestamp: testTime, + EventTimestamp: testTime - checkPointGap, State: moira.StateOK, Value: nil, Values: nil, @@ -1090,6 +1114,7 @@ func TestIgnoreNodataToOk(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateNODATA, Score: 0, + Clock: mockTime, }) }) } @@ -1097,28 +1122,34 @@ func TestIgnoreNodataToOk(t *testing.T) { func TestHandleTrigger(t *testing.T) { mockCtrl := gomock.NewController(t) dataBase := mock_moira_alert.NewMockDatabase(mockCtrl) + mockTime := mock_clock.NewMockClock(mockCtrl) logger, _ := logging.GetLogger("Test") logger.Level("info") // nolint: errcheck defer mockCtrl.Finish() + var metricsTTL int64 = 3600 var retention int64 = 10 var warnValue float64 = 10 var errValue float64 = 20 pattern := "super.puper.pattern" metric := "super.puper.metric" var ttl int64 = 600 + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + lastCheck := moira.CheckData{ Metrics: make(map[string]moira.MetricState), State: moira.StateNODATA, - Timestamp: 66, + Timestamp: testTime - metricsTTL, + Clock: mockTime, } + triggerChecker := TriggerChecker{ triggerID: "SuperId", database: dataBase, logger: logger, config: &Config{}, - from: 3617, - until: 3667, + from: testTime - 5*retention, + until: testTime, ttl: ttl, ttlState: moira.TTLStateNODATA, trigger: &moira.Trigger{ @@ -1137,10 +1168,12 @@ func TestHandleTrigger(t *testing.T) { lastCheck.MetricsToTargetRelation = conversion.GetRelations(aloneMetrics, triggerChecker.trigger.AloneMetrics) checkData := newCheckData(&lastCheck, triggerChecker.until) metricsToCheck := map[string]map[string]metricSource.MetricData{} + + mockTime.EXPECT().NowUnix().Return(testTime).Times(2) dataBase.EXPECT().PushNotificationEvent( &moira.NotificationEvent{ TriggerID: triggerChecker.triggerID, - Timestamp: 3617, + Timestamp: testTime - 5*retention, State: moira.StateOK, OldState: moira.StateNODATA, Metric: metric, @@ -1153,8 +1186,8 @@ func TestHandleTrigger(t *testing.T) { So(checkData, ShouldResemble, moira.CheckData{ Metrics: map[string]moira.MetricState{ metric: { - Timestamp: 3657, - EventTimestamp: 3617, + Timestamp: testTime - retention, + EventTimestamp: testTime - 5*retention, State: moira.StateOK, Value: nil, Values: map[string]float64{"t1": 4}, @@ -1164,20 +1197,22 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateNODATA, Score: 0, + Clock: mockTime, }) }) lastCheck = moira.CheckData{ Metrics: map[string]moira.MetricState{ metric: { - Timestamp: 3647, - EventTimestamp: 3607, + Timestamp: testTime - 2*retention, + EventTimestamp: testTime - 6*retention, State: moira.StateOK, Values: map[string]float64{"t1": 3}, }, }, State: moira.StateOK, - Timestamp: 3655, + Timestamp: testTime - retention - 2, + Clock: mockTime, } Convey("Last check is not empty", func() { @@ -1191,8 +1226,8 @@ func TestHandleTrigger(t *testing.T) { So(checkData, ShouldResemble, moira.CheckData{ Metrics: map[string]moira.MetricState{ metric: { - Timestamp: 3657, - EventTimestamp: 3607, + Timestamp: testTime - retention, + EventTimestamp: testTime - 6*retention, State: moira.StateOK, Value: nil, Values: map[string]float64{"t1": 4}, @@ -1202,13 +1237,15 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateOK, Score: 0, + Clock: mockTime, }) }) Convey("No data too long", func() { - triggerChecker.from = 4217 - triggerChecker.until = 4267 - lastCheck.Timestamp = 4267 + triggerChecker.from = testTime + ttl - 5*retention + triggerChecker.until = testTime + ttl + lastCheck.Timestamp = testTime + ttl + dataBase.EXPECT().PushNotificationEvent(&moira.NotificationEvent{ TriggerID: triggerChecker.triggerID, Timestamp: lastCheck.Timestamp, @@ -1238,14 +1275,15 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateOK, Score: 0, + Clock: mockTime, }) }) Convey("No data too long and ttlState is delete, the metric is not on Maintenance, so it will be removed", func() { - triggerChecker.from = 4217 - triggerChecker.until = 4267 + triggerChecker.from = testTime + ttl - 5*retention + triggerChecker.until = testTime + ttl triggerChecker.ttlState = moira.TTLStateDEL - lastCheck.Timestamp = 4267 + lastCheck.Timestamp = testTime + ttl dataBase.EXPECT().RemovePatternsMetrics(triggerChecker.trigger.Patterns).Return(nil) @@ -1263,18 +1301,19 @@ func TestHandleTrigger(t *testing.T) { Score: 0, LastSuccessfulCheckTimestamp: 0, MetricsToTargetRelation: map[string]string{}, + Clock: mockTime, }) }) metricState := lastCheck.Metrics[metric] - metricState.Maintenance = 5000 + metricState.Maintenance = testTime + ttl lastCheck.Metrics[metric] = metricState Convey("No data too long and ttlState is delete, but the metric is on maintenance and DeletedButKept is false, so it won't be deleted", func() { - triggerChecker.from = 4217 - triggerChecker.until = 4267 + triggerChecker.from = testTime + ttl - 5*retention + triggerChecker.until = testTime + ttl triggerChecker.ttlState = moira.TTLStateDEL - lastCheck.Timestamp = 4267 + lastCheck.Timestamp = testTime + ttl aloneMetrics := map[string]metricSource.MetricData{"t1": *metricSource.MakeMetricData(metric, []float64{}, retention, triggerChecker.from)} lastCheck.MetricsToTargetRelation = conversion.GetRelations(aloneMetrics, triggerChecker.trigger.AloneMetrics) @@ -1299,6 +1338,7 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateOK, Score: 0, + Clock: mockTime, }) }) @@ -1307,10 +1347,10 @@ func TestHandleTrigger(t *testing.T) { lastCheck.Metrics[metric] = metricState Convey("Metric on maintenance, DeletedButKept is true, ttlState is delete, but a new metric comes in and DeletedButKept becomes false", func() { - triggerChecker.from = 4217 - triggerChecker.until = 4267 + triggerChecker.from = testTime + ttl - 5*retention + triggerChecker.until = testTime + ttl triggerChecker.ttlState = moira.TTLStateDEL - lastCheck.Timestamp = 4227 + lastCheck.Timestamp = testTime + ttl + retention aloneMetrics := map[string]metricSource.MetricData{"t1": *metricSource.MakeMetricData(metric, []float64{5}, retention, triggerChecker.from)} lastCheck.MetricsToTargetRelation = conversion.GetRelations(aloneMetrics, triggerChecker.trigger.AloneMetrics) @@ -1335,18 +1375,19 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateOK, Score: 0, + Clock: mockTime, }) }) metricState = lastCheck.Metrics[metric] - metricState.Maintenance = 4000 + metricState.Maintenance = testTime + ttl - 10*retention lastCheck.Metrics[metric] = metricState Convey("No data too long and ttlState is delete, the time for Maintenance of metric is over, so it will be deleted", func() { - triggerChecker.from = 4217 - triggerChecker.until = 4267 + triggerChecker.from = testTime + ttl - 5*retention + triggerChecker.until = testTime + ttl triggerChecker.ttlState = moira.TTLStateDEL - lastCheck.Timestamp = 4267 + lastCheck.Timestamp = testTime + ttl dataBase.EXPECT().RemovePatternsMetrics(triggerChecker.trigger.Patterns).Return(nil) @@ -1364,6 +1405,7 @@ func TestHandleTrigger(t *testing.T) { Score: 0, LastSuccessfulCheckTimestamp: 0, MetricsToTargetRelation: map[string]string{}, + Clock: mockTime, }) }) }) @@ -1381,7 +1423,8 @@ func TestHandleTrigger(t *testing.T) { triggerChecker.lastCheck = &moira.CheckData{ Metrics: make(map[string]moira.MetricState), State: moira.StateNODATA, - Timestamp: 66, + Timestamp: testTime - metricsTTL, + Clock: mockTime, } Convey("Without any metrics", func() { @@ -1397,6 +1440,7 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateNODATA, Score: 0, + Clock: mockTime, }) }) @@ -1413,6 +1457,7 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateNODATA, Score: 0, + Clock: mockTime, }) }) @@ -1424,10 +1469,12 @@ func TestHandleTrigger(t *testing.T) { "t2": *metricSource.MakeMetricData(metric, []float64{5}, retention, triggerChecker.from), }, } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(2) dataBase.EXPECT().PushNotificationEvent( &moira.NotificationEvent{ TriggerID: triggerChecker.triggerID, - Timestamp: 4217, + Timestamp: testTime + ttl - 5*retention, State: moira.StateERROR, OldState: moira.StateNODATA, Metric: "test2", @@ -1440,9 +1487,9 @@ func TestHandleTrigger(t *testing.T) { So(checkData, ShouldResemble, moira.CheckData{ Metrics: map[string]moira.MetricState{ "test2": { - EventTimestamp: 4217, + EventTimestamp: testTime + ttl - 5*retention, State: moira.StateERROR, - Timestamp: 4217, + Timestamp: testTime + ttl - 5*retention, Values: map[string]float64{"t1": 5, "t2": 5}, }, }, @@ -1450,6 +1497,7 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateNODATA, Score: 0, + Clock: mockTime, }) }) @@ -1462,10 +1510,12 @@ func TestHandleTrigger(t *testing.T) { "t2": *metricSource.MakeMetricData(metric, []float64{5}, retention, triggerChecker.from), }, } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(2) dataBase.EXPECT().PushNotificationEvent( &moira.NotificationEvent{ TriggerID: triggerChecker.triggerID, - Timestamp: 4217, + Timestamp: testTime + ttl - 5*retention, State: moira.StateOK, OldState: moira.StateNODATA, Metric: "test1", @@ -1478,9 +1528,9 @@ func TestHandleTrigger(t *testing.T) { So(checkData, ShouldResemble, moira.CheckData{ Metrics: map[string]moira.MetricState{ "test1": { - EventTimestamp: 4217, + EventTimestamp: testTime + ttl - 5*retention, State: moira.StateOK, - Timestamp: 4217, + Timestamp: testTime + ttl - 5*retention, Values: map[string]float64{"t1": 10, "t2": 5}, }, }, @@ -1488,6 +1538,7 @@ func TestHandleTrigger(t *testing.T) { Timestamp: triggerChecker.until, State: moira.StateNODATA, Score: 0, + Clock: mockTime, }) }) }) diff --git a/checker/trigger_checker.go b/checker/trigger_checker.go index dfd3e94a5..655a47f42 100644 --- a/checker/trigger_checker.go +++ b/checker/trigger_checker.go @@ -5,11 +5,14 @@ import ( "time" "github.com/moira-alert/moira" + "github.com/moira-alert/moira/clock" "github.com/moira-alert/moira/database" metricSource "github.com/moira-alert/moira/metric_source" "github.com/moira-alert/moira/metrics" ) +var tenMinInSec = int64((10 * time.Minute).Seconds()) + // TriggerChecker represents data, used for handling new trigger state. type TriggerChecker struct { database moira.Database @@ -90,6 +93,7 @@ func MakeTriggerChecker( ttl: trigger.TTL, ttlState: getTTLState(trigger.TTLState), } + return triggerChecker, nil } @@ -111,6 +115,10 @@ func getLastCheck(dataBase moira.Database, triggerID string, emptyLastCheckTimes lastCheck.Timestamp = emptyLastCheckTimestamp } + if lastCheck.Clock == nil { + lastCheck.Clock = clock.NewSystemClock() + } + return &lastCheck, nil } @@ -118,6 +126,7 @@ func getTTLState(triggerTTLState *moira.TTLState) moira.TTLState { if triggerTTLState != nil { return *triggerTTLState } + return moira.TTLStateNODATA } @@ -125,5 +134,6 @@ func calculateFrom(lastCheckTimestamp, triggerTTL int64) int64 { if triggerTTL != 0 { return lastCheckTimestamp - triggerTTL } - return lastCheckTimestamp - 600 + + return lastCheckTimestamp - tenMinInSec } diff --git a/checker/trigger_checker_test.go b/checker/trigger_checker_test.go index 234ea9da0..164fb1385 100644 --- a/checker/trigger_checker_test.go +++ b/checker/trigger_checker_test.go @@ -3,8 +3,10 @@ package checker import ( "fmt" "testing" + "time" "github.com/moira-alert/moira" + "github.com/moira-alert/moira/clock" "github.com/moira-alert/moira/database" logging "github.com/moira-alert/moira/logging/zerolog_adapter" metricSource "github.com/moira-alert/moira/metric_source" @@ -15,6 +17,8 @@ import ( "go.uber.org/mock/gomock" ) +var hourInSec = int64(time.Hour.Seconds()) + func TestInitTriggerChecker(t *testing.T) { mockCtrl := gomock.NewController(t) logger, _ := logging.GetLogger("Test") @@ -120,6 +124,8 @@ func TestInitTriggerChecker(t *testing.T) { actual, err := MakeTriggerChecker(triggerID, dataBase, logger, config, metricSource.CreateTestMetricSourceProvider(localSource, nil, nil), checkerMetrics) So(err, ShouldBeNil) + expectedLastCheck := lastCheck + expectedLastCheck.Clock = clock.NewSystemClock() expected := TriggerChecker{ triggerID: triggerID, database: dataBase, @@ -129,7 +135,7 @@ func TestInitTriggerChecker(t *testing.T) { trigger: &trigger, ttl: trigger.TTL, ttlState: *trigger.TTLState, - lastCheck: &lastCheck, + lastCheck: &expectedLastCheck, from: lastCheck.Timestamp - ttl, until: actual.until, metrics: metrics, @@ -155,12 +161,14 @@ func TestInitTriggerChecker(t *testing.T) { lastCheck: &moira.CheckData{ Metrics: make(map[string]moira.MetricState), State: moira.StateOK, - Timestamp: actual.until - 3600, + Timestamp: actual.until - hourInSec, + Clock: clock.NewSystemClock(), }, - from: actual.until - 3600 - ttl, + from: actual.until - hourInSec - ttl, until: actual.until, metrics: metrics, } + So(*actual, ShouldResemble, expected) }) @@ -185,9 +193,10 @@ func TestInitTriggerChecker(t *testing.T) { lastCheck: &moira.CheckData{ Metrics: make(map[string]moira.MetricState), State: moira.StateOK, - Timestamp: actual.until - 3600, + Timestamp: actual.until - hourInSec, + Clock: clock.NewSystemClock(), }, - from: actual.until - 3600 - 600, + from: actual.until - hourInSec - tenMinInSec, until: actual.until, metrics: metrics, } @@ -201,6 +210,8 @@ func TestInitTriggerChecker(t *testing.T) { So(err, ShouldBeNil) + expectedLastCheck := lastCheck + expectedLastCheck.Clock = clock.NewSystemClock() expected := TriggerChecker{ triggerID: triggerID, database: dataBase, @@ -210,8 +221,8 @@ func TestInitTriggerChecker(t *testing.T) { trigger: &trigger, ttl: 0, ttlState: moira.TTLStateNODATA, - lastCheck: &lastCheck, - from: lastCheck.Timestamp - 600, + lastCheck: &expectedLastCheck, + from: lastCheck.Timestamp - tenMinInSec, until: actual.until, metrics: metrics, } diff --git a/clock/clock.go b/clock/clock.go index b5e253805..f78ed070c 100644 --- a/clock/clock.go +++ b/clock/clock.go @@ -10,7 +10,12 @@ func NewSystemClock() *SystemClock { return &SystemClock{} } -// Now returns time.Time. -func (t *SystemClock) Now() time.Time { +// Now returns now time.Time with UTC location. +func (t *SystemClock) NowUTC() time.Time { return time.Now().UTC() } + +// Now returns now time.Time as a Unix time. +func (t *SystemClock) NowUnix() int64 { + return time.Now().Unix() +} diff --git a/database/redis/last_check_test.go b/database/redis/last_check_test.go index 92dd43c32..e3105952c 100644 --- a/database/redis/last_check_test.go +++ b/database/redis/last_check_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/gofrs/uuid" + "github.com/moira-alert/moira/clock" logging "github.com/moira-alert/moira/logging/zerolog_adapter" . "github.com/smartystreets/goconvey/convey" @@ -303,6 +304,7 @@ func TestLastCheck(t *testing.T) { }, }, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }) }) }) @@ -514,14 +516,17 @@ func TestGetTriggersLastCheck(t *testing.T) { { Timestamp: 1, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, { Timestamp: 2, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, { Timestamp: 3, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, }) }) @@ -540,11 +545,13 @@ func TestGetTriggersLastCheck(t *testing.T) { { Timestamp: 1, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, nil, { Timestamp: 3, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, }) }) @@ -556,10 +563,12 @@ func TestGetTriggersLastCheck(t *testing.T) { { Timestamp: 1, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, { Timestamp: 2, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, nil, }) @@ -573,10 +582,12 @@ func TestGetTriggersLastCheck(t *testing.T) { { Timestamp: 2, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, { Timestamp: 3, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, }) }) @@ -726,6 +737,7 @@ var lastCheckTest = moira.CheckData{ }, }, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), } var lastCheckWithNoMetrics = moira.CheckData{ @@ -734,6 +746,7 @@ var lastCheckWithNoMetrics = moira.CheckData{ Timestamp: 1504509981, Metrics: make(map[string]moira.MetricState), MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), } var lastCheckWithNoMetricsWithMaintenance = moira.CheckData{ @@ -743,4 +756,5 @@ var lastCheckWithNoMetricsWithMaintenance = moira.CheckData{ Maintenance: 1000, Metrics: make(map[string]moira.MetricState), MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), } diff --git a/database/redis/metric.go b/database/redis/metric.go index 0e12806a6..f695ae44f 100644 --- a/database/redis/metric.go +++ b/database/redis/metric.go @@ -429,7 +429,7 @@ func (connector *DbConnector) CleanUpFutureMetrics(duration time.Duration) error return ErrCleanUpDurationLessThanZero } - fromTs := connector.clock.Now().Add(duration).Unix() + fromTs := connector.clock.NowUTC().Add(duration).Unix() from := strconv.FormatInt(fromTs, 10) to := "+inf" diff --git a/database/redis/metric_test.go b/database/redis/metric_test.go index 47f392284..74e6f17d5 100644 --- a/database/redis/metric_test.go +++ b/database/redis/metric_test.go @@ -973,7 +973,7 @@ func TestCleanupFutureMetrics(t *testing.T) { }) So(err, ShouldBeNil) - mockClock.EXPECT().Now().Return(testTime).Times(1) + mockClock.EXPECT().NowUTC().Return(testTime).Times(1) err = dataBase.CleanUpFutureMetrics(time.Hour) So(err, ShouldBeNil) @@ -1033,7 +1033,7 @@ func TestCleanupFutureMetrics(t *testing.T) { }) So(err, ShouldBeNil) - mockClock.EXPECT().Now().Return(testTime).Times(1) + mockClock.EXPECT().NowUTC().Return(testTime).Times(1) err = dataBase.CleanUpFutureMetrics(5 * time.Second) So(err, ShouldBeNil) diff --git a/database/redis/notification_test.go b/database/redis/notification_test.go index 06048b813..d0a25ce56 100644 --- a/database/redis/notification_test.go +++ b/database/redis/notification_test.go @@ -8,6 +8,7 @@ import ( "github.com/go-redis/redis/v8" "github.com/moira-alert/moira" + "github.com/moira-alert/moira/clock" logging "github.com/moira-alert/moira/logging/zerolog_adapter" "github.com/moira-alert/moira/notifier" "github.com/stretchr/testify/assert" @@ -1257,14 +1258,17 @@ func TestGetNotificationsTriggerChecks(t *testing.T) { { Timestamp: 1, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, { Timestamp: 1, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, { Timestamp: 2, MetricsToTargetRelation: map[string]string{}, + Clock: clock.NewSystemClock(), }, }) @@ -1283,7 +1287,7 @@ func TestGetNotificationsTriggerChecks(t *testing.T) { notifications := []*moira.ScheduledNotification{notification1, notification2, notification3} triggerChecks, err := database.getNotificationsTriggerChecks(notifications) So(err, ShouldBeNil) - So(triggerChecks, ShouldResemble, []*moira.CheckData{nil, nil, {Timestamp: 2, MetricsToTargetRelation: map[string]string{}}}) + So(triggerChecks, ShouldResemble, []*moira.CheckData{nil, nil, {Timestamp: 2, MetricsToTargetRelation: map[string]string{}, Clock: clock.NewSystemClock()}}) err = database.RemoveAllNotifications() So(err, ShouldBeNil) diff --git a/database/redis/reply/check.go b/database/redis/reply/check.go index f3f19ce78..25968fba3 100644 --- a/database/redis/reply/check.go +++ b/database/redis/reply/check.go @@ -7,6 +7,7 @@ import ( "github.com/go-redis/redis/v8" "github.com/moira-alert/moira" + "github.com/moira-alert/moira/clock" "github.com/moira-alert/moira/database" ) @@ -85,6 +86,7 @@ func (d checkDataStorageElement) toCheckData() moira.CheckData { Suppressed: d.Suppressed, SuppressedState: d.SuppressedState, Message: d.Message, + Clock: clock.NewSystemClock(), } } diff --git a/database/redis/trigger.go b/database/redis/trigger.go index 86ca752fd..71486e91b 100644 --- a/database/redis/trigger.go +++ b/database/redis/trigger.go @@ -254,7 +254,7 @@ func (connector *DbConnector) preSaveTrigger(newTrigger *moira.Trigger, oldTrigg newTrigger.Patterns = make([]string, 0) } - now := connector.clock.Now().Unix() + now := connector.clock.NowUnix() newTrigger.UpdatedAt = &now if oldTrigger != nil { newTrigger.CreatedAt = oldTrigger.CreatedAt diff --git a/database/redis/trigger_test.go b/database/redis/trigger_test.go index 957b2c2ca..d3ff59cdf 100644 --- a/database/redis/trigger_test.go +++ b/database/redis/trigger_test.go @@ -557,7 +557,7 @@ func TestRemoteTrigger(t *testing.T) { Convey("Saving remote trigger", t, func() { Convey("Trigger should be saved correctly", func() { - systemClock.EXPECT().Now().Return(time.Date(2022, time.June, 7, 10, 0, 0, 0, time.UTC)) + systemClock.EXPECT().NowUnix().Return(time.Date(2022, time.June, 7, 10, 0, 0, 0, time.UTC).Unix()) err := dataBase.SaveTrigger(trigger.ID, trigger) So(err, ShouldBeNil) @@ -603,7 +603,7 @@ func TestRemoteTrigger(t *testing.T) { trigger.TriggerSource = moira.GraphiteLocal trigger.Patterns = []string{pattern} Convey("Trigger should be saved correctly", func() { - systemClock.EXPECT().Now().Return(time.Date(2022, time.June, 7, 10, 0, 0, 0, time.UTC)) + systemClock.EXPECT().NowUnix().Return(time.Date(2022, time.June, 7, 10, 0, 0, 0, time.UTC).Unix()) err := dataBase.SaveTrigger(trigger.ID, trigger) So(err, ShouldBeNil) @@ -645,7 +645,7 @@ func TestRemoteTrigger(t *testing.T) { trigger.TriggerSource = moira.GraphiteRemote Convey("Update this trigger as remote", func() { - systemClock.EXPECT().Now().Return(time.Date(2022, time.June, 7, 10, 0, 0, 0, time.UTC)) + systemClock.EXPECT().NowUnix().Return(time.Date(2022, time.June, 7, 10, 0, 0, 0, time.UTC).Unix()) err := dataBase.SaveTrigger(trigger.ID, trigger) So(err, ShouldBeNil) @@ -729,7 +729,7 @@ func TestDbConnector_preSaveTrigger(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() systemClock := mock_clock.NewMockClock(mockCtrl) - systemClock.EXPECT().Now().Return(testTime).Times(6) + systemClock.EXPECT().NowUnix().Return(testTime.Unix()).Times(6) connector := &DbConnector{clock: systemClock} patterns := []string{"pattern-1", "pattern-2"} diff --git a/datatypes.go b/datatypes.go index 9796479ef..00fcd77d2 100644 --- a/datatypes.go +++ b/datatypes.go @@ -334,11 +334,11 @@ func (notification *ScheduledNotification) GetState(triggerCheck *CheckData) sch return RemovedNotification } - if !triggerCheck.IsMetricOnMaintenance(notification.Event.Metric) && !triggerCheck.IsTriggerOnMaintenance() { - return ValidNotification + if triggerCheck.IsMetricOnMaintenance(notification.Event.Metric) || triggerCheck.IsTriggerOnMaintenance() { + return ResavedNotification } - return ResavedNotification + return ValidNotification } // MatchedMetric represents parsed and matched metric data. @@ -535,6 +535,7 @@ type CheckData struct { Suppressed bool `json:"suppressed,omitempty" example:"true"` SuppressedState State `json:"suppressed_state,omitempty"` Message string `json:"msg,omitempty"` + Clock Clock `json:"-"` } // Need to not show the user metrics that should have been deleted due to ttlState = Del, @@ -559,7 +560,7 @@ func (checkData *CheckData) RemoveMetricsToTargetRelation() { // IsTriggerOnMaintenance checks if the trigger is on Maintenance. func (checkData *CheckData) IsTriggerOnMaintenance() bool { - return time.Now().Unix() <= checkData.Maintenance + return checkData.Clock.NowUnix() <= checkData.Maintenance } // IsMetricOnMaintenance checks if the metric of the given trigger is on Maintenance. @@ -573,7 +574,7 @@ func (checkData *CheckData) IsMetricOnMaintenance(metric string) bool { return false } - return time.Now().Unix() <= metricState.Maintenance + return checkData.Clock.NowUnix() <= metricState.Maintenance } // MetricState represents metric state data for given timestamp. @@ -780,11 +781,11 @@ func (event NotificationEvent) FormatTimestamp(location *time.Location, timeForm } // GetOrCreateMetricState gets metric state from check data or create new if CheckData has no state for given metric. -func (checkData *CheckData) GetOrCreateMetricState(metric string, emptyTimestampValue int64, muteNewMetric bool) MetricState { - _, ok := checkData.Metrics[metric] - if !ok { - checkData.Metrics[metric] = createEmptyMetricState(emptyTimestampValue, !muteNewMetric) +func (checkData *CheckData) GetOrCreateMetricState(metric string, muteFirstMetric bool, checkPointGap int64) MetricState { + if _, ok := checkData.Metrics[metric]; !ok { + checkData.Metrics[metric] = createEmptyMetricState(muteFirstMetric, checkPointGap, checkData.Clock) } + return checkData.Metrics[metric] } @@ -799,21 +800,19 @@ func (checkData *CheckData) GetMaintenance() (MaintenanceInfo, int64) { return checkData.MaintenanceInfo, checkData.Maintenance } -func createEmptyMetricState(defaultTimestampValue int64, firstStateIsNodata bool) MetricState { - if firstStateIsNodata { - return MetricState{ - State: StateNODATA, - Timestamp: defaultTimestampValue, - } +func createEmptyMetricState(muteFirstMetric bool, checkPointGap int64, clock Clock) MetricState { + metric := MetricState{ + Timestamp: clock.NowUnix(), + EventTimestamp: clock.NowUnix() - checkPointGap, } - unixNow := time.Now().Unix() - - return MetricState{ - State: StateOK, - Timestamp: unixNow, - EventTimestamp: unixNow, + if muteFirstMetric { + metric.State = StateOK + } else { + metric.State = StateNODATA } + + return metric } // GetCheckPoint gets check point for given MetricState. diff --git a/datatypes_test.go b/datatypes_test.go index 6f03438c7..757283c2f 100644 --- a/datatypes_test.go +++ b/datatypes_test.go @@ -5,7 +5,14 @@ import ( "testing" "time" + mock_clock "github.com/moira-alert/moira/mock/clock" . "github.com/smartystreets/goconvey/convey" + "go.uber.org/mock/gomock" +) + +const ( + checkPointGap = 120 + defaultMaintenance = 600 ) func TestIsScheduleAllows(t *testing.T) { @@ -298,6 +305,11 @@ func TestScheduledNotification_GetKey(t *testing.T) { } func TestScheduledNotification_GetState(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + Convey("Test get state of scheduled notifications", t, func() { notification := ScheduledNotification{ Event: NotificationEvent{ @@ -311,52 +323,93 @@ func TestScheduledNotification_GetState(t *testing.T) { }) Convey("Get Resaved state with metric on maintenance", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) + state := notification.GetState(&CheckData{ Metrics: map[string]MetricState{ "test": { - Maintenance: time.Now().Add(time.Hour).Unix(), + Maintenance: testTime + defaultMaintenance, }, }, + Clock: mockTime, }) + So(state, ShouldEqual, ResavedNotification) }) Convey("Get Resaved state with trigger on maintenance", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) + state := notification.GetState(&CheckData{ - Maintenance: time.Now().Add(time.Hour).Unix(), + Maintenance: testTime + defaultMaintenance, + Clock: mockTime, }) + So(state, ShouldEqual, ResavedNotification) }) Convey("Get Valid state with trigger without metrics", func() { - state := notification.GetState(&CheckData{}) + mockTime := mock_clock.NewMockClock(mockCtrl) + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) + + state := notification.GetState(&CheckData{ + Clock: mockTime, + }) + So(state, ShouldEqual, ValidNotification) }) Convey("Get Valid state with trigger with test metric", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + + mockTime.EXPECT().NowUnix().Return(testTime).Times(2) + state := notification.GetState(&CheckData{ Metrics: map[string]MetricState{ "test": {}, }, + Clock: mockTime, }) + So(state, ShouldEqual, ValidNotification) }) }) } func TestCheckData_GetOrCreateMetricState(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + Convey("Test no metric", t, func() { + mockClock := mock_clock.NewMockClock(mockCtrl) checkData := CheckData{ Metrics: make(map[string]MetricState), + Clock: mockClock, } - So(checkData.GetOrCreateMetricState("my.metric", 12343, false), ShouldResemble, MetricState{State: StateNODATA, Timestamp: 12343}) + + mockClock.EXPECT().NowUnix().Return(testTime).Times(2) + So(checkData.GetOrCreateMetricState("my.metric", false, checkPointGap), ShouldResemble, MetricState{State: StateNODATA, Timestamp: testTime, EventTimestamp: testTime - checkPointGap}) }) - Convey("Test no metric, notifyAboutNew = false", t, func() { + + Convey("Test no metric, mute new metric = true", t, func() { + mockClock := mock_clock.NewMockClock(mockCtrl) checkData := CheckData{ Metrics: make(map[string]MetricState), + Clock: mockClock, } - So(checkData.GetOrCreateMetricState("my.metric", 12343, true), ShouldResemble, MetricState{State: StateOK, Timestamp: time.Now().Unix(), EventTimestamp: time.Now().Unix()}) + + mockClock.EXPECT().NowUnix().Return(testTime).Times(2) + + So(checkData.GetOrCreateMetricState("my.metric", true, checkPointGap), ShouldResemble, MetricState{State: StateOK, Timestamp: testTime, EventTimestamp: testTime - checkPointGap}) }) + Convey("Test has metric", t, func() { metricState := MetricState{Timestamp: 11211} checkData := CheckData{ @@ -364,7 +417,8 @@ func TestCheckData_GetOrCreateMetricState(t *testing.T) { "my.metric": metricState, }, } - So(checkData.GetOrCreateMetricState("my.metric", 12343, false), ShouldResemble, metricState) + + So(checkData.GetOrCreateMetricState("my.metric", true, checkPointGap), ShouldResemble, metricState) }) } @@ -492,21 +546,33 @@ func TestTrigger_IsSimple(t *testing.T) { } func TestCheckData_IsTriggerOnMaintenance(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + Convey("IsTriggerOnMaintenance manipulations", t, func() { - checkData := &CheckData{ - Maintenance: time.Now().Add(time.Hour).Unix(), - } + Convey("Test with trigger check Maintenance equal with current time", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + checkData := &CheckData{ + Maintenance: testTime, + Clock: mockTime, + } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) - Convey("Test with trigger check Maintenance more than time now", func() { actual := checkData.IsTriggerOnMaintenance() So(actual, ShouldBeTrue) }) Convey("Test with trigger check Maintenance less than time now", func() { - checkData.Maintenance = time.Now().Add(-time.Hour).Unix() - defer func() { - checkData.Maintenance = time.Now().Add(time.Hour).Unix() - }() + mockTime := mock_clock.NewMockClock(mockCtrl) + checkData := &CheckData{ + Maintenance: testTime - defaultMaintenance, + Clock: mockTime, + } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) actual := checkData.IsTriggerOnMaintenance() So(actual, ShouldBeFalse) @@ -515,55 +581,82 @@ func TestCheckData_IsTriggerOnMaintenance(t *testing.T) { } func TestCheckData_IsMetricOnMaintenance(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + testTime := time.Date(2022, time.June, 6, 10, 0, 0, 0, time.UTC).Unix() + Convey("isMetricOnMaintenance manipulations", t, func() { - checkData := &CheckData{ - Metrics: map[string]MetricState{ - "test1": { - Maintenance: time.Now().Add(time.Hour).Unix(), + Convey("Test with a metric that is not in the trigger", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + checkData := &CheckData{ + Metrics: map[string]MetricState{ + "test1": {}, + "test2": {}, }, - "test2": {}, - }, - } + Clock: mockTime, + } - Convey("Test with a metric that is not in the trigger", func() { actual := checkData.IsMetricOnMaintenance("") So(actual, ShouldBeFalse) }) Convey("Test with metrics that are in the trigger but not on maintenance", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + checkData := &CheckData{ + Metrics: map[string]MetricState{ + "test1": {}, + "test2": {}, + }, + Clock: mockTime, + } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) + actual := checkData.IsMetricOnMaintenance("test2") So(actual, ShouldBeFalse) }) Convey("Test with metrics that are in the trigger and on maintenance", func() { + mockTime := mock_clock.NewMockClock(mockCtrl) + checkData := &CheckData{ + Metrics: map[string]MetricState{ + "test1": { + Maintenance: testTime + defaultMaintenance, + }, + "test2": {}, + }, + Clock: mockTime, + } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) + actual := checkData.IsMetricOnMaintenance("test1") So(actual, ShouldBeTrue) }) Convey("Test with the metric that is in the trigger, but the time now is more than Maintenance", func() { - metric := checkData.Metrics["test1"] - metric.Maintenance = time.Now().Add(-time.Hour).Unix() - checkData.Metrics["test1"] = metric - defer func() { - metric := checkData.Metrics["test1"] - metric.Maintenance = time.Now().Add(time.Hour).Unix() - checkData.Metrics["test1"] = metric - }() + mockTime := mock_clock.NewMockClock(mockCtrl) + checkData := &CheckData{ + Metrics: map[string]MetricState{ + "test1": { + Maintenance: testTime - defaultMaintenance, + }, + "test2": {}, + }, + Clock: mockTime, + } + + mockTime.EXPECT().NowUnix().Return(testTime).Times(1) actual := checkData.IsMetricOnMaintenance("test1") So(actual, ShouldBeFalse) }) Convey("Test with trigger without metrics", func() { - checkData.Metrics = make(map[string]MetricState) - defer func() { - checkData.Metrics = map[string]MetricState{ - "test1": { - Maintenance: time.Now().Add(time.Hour).Unix(), - }, - "test2": {}, - } - }() + checkData := &CheckData{ + Metrics: make(map[string]MetricState), + } actual := checkData.IsMetricOnMaintenance("test1") So(actual, ShouldBeFalse) diff --git a/filter/patterns_storage.go b/filter/patterns_storage.go index 2cfcd9dd8..950102b7b 100644 --- a/filter/patterns_storage.go +++ b/filter/patterns_storage.go @@ -114,7 +114,7 @@ func (storage *PatternStorage) ProcessIncomingMetric(lineBytes []byte, maxTTL ti return nil } - if parsedMetric.IsExpired(maxTTL, storage.clock.Now()) { + if parsedMetric.IsExpired(maxTTL, storage.clock.NowUTC()) { storage.logger.Debug(). String(moira.LogFieldNameMetricName, parsedMetric.Name). String(moira.LogFieldNameMetricTimestamp, fmt.Sprint(parsedMetric.Timestamp)). diff --git a/filter/patterns_storage_test.go b/filter/patterns_storage_test.go index 7caef651f..8c3bd76da 100644 --- a/filter/patterns_storage_test.go +++ b/filter/patterns_storage_test.go @@ -48,7 +48,7 @@ func TestProcessIncomingMetric(t *testing.T) { ) systemClock := mock_clock.NewMockClock(mockCtrl) - systemClock.EXPECT().Now().Return(time.Date(2009, 2, 13, 23, 31, 30, 0, time.UTC)).AnyTimes() + systemClock.EXPECT().NowUTC().Return(time.Date(2009, 2, 13, 23, 31, 30, 0, time.UTC)).AnyTimes() patternsStorage.clock = systemClock Convey("Create new pattern storage, should no error", t, func() { diff --git a/go.mod b/go.mod index ab02cbfde..91857a6eb 100644 --- a/go.mod +++ b/go.mod @@ -46,6 +46,7 @@ require ( require github.com/prometheus/common v0.37.0 require ( + github.com/golang/mock v1.6.0 github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/mattermost/mattermost/server/public v0.1.1 github.com/mitchellh/mapstructure v1.5.0 diff --git a/go.sum b/go.sum index 29ce7ac2f..d95d23b39 100644 --- a/go.sum +++ b/go.sum @@ -680,6 +680,7 @@ github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4= github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= diff --git a/interfaces.go b/interfaces.go index f3f1559ef..b6e4bcd18 100644 --- a/interfaces.go +++ b/interfaces.go @@ -228,5 +228,6 @@ type PlotTheme interface { // Clock is an interface to work with Time. type Clock interface { - Now() time.Time + NowUTC() time.Time + NowUnix() int64 } diff --git a/mock/clock/clock.go b/mock/clock/clock.go index 4aab4f192..8ed030da3 100644 --- a/mock/clock/clock.go +++ b/mock/clock/clock.go @@ -39,16 +39,30 @@ func (m *MockClock) EXPECT() *MockClockMockRecorder { return m.recorder } -// Now mocks base method. -func (m *MockClock) Now() time.Time { +// NowUTC mocks base method. +func (m *MockClock) NowUTC() time.Time { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Now") + ret := m.ctrl.Call(m, "NowUTC") ret0, _ := ret[0].(time.Time) return ret0 } -// Now indicates an expected call of Now. -func (mr *MockClockMockRecorder) Now() *gomock.Call { +// NowUTC indicates an expected call of NowUTC. +func (mr *MockClockMockRecorder) NowUTC() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Now", reflect.TypeOf((*MockClock)(nil).Now)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NowUTC", reflect.TypeOf((*MockClock)(nil).NowUTC)) +} + +// NowUnix mocks base method. +func (m *MockClock) NowUnix() int64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NowUnix") + ret0, _ := ret[0].(int64) + return ret0 +} + +// NowUnix indicates an expected call of NowUnix. +func (mr *MockClockMockRecorder) NowUnix() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NowUnix", reflect.TypeOf((*MockClock)(nil).NowUnix)) } diff --git a/notifier/events/event_test.go b/notifier/events/event_test.go index 219f03f60..4b79f4cbd 100644 --- a/notifier/events/event_test.go +++ b/notifier/events/event_test.go @@ -27,7 +27,7 @@ func TestEvent(t *testing.T) { scheduler := mock_scheduler.NewMockScheduler(mockCtrl) logger, _ := logging.GetLogger("Events") systemClock := mock_clock.NewMockClock(mockCtrl) - systemClock.EXPECT().Now().Return(time.Now()).AnyTimes() + systemClock.EXPECT().NowUTC().Return(time.Now()).AnyTimes() Convey("When event is TEST and subscription is disabled, should add new notification", t, func() { worker := FetchEventsWorker{ @@ -59,8 +59,8 @@ func TestEvent(t *testing.T) { SubscriptionID: event.SubscriptionID, }, SendFail: 0, - Timestamp: systemClock.Now().Unix(), - CreatedAt: systemClock.Now().Unix(), + Timestamp: systemClock.NowUTC().Unix(), + CreatedAt: systemClock.NowUTC().Unix(), Throttled: false, Contact: contact, } @@ -88,7 +88,7 @@ func TestEvent(t *testing.T) { } dataBase.EXPECT().GetContact(event.ContactID).Times(1).Return(contact, nil) dataBase.EXPECT().GetContact(contact.ID).Times(1).Return(contact, nil) - now := systemClock.Now() + now := systemClock.NowUTC() notification := moira.ScheduledNotification{ Event: moira.NotificationEvent{ TriggerID: "", diff --git a/notifier/scheduler.go b/notifier/scheduler.go index 8b69f0a35..d0348bc75 100644 --- a/notifier/scheduler.go +++ b/notifier/scheduler.go @@ -50,7 +50,7 @@ func (scheduler *StandardScheduler) ScheduleNotification(params moira.SchedulerP next time.Time throttled bool ) - now := scheduler.clock.Now() + now := scheduler.clock.NowUTC() if params.SendFail > 0 { next = now.Add(scheduler.config.ReschedulingDelay) throttled = params.ThrottledOld diff --git a/notifier/scheduler_test.go b/notifier/scheduler_test.go index cf1e75942..e148503bb 100644 --- a/notifier/scheduler_test.go +++ b/notifier/scheduler_test.go @@ -83,7 +83,7 @@ func TestThrottling(t *testing.T) { expected2 := expected expected2.SendFail = 1 expected2.Timestamp = now.Add(time.Minute).Unix() - systemClock.EXPECT().Now().Return(now).Times(1) + systemClock.EXPECT().NowUTC().Return(now).Times(1) notification := scheduler.ScheduleNotification(params2, logger) So(notification, ShouldResemble, &expected2) @@ -98,7 +98,7 @@ func TestThrottling(t *testing.T) { expected2.SendFail = 3 expected2.Timestamp = now.Add(time.Minute).Unix() expected2.Throttled = true - systemClock.EXPECT().Now().Return(now).Times(1) + systemClock.EXPECT().NowUTC().Return(now).Times(1) notification := scheduler.ScheduleNotification(params2, logger) So(notification, ShouldResemble, &expected2) @@ -119,7 +119,7 @@ func TestThrottling(t *testing.T) { expected3 := expected expected3.Event = testEvent - systemClock.EXPECT().Now().Return(now).Times(1) + systemClock.EXPECT().NowUTC().Return(now).Times(1) notification := scheduler.ScheduleNotification(params2, logger) So(notification, ShouldResemble, &expected3) @@ -130,7 +130,7 @@ func TestThrottling(t *testing.T) { dataBase.EXPECT().GetSubscription(*event.SubscriptionID).Times(1).Return(moira.SubscriptionData{}, fmt.Errorf("Error while read subscription")) params2 := params - systemClock.EXPECT().Now().Return(now).Times(1) + systemClock.EXPECT().NowUTC().Return(now).Times(1) notification := scheduler.ScheduleNotification(params2, logger) So(notification, ShouldResemble, &expected)