From 3c2f6094b2a90676558f4c4c58d2871bedd2b0df Mon Sep 17 00:00:00 2001 From: Maxim Neverov Date: Thu, 14 Feb 2019 16:31:15 +0100 Subject: [PATCH 1/2] Fix percent_diff calculation when points are nulls --- pkg/services/alerting/conditions/reducer.go | 19 +++++++++++-------- .../alerting/conditions/reducer_test.go | 12 ++++++++++++ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/pkg/services/alerting/conditions/reducer.go b/pkg/services/alerting/conditions/reducer.go index 1e8ae792746..485ea43561b 100644 --- a/pkg/services/alerting/conditions/reducer.go +++ b/pkg/services/alerting/conditions/reducer.go @@ -131,14 +131,17 @@ func (s *SimpleReducer) Reduce(series *tsdb.TimeSeries) null.Float { 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 + + 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 + value = math.Abs(val) + break + } } } case "count_non_null": diff --git a/pkg/services/alerting/conditions/reducer_test.go b/pkg/services/alerting/conditions/reducer_test.go index 7f11fc498bd..70b03f1dba8 100644 --- a/pkg/services/alerting/conditions/reducer_test.go +++ b/pkg/services/alerting/conditions/reducer_test.go @@ -157,6 +157,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) + }) }) } From 28eaac3a9c7082e8c496005c1cb66b4b70a4f82f Mon Sep 17 00:00:00 2001 From: Maxim Neverov Date: Thu, 14 Feb 2019 21:52:59 +0100 Subject: [PATCH 2/2] Extracted common code for diff calculation --- pkg/services/alerting/conditions/reducer.go | 87 +++++++++---------- .../alerting/conditions/reducer_test.go | 12 +++ 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/pkg/services/alerting/conditions/reducer.go b/pkg/services/alerting/conditions/reducer.go index 485ea43561b..f55545be311 100644 --- a/pkg/services/alerting/conditions/reducer.go +++ b/pkg/services/alerting/conditions/reducer.go @@ -95,55 +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 - } - } - - 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 - 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 { @@ -166,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 index 70b03f1dba8..d2c21771d0b 100644 --- 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))