Merge pull request #15444 from max-neverov/percent_diff_null

Fix percent_diff calculation when points are nulls

pkg/services/alerting/conditions/reducer.go

@@ -95,29 +95,33 @@ func (s *SimpleReducer) Reduce(series *tsdb.TimeSeries) null.Float {
 			}
 		}
 	case "diff":
-		var (
-			points = series.Points
-			first  float64
-			i      int
-		)
-		// get the newest point
-		for i = len(points) - 1; i >= 0; i-- {
-			if points[i][0].Valid {
-				allNull = false
-				first = points[i][0].Float64
-				break
+		allNull, value = calculateDiff(series, allNull, value, diff)
+	case "percent_diff":
+		allNull, value = calculateDiff(series, allNull, value, percentDiff)
+	case "count_non_null":
+		for _, v := range series.Points {
+			if v[0].Valid {
+				value++
 			}
 		}
-		// get the oldest point
-		points = points[0:i]
-		for i := 0; i < len(points); i++ {
-			if points[i][0].Valid {
+
+		if value > 0 {
 			allNull = false
-				value = first - points[i][0].Float64
-				break
 		}
 	}
-	case "percent_diff":
+
+	if allNull {
+		return null.FloatFromPtr(nil)
+	}
+
+	return null.FloatFrom(value)
+}
+
+func NewSimpleReducer(typ string) *SimpleReducer {
+	return &SimpleReducer{Type: typ}
+}
+
+func calculateDiff(series *tsdb.TimeSeries, allNull bool, value float64, fn func(float64, float64) float64) (bool, float64) {
 	var (
 		points = series.Points
 		first  float64
@@ -131,35 +135,25 @@ func (s *SimpleReducer) Reduce(series *tsdb.TimeSeries) null.Float {
 			break
 		}
 	}
+	if i >= 1 {
 		// get the oldest point
 		points = points[0:i]
 		for i := 0; i < len(points); i++ {
 			if points[i][0].Valid {
 				allNull = false
-				val := (first - points[i][0].Float64) / points[i][0].Float64 * 100
+				val := fn(first, points[i][0].Float64)
 				value = math.Abs(val)
 				break
 			}
 		}
-	case "count_non_null":
-		for _, v := range series.Points {
-			if v[0].Valid {
-				value++
-			}
-		}
-
-		if value > 0 {
-			allNull = false
-		}
-	}
-
-	if allNull {
-		return null.FloatFromPtr(nil)
 	}
+	return allNull, value
+}
 
-	return null.FloatFrom(value)
+var diff = func(newest, oldest float64) float64 {
+	return newest - oldest
 }
 
-func NewSimpleReducer(typ string) *SimpleReducer {
-	return &SimpleReducer{Type: typ}
+var percentDiff = func(newest, oldest float64) float64 {
+	return (newest - oldest) / oldest * 100
 }

pkg/services/alerting/conditions/reducer_test.go

@@ -143,6 +143,18 @@ func TestSimpleReducer(t *testing.T) {
 			So(result, ShouldEqual, float64(10))
 		})
 
+		Convey("diff with only nulls", func() {
+			reducer := NewSimpleReducer("diff")
+			series := &tsdb.TimeSeries{
+				Name: "test time serie",
+			}
+
+			series.Points = append(series.Points, tsdb.NewTimePoint(null.FloatFromPtr(nil), 1))
+			series.Points = append(series.Points, tsdb.NewTimePoint(null.FloatFromPtr(nil), 2))
+
+			So(reducer.Reduce(series).Valid, ShouldEqual, false)
+		})
+
 		Convey("percent_diff one point", func() {
 			result := testReducer("percent_diff", 40)
 			So(result, ShouldEqual, float64(0))
@@ -157,6 +169,18 @@ func TestSimpleReducer(t *testing.T) {
 			result := testReducer("percent_diff", 30, 40, 40)
 			So(result, ShouldEqual, float64(33.33333333333333))
 		})
+
+		Convey("percent_diff with only nulls", func() {
+			reducer := NewSimpleReducer("percent_diff")
+			series := &tsdb.TimeSeries{
+				Name: "test time serie",
+			}
+
+			series.Points = append(series.Points, tsdb.NewTimePoint(null.FloatFromPtr(nil), 1))
+			series.Points = append(series.Points, tsdb.NewTimePoint(null.FloatFromPtr(nil), 2))
+
+			So(reducer.Reduce(series).Valid, ShouldEqual, false)
+		})
 	})
 }