Merge pull request grafana#15444 from max-neverov/percent_diff_null

Fix percent_diff calculation when points are nulls
ryantxu · Feb 15, 2019 · 4c28ec8 · 4c28ec8
2 parents bc1aec9 + 28eaac3
commit 4c28ec8
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 45 deletions.
diff --git a/pkg/services/alerting/conditions/reducer.go b/pkg/services/alerting/conditions/reducer.go
@@ -95,52 +95,9 @@ 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
-			}
-		}
-		// get the oldest point
-		points = points[0:i]
-		for i := 0; i < len(points); i++ {
-			if points[i][0].Valid {
-				allNull = false
-				value = first - points[i][0].Float64
-				break
-			}
-		}
+		allNull, value = calculateDiff(series, allNull, value, diff)
 	case "percent_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
-			}
-		}
-		// 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
-				value = math.Abs(val)
-				break
-			}
-		}
+		allNull, value = calculateDiff(series, allNull, value, percentDiff)
 	case "count_non_null":
 		for _, v := range series.Points {
 			if v[0].Valid {
@@ -163,3 +120,40 @@ func (s *SimpleReducer) Reduce(series *tsdb.TimeSeries) null.Float {
 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
+		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
+		}
+	}
+	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 := fn(first, points[i][0].Float64)
+				value = math.Abs(val)
+				break
+			}
+		}
+	}
+	return allNull, value
+}
+
+var diff = func(newest, oldest float64) float64 {
+	return newest - oldest
+}
+
+var percentDiff = func(newest, oldest float64) float64 {
+	return (newest - oldest) / oldest * 100
+}
diff --git a/pkg/services/alerting/conditions/reducer_test.go b/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)
+		})
 	})
 }