Alerting: Fix mathexp.NoData in ConditionsCmd (#56812)

This commit fixes an issue where mathexp.NoData would return an error
in ConditionsCmd (Classic Condition) instead of no data. It further
refactors the Execute method to make it easier to understand.

pkg/expr/classic/classic.go

@@ -69,104 +69,106 @@ func (cmd *ConditionsCmd) NeedsVars() []string {
 // Execute runs the command and returns the results or an error if the command
 // failed to execute.
 func (cmd *ConditionsCmd) Execute(_ context.Context, _ time.Time, vars mathexp.Vars) (mathexp.Results, error) {
-	firing := true
+	// isFiring and isNoData tracks whether ConditionsCmd is firing or no data
-	newRes := mathexp.Results{}
+	var isFiring, isNoData bool
-	noDataFound := true
+	var res mathexp.Results
-
+
-	matches := []EvalMatch{}
+	matches := make([]EvalMatch, 0)
-
+	for ix, cond := range cmd.Conditions {
-	for i, c := range cmd.Conditions {
+		// isCondFiring and isCondNoData tracks whether the condition is firing or no data
-		querySeriesSet := vars[c.InputRefID]
+		//
-		nilReducedCount := 0
+		// There are a number of reasons a condition can have no data:
-		firingCount := 0
+		//
-		for _, val := range querySeriesSet.Values {
+		//  1. The input data vars[cond.InputRefID] has no values
-			var reducedNum mathexp.Number
+		//  2. The input data has one or more values, however all are mathexp.NoData
-			var name string
+		//  3. The input data has one or more values of mathexp.Number or mathexp.Series,
-			switch v := val.(type) {
+		//     however the either all mathexp.Number have a nil float64 or the reduce function
+		//     for all mathexp.Series returns a mathexp.Number with a nil float64
+		//  4. The input data is a combination of all mathexp.NoData, mathexp.Number with a nil
+		//     float64, or mathexp.Series that reduce to a nil float64
+		var isCondFiring, isCondNoData bool
+		var numSeriesNoData int
+
+		series := vars[cond.InputRefID]
+		for _, value := range series.Values {
+			var (
+				name   string
+				number mathexp.Number
+			)
+			switch v := value.(type) {
 			case mathexp.NoData:
+				// Reduce expressions return v.New(), however classic conditions use the operator
+				// in the condition to determine if the outcome of ConditionsCmd is no data.
 				// To keep this code as simple as possible we translate mathexp.NoData into a
 				// mathexp.Number with a nil value so number.GetFloat64Value() returns nil
-				reducedNum = mathexp.NewNumber("no data", nil)
+				number = mathexp.NewNumber("no data", nil)
-				reducedNum.SetValue(nil)
+				number.SetValue(nil)
-			case mathexp.Series:
-				reducedNum = c.Reducer.Reduce(v)
-				name = v.GetName()
 			case mathexp.Number:
-				reducedNum = v
 				if len(v.Frame.Fields) > 0 {
 					name = v.Frame.Fields[0].Name
 				}
+				number = v
+			case mathexp.Series:
+				name = v.GetName()
+				number = cond.Reducer.Reduce(v)
 			default:
-				return newRes, fmt.Errorf("can only reduce type series, got type %v", val.Type())
+				return res, fmt.Errorf("can only reduce type series, got type %v", v.Type())
 			}
 
-			// TODO handle error / no data signals
+			// Check if the value was either a mathexp.NoData, a mathexp.Number with a nil float64,
-			thisCondNoDataFound := reducedNum.GetFloat64Value() == nil
+			// or mathexp.Series that reduced to a nil float64
-
+			if number.GetFloat64Value() == nil {
-			if thisCondNoDataFound {
+				numSeriesNoData += 1
-				nilReducedCount++
+			} else if isValueFiring := cond.Evaluator.Eval(number); isValueFiring {
-			}
+				isCondFiring = true
-
+				// If the condition is met then add it to the list of matching conditions
-			evalRes := c.Evaluator.Eval(reducedNum)
+				labels := number.GetLabels()
-
+				if labels != nil {
-			if evalRes {
+					labels = labels.Copy()
-				match := EvalMatch{
-					Value:  reducedNum.GetFloat64Value(),
-					Metric: name,
 				}
-				if reducedNum.GetLabels() != nil {
+				matches = append(matches, EvalMatch{
-					match.Labels = reducedNum.GetLabels().Copy()
+					Metric: name,
-				}
+					Value:  number.GetFloat64Value(),
-				matches = append(matches, match)
+					Labels: labels,
-				firingCount++
+				})
 			}
 		}
 
-		thisCondFiring := firingCount > 0
+		// The condition is no data iff all the input data is a combination of all mathexp.NoData,
-		thisCondNoData := len(querySeriesSet.Values) == nilReducedCount
+		// mathexp.Number with a nil loat64, or mathexp.Series that reduce to a nil float64
-
+		isCondNoData = numSeriesNoData == len(series.Values)
-		if i == 0 {
+		if isCondNoData {
-			firing = thisCondFiring
-			noDataFound = thisCondNoData
-		}
-
-		if c.Operator == "or" {
-			firing = firing || thisCondFiring
-			noDataFound = noDataFound || thisCondNoData
-		} else {
-			firing = firing && thisCondFiring
-			noDataFound = noDataFound && thisCondNoData
-		}
-
-		if thisCondNoData {
 			matches = append(matches, EvalMatch{
 				Metric: "NoData",
 			})
-			noDataFound = true
 		}
 
-		firingCount = 0
+		if ix == 0 {
-		nilReducedCount = 0
+			isFiring = isCondFiring
+			isNoData = isCondNoData
+		} else if cond.Operator == "or" {
+			isFiring = isFiring || isCondFiring
+			isNoData = isNoData || isCondNoData
+		} else {
+			isFiring = isFiring && isCondFiring
+			isNoData = isNoData && isCondNoData
+		}
 	}
 
-	num := mathexp.NewNumber("", nil)
-
-	num.SetMeta(matches)
-
 	var v float64
-	switch {
+	number := mathexp.NewNumber("", nil)
-	case noDataFound:
+	number.SetMeta(matches)
-		num.SetValue(nil)
+	if isFiring {
-	case firing:
 		v = 1
-		num.SetValue(&v)
+		number.SetValue(&v)
-	case !firing:
+	} else if isNoData {
-		num.SetValue(&v)
+		number.SetValue(nil)
+	} else {
+		number.SetValue(&v)
 	}
 
-	newRes.Values = append(newRes.Values, num)
+	res.Values = append(res.Values, number)
-
+	return res, nil
-	return newRes, nil
 }
 
 // EvalMatch represents the series violating the threshold.