From 6a32bd03c0a03e56e1e9c8c19619967404eff976 Mon Sep 17 00:00:00 2001 From: Nikolay Date: Tue, 11 Aug 2020 14:02:32 +0500 Subject: [PATCH] fix: empty values in state * Added update of variables if they were missing --- checker/check.go | 68 ++++++++--------------- checker/fetch.go | 19 ------- checker/fetch_test.go | 4 -- database/redis/last_check.go | 8 ++- docker-compose.yml | 5 -- filter/patterns_storage.go | 3 -- metric_source/local/local.go | 10 ++-- metric_source/local/local_test.go | 89 ++++++++++++++++--------------- mock/moira-alert/database.go | 2 +- 9 files changed, 75 insertions(+), 133 deletions(-) diff --git a/checker/check.go b/checker/check.go index 5eb883e56..af9406e54 100644 --- a/checker/check.go +++ b/checker/check.go @@ -20,31 +20,21 @@ const ( func (triggerChecker *TriggerChecker) Check() error { passError := false checkData := newCheckData(triggerChecker.lastCheck, triggerChecker.until) - logIfNoValues(checkData, triggerChecker.logger, triggerChecker.triggerID, "newCheckData") triggerMetricsData, err := triggerChecker.fetchTriggerMetrics() if err != nil { return triggerChecker.handleFetchError(checkData, err) } - logIfTriggerKbaGateway(triggerChecker.logger, triggerChecker.triggerID, "fetchTriggerMetrics", triggerMetricsData) - preparedMetrics, aloneMetrics, err := triggerChecker.prepareMetrics(triggerMetricsData) if err != nil { - logIfTriggerKbaGateway(triggerChecker.logger, triggerChecker.triggerID, fmt.Sprintf("prepareMetrics ERROR: %#v", err), "") passError, checkData, err = triggerChecker.handlePrepareError(checkData, err) if !passError { return err } } - logIfTriggerKbaGateway(triggerChecker.logger, triggerChecker.triggerID, "preparedMetrics", preparedMetrics) - logIfTriggerKbaGateway(triggerChecker.logger, triggerChecker.triggerID, "aloneMetrics", aloneMetrics) - checkData.MetricsToTargetRelation = conversion.GetRelations(aloneMetrics) - logIfTriggerKbaGateway(triggerChecker.logger, triggerChecker.triggerID, "checkData.MetricsToTargetRelation", - checkData.MetricsToTargetRelation) checkData, err = triggerChecker.check(preparedMetrics, aloneMetrics, checkData) - logIfNoValues(checkData, triggerChecker.logger, triggerChecker.triggerID, "triggerChecker.check") if err != nil { return triggerChecker.handleUndefinedError(checkData, err) } @@ -60,42 +50,9 @@ func (triggerChecker *TriggerChecker) Check() error { } } checkData.UpdateScore() - logIfNoValues(checkData, triggerChecker.logger, triggerChecker.triggerID, "checkData.UpdateScore()") return triggerChecker.database.SetTriggerLastCheck(triggerChecker.triggerID, &checkData, triggerChecker.trigger.IsRemote) } -func logIfTriggerKbaGateway(logger moira.Logger, trigger, prefix string, Metrics interface{}) { - if trigger == "265cb2bf-e029-4df2-9836-b628c64a8373" { - data := "" - switch m := Metrics.(type) { - case map[string][]metricSource.MetricData: - data = fmt.Sprintf("%#v", m) - case map[string]map[string]metricSource.MetricData: - data = fmt.Sprintf("%#v", m) - case map[string]moira.MetricState: - data = fmt.Sprintf("%#v", m) - case moira.MetricState: - data = fmt.Sprintf("%#v", m) - case map[string]metricSource.MetricData: - data = fmt.Sprintf("%#v", m) - default: - data = fmt.Sprintf("default %#v", m) - } - - logger.Warningf(" FINDNOVALUES TRIGGER:%s, PREFIX:%s, DATA:%s", trigger, prefix, data) - } -} - -func logIfNoValues(data moira.CheckData, logger moira.Logger, trigger, prefix string) { - if len(data.Metrics) > 0 { - for target, metric := range data.Metrics { - if len(metric.Values) == 0 { - logIfTriggerKbaGateway(logger, trigger, prefix+" values no exists", target) - } - } - } -} - // handlePrepareError is a function that checks error returned from prepareMetrics function. If error // is not serious and check process can be continued first return value became true and Filled CheckData returned. // in the other case first return value became true and error passed to this function is handled. @@ -312,19 +269,27 @@ func (triggerChecker *TriggerChecker) checkTargets(metricName string, metrics ma if err != nil { return lastState, needToDeleteMetric, err } + for _, currentState := range metricStates { - lastState, err = triggerChecker.compareMetricStates(metricName, currentState, lastState) + state, err := triggerChecker.compareMetricStates(metricName, currentState, lastState) if err != nil { return lastState, needToDeleteMetric, err } + + if len(state.Values) > 0 { + lastState = state + } } + needToDeleteMetric, noDataState := triggerChecker.checkForNoData(metricName, lastState) if needToDeleteMetric { return lastState, needToDeleteMetric, err } + if noDataState != nil { lastState, err = triggerChecker.compareMetricStates(metricName, *noDataState, lastState) } + return lastState, needToDeleteMetric, err } @@ -370,22 +335,28 @@ func (triggerChecker *TriggerChecker) getMetricStepsStates(metricName string, me previousState := last difference := moira.MaxInt64(checkPoint-startTime, 0) stepsDifference := difference / stepTime + if (difference % stepTime) > 0 { stepsDifference++ } + valueTimestamp := startTime + stepTime*stepsDifference endTimestamp := triggerChecker.until + stepTime + for ; valueTimestamp < endTimestamp; valueTimestamp += stepTime { metricNewState, err := triggerChecker.getMetricDataState(&metricName, &metrics, &previousState, &valueTimestamp, &checkPoint) if err != nil { return last, current, err } + if metricNewState == nil { continue } + previousState = *metricNewState current = append(current, *metricNewState) } + return last, current, nil } @@ -393,7 +364,7 @@ func (triggerChecker *TriggerChecker) getMetricDataState(metricName *string, met if *valueTimestamp <= *checkPoint { return nil, nil } - triggerExpression, values, noEmptyValues := getExpressionValues(metrics, valueTimestamp) + triggerExpression, values, noEmptyValues := getExpressionValues(len(lastState.Values) == 0, metrics, valueTimestamp) if !noEmptyValues { return nil, nil } @@ -418,7 +389,7 @@ func (triggerChecker *TriggerChecker) getMetricDataState(metricName *string, met ), nil } -func getExpressionValues(metrics *map[string]metricSource.MetricData, valueTimestamp *int64) (*expression.TriggerExpression, map[string]float64, bool) { +func getExpressionValues(lastNoExistsValues bool, metrics *map[string]metricSource.MetricData, valueTimestamp *int64) (*expression.TriggerExpression, map[string]float64, bool) { expression := &expression.TriggerExpression{ AdditionalTargetsValues: make(map[string]float64, len(*metrics)-1), } @@ -428,10 +399,13 @@ func getExpressionValues(metrics *map[string]metricSource.MetricData, valueTimes targetName := fmt.Sprintf("t%d", i+1) metric := (*metrics)[targetName] value := metric.GetTimestampValue(*valueTimestamp) - values[targetName] = value if !moira.IsValidFloat64(value) { + if lastNoExistsValues { + continue + } return expression, values, false } + values[targetName] = value if i == 0 { expression.MainTargetValue = value continue diff --git a/checker/fetch.go b/checker/fetch.go index d97f0fbd6..f30d9399f 100644 --- a/checker/fetch.go +++ b/checker/fetch.go @@ -35,25 +35,11 @@ func (triggerChecker *TriggerChecker) fetch() (map[string][]metricSource.MetricD targetIndex++ // increasing target index to have target names started from 1 instead of 0 fetchResult, err := triggerChecker.source.Fetch(target, triggerChecker.from, triggerChecker.until, isSimpleTrigger) if err != nil { - id := "" - if triggerChecker.trigger != nil { - id = triggerChecker.trigger.ID - } - triggerChecker.logger.Warningf("NOVARIABLES triggerChecker.source.Fetch ID: %s, ERROR: %v, ", - id, err) return nil, nil, err } metricsData := fetchResult.GetMetricsData() metricsFetchResult, metricsErr := fetchResult.GetPatternMetrics() - if metricsErr != nil { - id := "" - if triggerChecker.trigger != nil { - id = triggerChecker.trigger.ID - } - triggerChecker.logger.Warningf("NOVARIABLES GetPatternMetrics ID: %s, ERROR: %v, ", - id, metricsErr) - } if metricsErr == nil { metricsArr = append(metricsArr, metricsFetchResult...) @@ -62,11 +48,6 @@ func (triggerChecker *TriggerChecker) fetch() (map[string][]metricSource.MetricD targetName := fmt.Sprintf("t%d", targetIndex) triggerMetricsData[targetName] = metricsData } - - if triggerChecker.trigger.ID == "265cb2bf-e029-4df2-9836-b628c64a8373" { - triggerChecker.logger.Warningf("NOVARIABLES triggerMetricsData: %#v, ",triggerMetricsData) - } - return triggerMetricsData, metricsArr, nil } diff --git a/checker/fetch_test.go b/checker/fetch_test.go index 2d5d183f9..512e8da72 100644 --- a/checker/fetch_test.go +++ b/checker/fetch_test.go @@ -4,8 +4,6 @@ import ( "fmt" "testing" - "github.com/op/go-logging" - "github.com/golang/mock/gomock" "github.com/moira-alert/moira" metricSource "github.com/moira-alert/moira/metric_source" @@ -100,7 +98,6 @@ func TestFetchTriggerMetrics(t *testing.T) { func TestFetch(t *testing.T) { mockCtrl := gomock.NewController(t) dataBase := mock_moira_alert.NewMockDatabase(mockCtrl) - logger, _ := logging.GetLogger("Test") source := mock_metric_source.NewMockMetricSource(mockCtrl) fetchResult := mock_metric_source.NewMockFetchResult(mockCtrl) defer mockCtrl.Finish() @@ -121,7 +118,6 @@ func TestFetch(t *testing.T) { source: source, from: from, until: until, - logger: logger, trigger: &moira.Trigger{ Targets: []string{pattern}, Patterns: []string{pattern}, diff --git a/database/redis/last_check.go b/database/redis/last_check.go index dc46f5f97..b3c7aa11f 100644 --- a/database/redis/last_check.go +++ b/database/redis/last_check.go @@ -95,7 +95,7 @@ func (connector *DbConnector) SetTriggerCheckMaintenance(triggerID string, metri return readingErr } for readingErr != redis.ErrNil { - lastCheck := moira.CheckData{} + var lastCheck = moira.CheckData{} err := json.Unmarshal([]byte(lastCheckString), &lastCheck) if err != nil { return fmt.Errorf("failed to parse lastCheck json %s: %s", lastCheckString, err.Error()) @@ -145,10 +145,8 @@ func (connector *DbConnector) checkDataScoreChanged(triggerID string, checkData return oldScore != checkData.Score } -var ( - badStateTriggersKey = "moira-bad-state-triggers" - triggersChecksKey = "moira-triggers-checks" -) +var badStateTriggersKey = "moira-bad-state-triggers" +var triggersChecksKey = "moira-triggers-checks" func metricLastCheckKey(triggerID string) string { return "moira-metric-last-check:" + triggerID diff --git a/docker-compose.yml b/docker-compose.yml index f5d9710e0..724cb5b93 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -114,8 +114,3 @@ networks: balancer: volumes: data: - driver: local - driver_opts: - type: 'none' - o: 'bind' - device: '/home/roo/data/' diff --git a/filter/patterns_storage.go b/filter/patterns_storage.go index c73c17bcb..f3497fc1d 100644 --- a/filter/patterns_storage.go +++ b/filter/patterns_storage.go @@ -1,7 +1,6 @@ package filter import ( - "fmt" "sync/atomic" "time" @@ -70,8 +69,6 @@ func (storage *PatternStorage) ProcessIncomingMetric(lineBytes []byte) *moira.Ma if count%10 == 0 { storage.metrics.MatchingTimer.UpdateSince(matchingStart) } - - fmt.Printf("FILTERCHECK: %#v\n", matchedPatterns) if len(matchedPatterns) > 0 { storage.metrics.MatchingMetricsReceived.Inc() return &moira.MatchedMetric{ diff --git a/metric_source/local/local.go b/metric_source/local/local.go index d26aab6b1..8e59c2897 100644 --- a/metric_source/local/local.go +++ b/metric_source/local/local.go @@ -45,15 +45,14 @@ func (local *Local) Fetch(target string, from int64, until int64, allowRealTimeA expr2, _, err := parser.ParseExpr(target) if err != nil { return nil, ErrParseExpr{ - internalError: fmt.Errorf("parser.ParseExpr %v", err), + internalError: err, target: target, } } patterns := expr2.Metrics() metricsMap, metrics, err := getPatternsMetricData(local.dataBase, patterns, from, until, allowRealTimeAlerting) if err != nil { - //return nil, err - return nil, fmt.Errorf("getPatternsMetricData: %v", err) + return nil, err } rewritten, newTargets, err := expr.RewriteExpr(expr2, from, until, metricsMap) if err != nil && err != parser.ErrSeriesDoesNotExist { @@ -77,15 +76,14 @@ func (local *Local) Fetch(target string, from int64, until int64, allowRealTimeA } else { err = ErrEvalExpr{ target: target, - internalError: fmt.Errorf("ErrEvalExpr %v", err), + internalError: err, } } } return result, err }() if err != nil { - //return nil, err - return nil, fmt.Errorf("metricsData: %v", err) + return nil, err } for _, metricData := range metricsData { md := *metricData diff --git a/metric_source/local/local_test.go b/metric_source/local/local_test.go index 948fd6393..c6dfc9c84 100644 --- a/metric_source/local/local_test.go +++ b/metric_source/local/local_test.go @@ -1,9 +1,12 @@ package local import ( + "fmt" + "strings" "testing" "github.com/go-graphite/carbonapi/expr/functions" + "github.com/go-graphite/carbonapi/pkg/parser" "github.com/golang/mock/gomock" "github.com/moira-alert/moira" metricSource "github.com/moira-alert/moira/metric_source" @@ -21,7 +24,7 @@ func TestEvaluateTarget(t *testing.T) { localSource := Create(dataBase) defer mockCtrl.Finish() - //pattern := "super.puper.pattern" + pattern := "super.puper.pattern" metric := "super.puper.metric" dataList := map[string][]*moira.MetricValue{ metric: { @@ -57,50 +60,50 @@ func TestEvaluateTarget(t *testing.T) { var until int64 = 67 var retention int64 = 10 var metricsTTL int64 = 3600 - //metricErr := fmt.Errorf("Ooops, metric error") + metricErr := fmt.Errorf("Ooops, metric error") Convey("Errors tests", t, func() { - //Convey("Error while ParseExpr", func() { - // dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) - // result, err := localSource.Fetch("", from, until, true) - // So(err, ShouldResemble, ErrParseExpr{target: "", internalError: parser.ErrMissingExpr}) - // So(err.Error(), ShouldResemble, "failed to parse target '': missing expression") - // So(result, ShouldBeNil) - //}) - - //Convey("Error in fetch data", func() { - // dataBase.EXPECT().AllowStale().Return(dataBase) - // dataBase.EXPECT().GetPatternMetrics(pattern).Return([]string{metric}, nil) - // dataBase.EXPECT().GetMetricRetention(metric).Return(retention, nil) - // dataBase.EXPECT().GetMetricsValues([]string{metric}, from, until).Return(nil, metricErr) - // dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) - // result, err := localSource.Fetch("super.puper.pattern", from, until, true) - // So(err, ShouldResemble, metricErr) - // So(result, ShouldBeNil) - //}) - - //Convey("Error evaluate target", func() { - // dataBase.EXPECT().AllowStale().Return(dataBase) - // dataBase.EXPECT().GetPatternMetrics("super.puper.pattern").Return([]string{metric}, nil) - // dataBase.EXPECT().GetMetricRetention(metric).Return(retention, nil) - // dataBase.EXPECT().GetMetricsValues([]string{metric}, from, until).Return(dataList, nil) - // dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) - // result, err := localSource.Fetch("aliasByNoe(super.puper.pattern, 2)", from, until, true) - // So(err.Error(), ShouldResemble, "Unknown graphite function: \"aliasByNoe\"") - // So(result, ShouldBeNil) - //}) - // - //Convey("Panic while evaluate target", func() { - // dataBase.EXPECT().AllowStale().Return(dataBase) - // dataBase.EXPECT().GetPatternMetrics("super.puper.pattern").Return([]string{metric}, nil) - // dataBase.EXPECT().GetMetricRetention(metric).Return(retention, nil) - // dataBase.EXPECT().GetMetricsValues([]string{metric}, from, until).Return(dataList, nil) - // dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) - // result, err := localSource.Fetch("movingAverage(super.puper.pattern, -1)", from, until, true) - // expectedErrSubstring := strings.Split(ErrEvaluateTargetFailedWithPanic{target: "movingAverage(super.puper.pattern, -1)"}.Error(), ":")[0] - // So(err.Error(), ShouldStartWith, expectedErrSubstring) - // So(result, ShouldBeNil) - //}) + Convey("Error while ParseExpr", func() { + dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) + result, err := localSource.Fetch("", from, until, true) + So(err, ShouldResemble, ErrParseExpr{target: "", internalError: parser.ErrMissingExpr}) + So(err.Error(), ShouldResemble, "failed to parse target '': missing expression") + So(result, ShouldBeNil) + }) + + Convey("Error in fetch data", func() { + dataBase.EXPECT().AllowStale().Return(dataBase) + dataBase.EXPECT().GetPatternMetrics(pattern).Return([]string{metric}, nil) + dataBase.EXPECT().GetMetricRetention(metric).Return(retention, nil) + dataBase.EXPECT().GetMetricsValues([]string{metric}, from, until).Return(nil, metricErr) + dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) + result, err := localSource.Fetch("super.puper.pattern", from, until, true) + So(err, ShouldResemble, metricErr) + So(result, ShouldBeNil) + }) + + Convey("Error evaluate target", func() { + dataBase.EXPECT().AllowStale().Return(dataBase) + dataBase.EXPECT().GetPatternMetrics("super.puper.pattern").Return([]string{metric}, nil) + dataBase.EXPECT().GetMetricRetention(metric).Return(retention, nil) + dataBase.EXPECT().GetMetricsValues([]string{metric}, from, until).Return(dataList, nil) + dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) + result, err := localSource.Fetch("aliasByNoe(super.puper.pattern, 2)", from, until, true) + So(err.Error(), ShouldResemble, "Unknown graphite function: \"aliasByNoe\"") + So(result, ShouldBeNil) + }) + + Convey("Panic while evaluate target", func() { + dataBase.EXPECT().AllowStale().Return(dataBase) + dataBase.EXPECT().GetPatternMetrics("super.puper.pattern").Return([]string{metric}, nil) + dataBase.EXPECT().GetMetricRetention(metric).Return(retention, nil) + dataBase.EXPECT().GetMetricsValues([]string{metric}, from, until).Return(dataList, nil) + dataBase.EXPECT().GetMetricsTTLSeconds().Return(metricsTTL) + result, err := localSource.Fetch("movingAverage(super.puper.pattern, -1)", from, until, true) + expectedErrSubstring := strings.Split(ErrEvaluateTargetFailedWithPanic{target: "movingAverage(super.puper.pattern, -1)"}.Error(), ":")[0] + So(err.Error(), ShouldStartWith, expectedErrSubstring) + So(result, ShouldBeNil) + }) }) Convey("Test no metrics", t, func() { diff --git a/mock/moira-alert/database.go b/mock/moira-alert/database.go index a8ca40816..a153b12ed 100644 --- a/mock/moira-alert/database.go +++ b/mock/moira-alert/database.go @@ -1083,7 +1083,7 @@ func (mr *MockDatabaseMockRecorder) SaveContact(arg0 interface{}) *gomock.Call { // SaveMetrics mocks base method func (m *MockDatabase) SaveMetrics(arg0 map[string]*moira.MatchedMetric) error { - fmt.Printf("SaveMetrics %#v\n", arg0) + fmt.Printf("SaveMetrics-CHECK: %#v\n", arg0) m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SaveMetrics", arg0) ret0, _ := ret[0].(error)