IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Handle NaN in reducers (#22053)

Browse Source 
- Fix bug with NaN in alerting - Closes #21953
- Alert reducers (avg/max/etc) drop null values from their calculation. This change makes it so NaN values are handled in the same way as null values.
This commit is contained in:
Andrii Melnyk
2020-02-11 01:28:07 +01:00
committed by GitHub
parent 26e294c29b
commit 47314d0f13
2 changed files with 29 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
pkg/services/alerting/conditions/reducer.go
View File

@@ -29,7 +29,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
case "avg": case "avg":
validPointsCount := 0 validPointsCount := 0
for _, point := range series.Points { for _, point := range series.Points {
if point[0].Valid { if isValid(point[0]) {
value += point[0].Float64 value += point[0].Float64
validPointsCount++ validPointsCount++
allNull = false allNull = false
@@ -40,7 +40,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
} }
case "sum": case "sum":
for _, point := range series.Points { for _, point := range series.Points {
if point[0].Valid { if isValid(point[0]) {
value += point[0].Float64 value += point[0].Float64
allNull = false allNull = false
} }
@@ -48,7 +48,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
case "min": case "min":
value = math.MaxFloat64 value = math.MaxFloat64
for _, point := range series.Points { for _, point := range series.Points {
if point[0].Valid { if isValid(point[0]) {
allNull = false allNull = false
if value > point[0].Float64 { if value > point[0].Float64 {
value = point[0].Float64 value = point[0].Float64
@@ -58,7 +58,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
case "max": case "max":
value = -math.MaxFloat64 value = -math.MaxFloat64
for _, point := range series.Points { for _, point := range series.Points {
if point[0].Valid { if isValid(point[0]) {
allNull = false allNull = false
if value < point[0].Float64 { if value < point[0].Float64 {
value = point[0].Float64 value = point[0].Float64
@@ -71,7 +71,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
case "last": case "last":
points := series.Points points := series.Points
for i := len(points) - 1; i >= 0; i-- { for i := len(points) - 1; i >= 0; i-- {
if points[i][0].Valid { if isValid(points[i][0]) {
value = points[i][0].Float64 value = points[i][0].Float64
allNull = false allNull = false
break break
@@ -80,7 +80,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
case "median": case "median":
var values []float64 var values []float64
for _, v := range series.Points { for _, v := range series.Points {
if v[0].Valid { if isValid(v[0]) {
allNull = false allNull = false
values = append(values, v[0].Float64) values = append(values, v[0].Float64)
} }
@@ -100,7 +100,7 @@ func (s *queryReducer) Reduce(series *tsdb.TimeSeries) null.Float {
allNull, value = calculateDiff(series, allNull, value, percentDiff) allNull, value = calculateDiff(series, allNull, value, percentDiff)
case "count_non_null": case "count_non_null":
for _, v := range series.Points { for _, v := range series.Points {
if v[0].Valid { if isValid(v[0]) {
value++ value++
} }
} }
@@ -129,7 +129,7 @@ func calculateDiff(series *tsdb.TimeSeries, allNull bool, value float64, fn func
) )
// get the newest point // get the newest point
for i = len(points) - 1; i >= 0; i-- { for i = len(points) - 1; i >= 0; i-- {
if points[i][0].Valid { if isValid(points[i][0]) {
allNull = false allNull = false
first = points[i][0].Float64 first = points[i][0].Float64
break break
@@ -139,7 +139,7 @@ func calculateDiff(series *tsdb.TimeSeries, allNull bool, value float64, fn func
// get the oldest point // get the oldest point
points = points[0:i] points = points[0:i]
for i := 0; i < len(points); i++ { for i := 0; i < len(points); i++ {
if points[i][0].Valid { if isValid(points[i][0]) {
allNull = false allNull = false
val := fn(first, points[i][0].Float64) val := fn(first, points[i][0].Float64)
value = math.Abs(val) value = math.Abs(val)
@@ -150,6 +150,10 @@ func calculateDiff(series *tsdb.TimeSeries, allNull bool, value float64, fn func
return allNull, value return allNull, value
} }
func isValid(f null.Float) bool {
return f.Valid && !math.IsNaN(f.Float64)
}
var diff = func(newest, oldest float64) float64 { var diff = func(newest, oldest float64) float64 {
return newest - oldest return newest - oldest
} }

16
pkg/services/alerting/conditions/reducer_test.go
View File

@@ -1,6 +1,7 @@
package conditions package conditions
import ( import (
"math"
"testing" "testing"
. "github.com/smartystreets/goconvey/convey" . "github.com/smartystreets/goconvey/convey"
@@ -181,6 +182,21 @@ func TestSimpleReducer(t *testing.T) {
So(reducer.Reduce(series).Valid, ShouldEqual, false) So(reducer.Reduce(series).Valid, ShouldEqual, false)
}) })
Convey("min should work with NaNs", func() {
result := testReducer("min", math.NaN(), math.NaN(), math.NaN())
So(result, ShouldEqual, float64(0))
})
Convey("isValid should treat NaN as invalid", func() {
result := isValid(null.FloatFrom(math.NaN()))
So(result, ShouldBeFalse)
})
Convey("isValid should treat invalid null.Float as invalid", func() {
result := isValid(null.FloatFromPtr(nil))
So(result, ShouldBeFalse)
})
}) })
} }