diff --git a/api/controller/trigger_metrics.go b/api/controller/trigger_metrics.go index 914460efe..5a2007dfa 100644 --- a/api/controller/trigger_metrics.go +++ b/api/controller/trigger_metrics.go @@ -110,12 +110,15 @@ func deleteTriggerMetrics(dataBase moira.Database, metricName string, triggerID delete(lastCheck.Metrics, metricName) } } + lastCheck.UpdateScore() if err = dataBase.RemovePatternsMetrics(trigger.Patterns); err != nil { return api.ErrorInternalServer(err) } + if err = dataBase.SetTriggerLastCheck(triggerID, &lastCheck, trigger.TriggerSource); err != nil { return api.ErrorInternalServer(err) } + return nil } diff --git a/api/dto/triggers.go b/api/dto/triggers.go index 78755b53f..7e8a0d0d6 100644 --- a/api/dto/triggers.go +++ b/api/dto/triggers.go @@ -142,27 +142,34 @@ func (trigger *Trigger) Bind(request *http.Request) error { if len(trigger.Targets) == 0 { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("targets is required")} } + if len(trigger.Tags) == 0 { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("tags is required")} } + if trigger.Name == "" { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("trigger name is required")} } + if err := checkWarnErrorExpression(trigger); err != nil { return api.ErrInvalidRequestContent{ValidationError: err} } + if len(trigger.Targets) <= 1 { // we should have empty alone metrics dictionary when there is only one target trigger.AloneMetrics = map[string]bool{} } + for targetName := range trigger.AloneMetrics { if !targetNameRegex.MatchString(targetName) { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("alone metrics target name should be in pattern: t\\d+")} } + targetIndexStr := targetNameRegex.FindStringSubmatch(targetName)[1] targetIndex, err := strconv.Atoi(targetIndexStr) if err != nil { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("alone metrics target index should be valid number: %w", err)} } + if targetIndex < 0 || targetIndex > len(trigger.Targets) { return api.ErrInvalidRequestContent{ValidationError: fmt.Errorf("alone metrics target index should be in range from 1 to length of targets")} } @@ -193,6 +200,7 @@ func (trigger *Trigger) Bind(request *http.Request) error { if err != nil { return err } + // TODO(litleleprikon): Remove after https://github.com/moira-alert/moira/issues/550 will be resolved for _, pattern := range trigger.Patterns { if pattern == asteriskPattern { diff --git a/checker/check.go b/checker/check.go index c04dbb3b8..6ad4db0b0 100644 --- a/checker/check.go +++ b/checker/check.go @@ -107,6 +107,7 @@ func (triggerChecker *TriggerChecker) handlePrepareError(checkData moira.CheckDa &checkData, triggerChecker.trigger.TriggerSource, ) + return MustStopCheck, checkData, err } @@ -252,7 +253,6 @@ func (triggerChecker *TriggerChecker) prepareMetrics(fetchedMetrics map[string][ } multiMetricTargets, aloneMetrics, err := preparedPatternMetrics.FilterAloneMetrics(triggerChecker.trigger.AloneMetrics) - if err != nil { return nil, nil, err } @@ -268,6 +268,7 @@ func (triggerChecker *TriggerChecker) prepareMetrics(fetchedMetrics map[string][ if len(duplicates) > 0 { return converted, populatedAloneMetrics, NewErrTriggerHasSameMetricNames(duplicates) } + return converted, populatedAloneMetrics, nil } @@ -319,7 +320,10 @@ func (triggerChecker *TriggerChecker) check( metricState, needToDeleteMetric, err := triggerChecker.checkTargets(metricName, targets, log) if needToDeleteMetric { - log.Debug().String("metric_name", metricName).Msg("Remove metric") + log.Debug(). + String("metric_name", metricName). + Msg("Remove metric") + checkData.RemoveMetricState(metricName) err = triggerChecker.database.RemovePatternsMetrics(triggerChecker.trigger.Patterns) } else { @@ -509,6 +513,7 @@ func getExpressionValues(metrics map[string]metricSource.MetricData, valueTimest expression := &expression.TriggerExpression{ AdditionalTargetsValues: make(map[string]float64, len(metrics)-1), } + values = make(map[string]float64, len(metrics)) for i := 0; i < len(metrics); i++ { @@ -521,11 +526,14 @@ func getExpressionValues(metrics map[string]metricSource.MetricData, valueTimest if !moira.IsFiniteNumber(value) { return expression, values, false } + if i == 0 { expression.MainTargetValue = value continue } + expression.AdditionalTargetsValues[targetName] = value } + return expression, values, true } diff --git a/checker/metrics/conversion/alone_metrics.go b/checker/metrics/conversion/alone_metrics.go index 16a13edca..71e5b0fce 100644 --- a/checker/metrics/conversion/alone_metrics.go +++ b/checker/metrics/conversion/alone_metrics.go @@ -42,12 +42,18 @@ func (m AloneMetrics) Populate(lastCheckMetricsToTargetRelation map[string]strin break } - for targetName := range declaredAloneMetrics { + for targetName, isAlone := range declaredAloneMetrics { + // We don't need to populate metrics from not alone targets + if !isAlone { + continue + } + metricName, existInLastCheck := lastCheckMetricsToTargetRelation[targetName] metric, existInCurrentAloneMetrics := m[targetName] if !existInCurrentAloneMetrics && !existInLastCheck { return AloneMetrics{}, NewErrEmptyAloneMetricsTarget(targetName) } + if !existInCurrentAloneMetrics { step := defaultStep if len(m) > 0 && firstMetric.StepTime != 0 { @@ -55,6 +61,7 @@ func (m AloneMetrics) Populate(lastCheckMetricsToTargetRelation map[string]strin } metric = *metricSource.MakeEmptyMetricData(metricName, step, from, to) } + result[targetName] = metric } diff --git a/checker/metrics/conversion/alone_metrics_test.go b/checker/metrics/conversion/alone_metrics_test.go index 8f9caaea5..3f361b176 100644 --- a/checker/metrics/conversion/alone_metrics_test.go +++ b/checker/metrics/conversion/alone_metrics_test.go @@ -44,6 +44,7 @@ func TestAloneMetrics_Populate(t *testing.T) { So(populated["t2"].StepTime, ShouldResemble, int64(60)) So(populated["t2"].Values, ShouldHaveLength, 60) }) + Convey("Missing one metric and no other metrics provided. Use default values", func() { m := AloneMetrics{} lastCheckMetricsToTargetRelation := map[string]string{ @@ -54,6 +55,7 @@ func TestAloneMetrics_Populate(t *testing.T) { } const from = 0 const to = 3600 + populated, err := m.Populate(lastCheckMetricsToTargetRelation, declaredAloneMetrics, from, to) So(err, ShouldBeNil) // We assume alone metrics to be like this @@ -68,6 +70,7 @@ func TestAloneMetrics_Populate(t *testing.T) { So(populated["t1"].StepTime, ShouldResemble, int64(60)) So(populated["t1"].Values, ShouldHaveLength, 60) }) + Convey("One declared alone metrics target do not have metrics and metrics in last check", func() { m := AloneMetrics{} lastCheckMetricsToTargetRelation := map[string]string{} @@ -76,9 +79,33 @@ func TestAloneMetrics_Populate(t *testing.T) { } const from = 0 const to = 3600 + populated, err := m.Populate(lastCheckMetricsToTargetRelation, declaredAloneMetrics, from, to) So(err, ShouldResemble, ErrEmptyAloneMetricsTarget{targetName: "t1"}) So(populated, ShouldResemble, AloneMetrics{}) }) + + Convey("Checking that we're not trying to populate metrics from not alone targets that are in declaredAloneMetrics", func() { + m := AloneMetrics{} + lastCheckMetricsToTargetRelation := map[string]string{ + "t2": "metric.test.2", + } + declaredAloneMetrics := map[string]bool{ + "t1": false, + "t2": true, + "t3": false, + } + const from = 0 + const to = 3600 + + populated, err := m.Populate(lastCheckMetricsToTargetRelation, declaredAloneMetrics, from, to) + So(err, ShouldBeNil) + So(populated, ShouldHaveLength, 1) + So(populated, ShouldContainKey, "t2") + So(populated["t2"].StartTime, ShouldResemble, int64(0)) + So(populated["t2"].StopTime, ShouldResemble, int64(3600)) + So(populated["t2"].StepTime, ShouldResemble, int64(60)) + So(populated["t2"].Values, ShouldHaveLength, 60) + }) }) } diff --git a/checker/metrics/conversion/trigger_metrics.go b/checker/metrics/conversion/trigger_metrics.go index deeaef8f9..594cd3324 100644 --- a/checker/metrics/conversion/trigger_metrics.go +++ b/checker/metrics/conversion/trigger_metrics.go @@ -109,6 +109,7 @@ func (triggerMetrics TriggerMetrics) Populate(lastMetrics map[string]moira.Metri targetMetrics = targetMetrics.Populate(metrics, from, to) result[targetName] = targetMetrics } + return result } @@ -166,11 +167,14 @@ func (triggerMetrics TriggerMetrics) FilterAloneMetrics(declaredAloneMetrics map errorBuilder.addUnexpected(targetName, targetMetrics) continue } + aloneMetrics[targetName] = targetMetrics[metricName] } + if err := errorBuilder.build(); err != nil { return TriggerMetrics{}, AloneMetrics{}, err } + return result, aloneMetrics, nil } diff --git a/metric_source/metric_data.go b/metric_source/metric_data.go index a70b49804..10a27bd3f 100644 --- a/metric_source/metric_data.go +++ b/metric_source/metric_data.go @@ -53,6 +53,7 @@ func (metricData *MetricData) GetTimestampValue(valueTimestamp int64) float64 { if len(metricData.Values) <= valueIndex { return math.NaN() } + return metricData.Values[valueIndex] }