fix(checker): fix nodata when toggling alone metrics checkbox (#983)

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
 }

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 {

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
 }

checker/metrics/conversion/alone_metrics.go

@@ -42,19 +42,26 @@ 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 {
 				step = firstMetric.StepTime
 			}
 			metric = *metricSource.MakeEmptyMetricData(metricName, step, from, to)
 		}
+
 		result[targetName] = metric
 	}
 

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)
+		})
 	})
 }

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
 }
 

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]
 }