From 040ce401134d72e0260170edfd40fbb1372731e1 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Wed, 2 Feb 2022 08:50:44 -0500 Subject: [PATCH] SSE: Mode to drop NaN/Inf/Null in Reduction operations (#43583) Co-authored-by: Yuriy Tseretyan Co-authored-by: gillesdemey --- .../about-expressions.md | 23 +- pkg/expr/classic/reduce_test.go | 10 +- pkg/expr/commands.go | 58 +++- pkg/expr/commands_test.go | 91 ++++++ pkg/expr/mathexp/exp.go | 24 +- pkg/expr/mathexp/exp_scalar_no_test.go | 26 +- pkg/expr/mathexp/exp_test.go | 5 +- pkg/expr/mathexp/funcs.go | 8 +- pkg/expr/mathexp/reduce.go | 121 ++++++-- pkg/expr/mathexp/reduce_test.go | 265 +++++++++++++++++- pkg/expr/mathexp/resample.go | 4 +- pkg/expr/mathexp/type_series.go | 10 +- .../expressions/components/Reduce.tsx | 51 +++- public/app/features/expressions/types.ts | 31 ++ 14 files changed, 627 insertions(+), 100 deletions(-) create mode 100644 pkg/expr/commands_test.go diff --git a/docs/sources/panels/query-a-data-source/use-expressions-to-manipulate-data/about-expressions.md b/docs/sources/panels/query-a-data-source/use-expressions-to-manipulate-data/about-expressions.md index 5fdade2cdd2..3563b22e542 100644 --- a/docs/sources/panels/query-a-data-source/use-expressions-to-manipulate-data/about-expressions.md +++ b/docs/sources/panels/query-a-data-source/use-expressions-to-manipulate-data/about-expressions.md @@ -148,31 +148,44 @@ Reduce takes one or more time series returned from a query or an expression and - **Function -** The reduction function to use - **Input -** The variable (refID (such as `A`)) to resample +- **Mode -** Allows control behavior of reduction function when a series contains non-numerical values (null, NaN, +\-Inf) #### Reduction Functions -> **Note:** In the future we plan to add options to control empty, NaN, and null behavior for reduction functions. - ##### Count Count returns the number of points in each series. ##### Mean -Mean returns the total of all values in each series divided by the number of points in that series. If any values in the series are null or nan, or if the series is empty, NaN is returned. +Mean returns the total of all values in each series divided by the number of points in that series. In `strict` mode if any values in the series are null or nan, or if the series is empty, NaN is returned. ##### Min and Max -Min and Max return the smallest or largest value in the series respectively. If any values in the series are null or nan, or if the series is empty, NaN is returned. +Min and Max return the smallest or largest value in the series respectively. In `strict` mode if any values in the series are null or nan, or if the series is empty, NaN is returned. ##### Sum -Sum returns the total of all values in the series. If series is of zero length, the sum will be 0. If there are any NaN or Null values in the series, NaN is returned. +Sum returns the total of all values in the series. If series is of zero length, the sum will be 0. In `strict` mode if there are any NaN or Null values in the series, NaN is returned. #### Last Last returns the last number in the series. If the series has no values then returns NaN. +#### Reduction Modes + +##### Strict + +In Strict mode the input series is processed as is. If any values in the series are non-numeric (null, NaN or +\-Inf), NaN is returned. + +##### Drop Non-Numeric + +In this mode all non-numeric values (null, NaN or +\-Inf) in the input series are filtered out before executing the reduction function. + +##### Replace Non-Numeric + +In this mode all non-numeric values are replaced by a pre-defined value. + ### Resample Resample changes the time stamps in each time series to have a consistent time interval. The main use case is so you can resample time series that do not share the same timestamps so math can be performed between them. This can be done by resample each of the two series, and then in a Math operation referencing the resampled variables. diff --git a/pkg/expr/classic/reduce_test.go b/pkg/expr/classic/reduce_test.go index c30d33966d0..50871dd61cd 100644 --- a/pkg/expr/classic/reduce_test.go +++ b/pkg/expr/classic/reduce_test.go @@ -402,10 +402,7 @@ func TestPercentDiffAbsReducer(t *testing.T) { func valBasedSeries(vals ...*float64) mathexp.Series { newSeries := mathexp.NewSeries("", nil, len(vals)) for idx, f := range vals { - err := newSeries.SetPoint(idx, time.Unix(int64(idx), 0), f) - if err != nil { - panic(err) - } + newSeries.SetPoint(idx, time.Unix(int64(idx), 0), f) } return newSeries } @@ -413,10 +410,7 @@ func valBasedSeries(vals ...*float64) mathexp.Series { func valBasedSeriesWithLabels(l data.Labels, vals ...*float64) mathexp.Series { newSeries := mathexp.NewSeries("", l, len(vals)) for idx, f := range vals { - err := newSeries.SetPoint(idx, time.Unix(int64(idx), 0), f) - if err != nil { - panic(err) - } + newSeries.SetPoint(idx, time.Unix(int64(idx), 0), f) } return newSeries } diff --git a/pkg/expr/commands.go b/pkg/expr/commands.go index f31b9164f67..f238e4ef9f5 100644 --- a/pkg/expr/commands.go +++ b/pkg/expr/commands.go @@ -7,6 +7,7 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/backend/gtime" + "github.com/grafana/grafana/pkg/expr/mathexp" ) @@ -69,19 +70,25 @@ func (gm *MathCommand) Execute(ctx context.Context, vars mathexp.Vars) (mathexp. // ReduceCommand is an expression command for reduction of a timeseries such as a min, mean, or max. type ReduceCommand struct { - Reducer string - VarToReduce string - refID string + Reducer string + VarToReduce string + refID string + seriesMapper mathexp.ReduceMapper } // NewReduceCommand creates a new ReduceCMD. -func NewReduceCommand(refID, reducer, varToReduce string) *ReduceCommand { - // TODO: validate reducer here, before execution - return &ReduceCommand{ - Reducer: reducer, - VarToReduce: varToReduce, - refID: refID, +func NewReduceCommand(refID, reducer, varToReduce string, mapper mathexp.ReduceMapper) (*ReduceCommand, error) { + _, err := mathexp.GetReduceFunc(reducer) + if err != nil { + return nil, err } + + return &ReduceCommand{ + Reducer: reducer, + VarToReduce: varToReduce, + refID: refID, + seriesMapper: mapper, + }, nil } // UnmarshalReduceCommand creates a MathCMD from Grafana's frontend query. @@ -105,7 +112,36 @@ func UnmarshalReduceCommand(rn *rawNode) (*ReduceCommand, error) { return nil, fmt.Errorf("expected reducer to be a string, got %T for refId %v", rawReducer, rn.RefID) } - return NewReduceCommand(rn.RefID, redFunc, varToReduce), nil + var mapper mathexp.ReduceMapper = nil + settings, ok := rn.Query["settings"] + if ok { + switch s := settings.(type) { + case map[string]interface{}: + mode, ok := s["mode"] + if ok && mode != "" { + switch mode { + case "dropNN": + mapper = mathexp.DropNonNumber{} + case "replaceNN": + valueStr, ok := s["replaceWithValue"] + if !ok { + return nil, fmt.Errorf("expected settings.replaceWithValue to be specified when mode is 'replaceNN' for refId %v", rn.RefID) + } + switch value := valueStr.(type) { + case float64: + mapper = mathexp.ReplaceNonNumberWithValue{Value: value} + default: + return nil, fmt.Errorf("expected settings.replaceWithValue to be a number, got %T for refId %v", value, rn.RefID) + } + default: + return nil, fmt.Errorf("reducer mode %s is not supported for refId %v. Supported only: [dropNN,replaceNN]", mode, rn.RefID) + } + } + default: + return nil, fmt.Errorf("expected settings to be an object, got %T for refId %v", s, rn.RefID) + } + } + return NewReduceCommand(rn.RefID, redFunc, varToReduce, mapper) } // NeedsVars returns the variable names (refIds) that are dependencies @@ -123,7 +159,7 @@ func (gr *ReduceCommand) Execute(ctx context.Context, vars mathexp.Vars) (mathex if !ok { return newRes, fmt.Errorf("can only reduce type series, got type %v", val.Type()) } - num, err := series.Reduce(gr.refID, gr.Reducer) + num, err := series.Reduce(gr.refID, gr.Reducer, gr.seriesMapper) if err != nil { return newRes, err } diff --git a/pkg/expr/commands_test.go b/pkg/expr/commands_test.go new file mode 100644 index 00000000000..d467dd76a4b --- /dev/null +++ b/pkg/expr/commands_test.go @@ -0,0 +1,91 @@ +package expr + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/expr/mathexp" +) + +func Test_UnmarshalReduceCommand_Settings(t *testing.T) { + var tests = []struct { + name string + querySettings string + isError bool + expectedMapper mathexp.ReduceMapper + }{ + { + name: "no mapper function when settings is not specified", + querySettings: ``, + expectedMapper: nil, + }, + { + name: "no mapper function when mode is not specified", + querySettings: `, "settings" : { }`, + expectedMapper: nil, + }, + { + name: "error when settings is not object", + querySettings: `, "settings" : "drop-nan"`, + isError: true, + }, + { + name: "no mapper function when mode is empty", + querySettings: `, "settings" : { "mode": "" }`, + expectedMapper: nil, + }, + { + name: "error when mode is not known", + querySettings: `, "settings" : { "mode": "test" }`, + isError: true, + }, + { + name: "filterNonNumber function when mode is 'dropNN'", + querySettings: `, "settings" : { "mode": "dropNN" }`, + expectedMapper: mathexp.DropNonNumber{}, + }, + { + name: "replaceNanWithValue function when mode is 'dropNN'", + querySettings: `, "settings" : { "mode": "replaceNN" , "replaceWithValue": -12 }`, + expectedMapper: mathexp.ReplaceNonNumberWithValue{Value: -12}, + }, + { + name: "error if mode is 'replaceNN' but field replaceWithValue is not specified", + querySettings: `, "settings" : { "mode": "replaceNN" }`, + isError: true, + }, + { + name: "error if mode is 'replaceNN' but field replaceWithValue is not a number", + querySettings: `, "settings" : { "mode": "replaceNN", "replaceWithValue" : "-12" }`, + isError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + q := fmt.Sprintf(`{ "expression" : "$A", "reducer": "sum"%s }`, test.querySettings) + var qmap = make(map[string]interface{}) + require.NoError(t, json.Unmarshal([]byte(q), &qmap)) + + cmd, err := UnmarshalReduceCommand(&rawNode{ + RefID: "A", + Query: qmap, + QueryType: "", + TimeRange: TimeRange{}, + DataSource: nil, + }) + + if test.isError { + require.Error(t, err) + return + } + + require.NotNil(t, cmd) + + require.Equal(t, test.expectedMapper, cmd.seriesMapper) + }) + } +} diff --git a/pkg/expr/mathexp/exp.go b/pkg/expr/mathexp/exp.go index 43b9d11689e..afb00c3a74f 100644 --- a/pkg/expr/mathexp/exp.go +++ b/pkg/expr/mathexp/exp.go @@ -133,18 +133,14 @@ func (e *State) unarySeries(s Series, op string) (Series, error) { for i := 0; i < s.Len(); i++ { t, f := s.GetPoint(i) if f == nil { - if err := newSeries.SetPoint(i, t, nil); err != nil { - return newSeries, err - } + newSeries.SetPoint(i, t, nil) continue } newF, err := unaryOp(op, *f) if err != nil { return newSeries, err } - if err := newSeries.SetPoint(i, t, &newF); err != nil { - return newSeries, err - } + newSeries.SetPoint(i, t, &newF) } return newSeries, nil } @@ -437,9 +433,7 @@ func (e *State) biSeriesNumber(labels data.Labels, op string, s Series, scalarVa nF := math.NaN() t, f := s.GetPoint(i) if f == nil || scalarVal == nil { - if err := newSeries.SetPoint(i, t, nil); err != nil { - return newSeries, err - } + newSeries.SetPoint(i, t, nil) continue } if seriesFirst { @@ -450,9 +444,7 @@ func (e *State) biSeriesNumber(labels data.Labels, op string, s Series, scalarVa if err != nil { return newSeries, err } - if err := newSeries.SetPoint(i, t, &nF); err != nil { - return newSeries, err - } + newSeries.SetPoint(i, t, &nF) } return newSeries, nil } @@ -475,18 +467,14 @@ func (e *State) biSeriesSeries(labels data.Labels, op string, aSeries, bSeries S continue } if aF == nil || bF == nil { - if err := newSeries.AppendPoint(aIdx, aTime, nil); err != nil { - return newSeries, err - } + newSeries.AppendPoint(aTime, nil) continue } nF, err := binaryOp(op, *aF, *bF) if err != nil { return newSeries, err } - if err := newSeries.AppendPoint(aIdx, aTime, &nF); err != nil { - return newSeries, err - } + newSeries.AppendPoint(aTime, &nF) } return newSeries, nil } diff --git a/pkg/expr/mathexp/exp_scalar_no_test.go b/pkg/expr/mathexp/exp_scalar_no_test.go index 33b79e0ccb5..fa8a015e42a 100644 --- a/pkg/expr/mathexp/exp_scalar_no_test.go +++ b/pkg/expr/mathexp/exp_scalar_no_test.go @@ -116,28 +116,24 @@ func TestNumberExpr(t *testing.T) { results: Results{[]Value{makeNumber("", nil, float64Pointer(-2.0))}}, }, { - name: "binary: Scalar Op Number (Number will nil val) - currently Panics", + name: "binary: Scalar Op Number (Number will nil val) returns nil", expr: "1 + $A", + newErrIs: assert.NoError, + execErrIs: assert.NoError, + resultIs: assert.Equal, vars: Vars{"A": Results{[]Value{makeNumber("", nil, nil)}}}, - willPanic: true, + results: Results{[]Value{makeNumber("", nil, nil)}}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - testBlock := func() { - e, err := New(tt.expr) - tt.newErrIs(t, err) - if e != nil { - res, err := e.Execute("", tt.vars) - tt.execErrIs(t, err) - tt.resultIs(t, tt.results, res) - } - } - if tt.willPanic { - assert.Panics(t, testBlock) - } else { - assert.NotPanics(t, testBlock) + e, err := New(tt.expr) + tt.newErrIs(t, err) + if e != nil { + res, err := e.Execute("", tt.vars) + tt.execErrIs(t, err) + tt.resultIs(t, tt.results, res) } }) } diff --git a/pkg/expr/mathexp/exp_test.go b/pkg/expr/mathexp/exp_test.go index e368545e5c1..85936c2ca74 100644 --- a/pkg/expr/mathexp/exp_test.go +++ b/pkg/expr/mathexp/exp_test.go @@ -16,10 +16,7 @@ type tp struct { func makeSeries(name string, labels data.Labels, points ...tp) Series { newSeries := NewSeries(name, labels, len(points)) for idx, p := range points { - err := newSeries.SetPoint(idx, p.t, p.f) - if err != nil { - panic(err) - } + newSeries.SetPoint(idx, p.t, p.f) } return newSeries } diff --git a/pkg/expr/mathexp/funcs.go b/pkg/expr/mathexp/funcs.go index 681d3d9afbd..a299e3462e5 100644 --- a/pkg/expr/mathexp/funcs.go +++ b/pkg/expr/mathexp/funcs.go @@ -226,9 +226,7 @@ func perFloat(e *State, val Value, floatF func(x float64) float64) (Value, error if f != nil { nF = floatF(*f) } - if err := newSeries.SetPoint(i, t, &nF); err != nil { - return newSeries, err - } + newSeries.SetPoint(i, t, &nF) } newVal = newSeries default: @@ -257,9 +255,7 @@ func perNullableFloat(e *State, val Value, floatF func(x *float64) *float64) (Va newSeries := NewSeries(e.RefID, resSeries.GetLabels(), resSeries.Len()) for i := 0; i < resSeries.Len(); i++ { t, f := resSeries.GetPoint(i) - if err := newSeries.SetPoint(i, t, floatF(f)); err != nil { - return newSeries, err - } + newSeries.SetPoint(i, t, floatF(f)) } newVal = newSeries default: diff --git a/pkg/expr/mathexp/reduce.go b/pkg/expr/mathexp/reduce.go index be8c7816c36..7bd55d72e30 100644 --- a/pkg/expr/mathexp/reduce.go +++ b/pkg/expr/mathexp/reduce.go @@ -3,10 +3,13 @@ package mathexp import ( "fmt" "math" + "strings" "github.com/grafana/grafana-plugin-sdk-go/data" ) +type ReducerFunc = func(fv *Float64Field) *float64 + func Sum(fv *Float64Field) *float64 { var sum float64 for i := 0; i < fv.Len(); i++ { @@ -75,38 +78,114 @@ func Last(fv *Float64Field) *float64 { f = math.NaN() return &f } - v := fv.GetValue(fv.Len() - 1) - f = *v - return &f + return fv.GetValue(fv.Len() - 1) +} + +func GetReduceFunc(rFunc string) (ReducerFunc, error) { + switch strings.ToLower(rFunc) { + case "sum": + return Sum, nil + case "mean": + return Avg, nil + case "min": + return Min, nil + case "max": + return Max, nil + case "count": + return Count, nil + case "last": + return Last, nil + default: + return nil, fmt.Errorf("reduction %v not implemented", rFunc) + } } // Reduce turns the Series into a Number based on the given reduction function -func (s Series) Reduce(refID, rFunc string) (Number, error) { +// if ReduceMapper is defined it applies it to the provided series and performs reduction of the resulting series. +// Otherwise, the reduction operation is done against the original series. +func (s Series) Reduce(refID, rFunc string, mapper ReduceMapper) (Number, error) { var l data.Labels if s.GetLabels() != nil { l = s.GetLabels().Copy() } number := NewNumber(refID, l) var f *float64 - fVec := s.Frame.Fields[seriesTypeValIdx] + series := s + if mapper != nil { + series = mapSeries(s, mapper) + } + fVec := series.Frame.Fields[seriesTypeValIdx] floatField := Float64Field(*fVec) - switch rFunc { - case "sum": - f = Sum(&floatField) - case "mean": - f = Avg(&floatField) - case "min": - f = Min(&floatField) - case "max": - f = Max(&floatField) - case "count": - f = Count(&floatField) - case "last": - f = Last(&floatField) - default: - return number, fmt.Errorf("reduction %v not implemented", rFunc) + reduceFunc, err := GetReduceFunc(rFunc) + if err != nil { + return number, err + } + f = reduceFunc(&floatField) + if f != nil && mapper != nil { + f = mapper.MapOutput(f) } number.SetValue(f) - return number, nil } + +type ReduceMapper interface { + MapInput(s *float64) *float64 + MapOutput(v *float64) *float64 +} + +// mapSeries creates a series where all points are mapped using the provided map function ReduceMapper.MapInput +func mapSeries(s Series, mapper ReduceMapper) Series { + newSeries := NewSeries(s.Frame.RefID, s.GetLabels(), 0) + for i := 0; i < s.Len(); i++ { + f := s.GetValue(i) + f = mapper.MapInput(f) + if f == nil { + continue + } + newFloat := *f + newSeries.AppendPoint(s.GetTime(i), &newFloat) + } + return newSeries +} + +type DropNonNumber struct { +} + +// MapInput returns nil if the input parameter is nil or point to either a NaN or a Inf +func (d DropNonNumber) MapInput(s *float64) *float64 { + if s == nil || math.IsNaN(*s) || math.IsInf(*s, 0) { + return nil + } + return s +} + +// MapOutput returns nil if the input parameter is nil or point to either a NaN or a Inf +func (d DropNonNumber) MapOutput(s *float64) *float64 { + if s != nil && math.IsNaN(*s) { + return nil + } + return s +} + +type ReplaceNonNumberWithValue struct { + Value float64 +} + +// MapInput returns a pointer to ReplaceNonNumberWithValue.Value if input parameter is nil or points to either a NaN or an Inf. +// Otherwise, returns the input pointer as is. +func (r ReplaceNonNumberWithValue) MapInput(v *float64) *float64 { + if v == nil || math.IsNaN(*v) || math.IsInf(*v, 0) { + return &r.Value + } else { + return v + } +} + +// MapOutput returns a pointer to ReplaceNonNumberWithValue.Value if input parameter is nil or points to either a NaN or an Inf. +// Otherwise, returns the input pointer as is. +func (r ReplaceNonNumberWithValue) MapOutput(s *float64) *float64 { + if s != nil && math.IsNaN(*s) { + return &r.Value + } + return s +} diff --git a/pkg/expr/mathexp/reduce_test.go b/pkg/expr/mathexp/reduce_test.go index 2bd13780018..e4576e555d7 100644 --- a/pkg/expr/mathexp/reduce_test.go +++ b/pkg/expr/mathexp/reduce_test.go @@ -2,6 +2,7 @@ package mathexp import ( "math" + "math/rand" "testing" "time" @@ -227,6 +228,19 @@ func TestSeriesReduce(t *testing.T) { }, }, }, + { + name: "last null series", + red: "last", + varToReduce: "A", + vars: seriesWithNil, + errIs: require.NoError, + resultsIs: require.Equal, + results: Results{ + []Value{ + makeNumber("", nil, nil), + }, + }, + }, } for _, tt := range tests { @@ -234,7 +248,7 @@ func TestSeriesReduce(t *testing.T) { results := Results{} seriesSet := tt.vars[tt.varToReduce] for _, series := range seriesSet.Values { - ns, err := series.Value().(*Series).Reduce("", tt.red) + ns, err := series.Value().(*Series).Reduce("", tt.red, nil) tt.errIs(t, err) if err != nil { return @@ -251,3 +265,252 @@ func TestSeriesReduce(t *testing.T) { }) } } + +var seriesNonNumbers = Vars{ + "A": Results{ + []Value{ + makeSeries("temp", nil, + tp{time.Unix(5, 0), NaN}, + tp{time.Unix(10, 0), float64Pointer(math.Inf(-1))}, + tp{time.Unix(15, 0), float64Pointer(math.Inf(1))}, + tp{time.Unix(15, 0), nil}), + }, + }, +} + +func TestSeriesReduceDropNN(t *testing.T) { + var tests = []struct { + name string + red string + vars Vars + varToReduce string + results Results + }{ + { + name: "dropNN: sum series", + red: "sum", + varToReduce: "A", + vars: aSeries, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(3)), + }, + }, + }, + { + name: "dropNN: sum series with a nil value", + red: "sum", + varToReduce: "A", + vars: seriesWithNil, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(2)), + }, + }, + }, + { + name: "dropNN: sum empty series", + red: "sum", + varToReduce: "A", + vars: seriesEmpty, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(0)), + }, + }, + }, + { + name: "dropNN: mean series with a nil value and real value", + red: "mean", + varToReduce: "A", + vars: seriesWithNil, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(2)), + }, + }, + }, + { + name: "DropNN: mean empty series", + red: "mean", + varToReduce: "A", + vars: seriesEmpty, + results: Results{ + []Value{ + makeNumber("", nil, nil), + }, + }, + }, + { + name: "DropNN: mean series that becomes empty after filtering non-number", + red: "mean", + varToReduce: "A", + vars: seriesNonNumbers, + results: Results{ + []Value{ + makeNumber("", nil, nil), + }, + }, + }, + { + name: "DropNN: count empty series", + red: "count", + varToReduce: "A", + vars: seriesEmpty, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(0)), + }, + }, + }, + { + name: "DropNN: count series with nil and value should only count real numbers", + red: "count", + varToReduce: "A", + vars: seriesWithNil, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(1)), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + results := Results{} + seriesSet := tt.vars[tt.varToReduce] + for _, series := range seriesSet.Values { + ns, err := series.Value().(*Series).Reduce("", tt.red, DropNonNumber{}) + require.NoError(t, err) + results.Values = append(results.Values, ns) + } + opt := cmp.Comparer(func(x, y float64) bool { + return (math.IsNaN(x) && math.IsNaN(y)) || x == y + }) + options := append([]cmp.Option{opt}, data.FrameTestCompareOptions()...) + if diff := cmp.Diff(tt.results, results, options...); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestSeriesReduceReplaceNN(t *testing.T) { + replaceWith := rand.Float64() + var tests = []struct { + name string + red string + vars Vars + varToReduce string + results Results + }{ + { + name: "replaceNN: sum series", + red: "sum", + varToReduce: "A", + vars: aSeries, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(3)), + }, + }, + }, + { + name: "replaceNN: sum series with a nil value", + red: "sum", + varToReduce: "A", + vars: seriesWithNil, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(replaceWith+2)), + }, + }, + }, + { + name: "replaceNN: sum empty series", + red: "sum", + varToReduce: "A", + vars: seriesEmpty, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(0)), + }, + }, + }, + { + name: "replaceNN: mean series with a nil value and real value", + red: "mean", + varToReduce: "A", + vars: seriesWithNil, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer((2+replaceWith)/2e0)), + }, + }, + }, + { + name: "replaceNN: mean empty series", + red: "mean", + varToReduce: "A", + vars: seriesEmpty, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(replaceWith)), + }, + }, + }, + { + name: "replaceNN: mean series that becomes empty after filtering non-number", + red: "mean", + varToReduce: "A", + vars: seriesNonNumbers, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(replaceWith)), + }, + }, + }, + { + name: "replaceNN: count empty series", + red: "count", + varToReduce: "A", + vars: seriesEmpty, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(0)), + }, + }, + }, + { + name: "replaceNN: count series with nil and value should only count real numbers", + red: "count", + varToReduce: "A", + vars: seriesWithNil, + results: Results{ + []Value{ + makeNumber("", nil, float64Pointer(2)), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + results := Results{} + seriesSet := tt.vars[tt.varToReduce] + for _, series := range seriesSet.Values { + ns, err := series.Value().(*Series).Reduce("", tt.red, ReplaceNonNumberWithValue{Value: replaceWith}) + require.NoError(t, err) + results.Values = append(results.Values, ns) + } + opt := cmp.Comparer(func(x, y float64) bool { + return (math.IsNaN(x) && math.IsNaN(y)) || x == y + }) + options := append([]cmp.Option{opt}, data.FrameTestCompareOptions()...) + if diff := cmp.Diff(tt.results, results, options...); diff != "" { + t.Errorf("Result mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/expr/mathexp/resample.go b/pkg/expr/mathexp/resample.go index e568adc5fbd..13cd4b0914a 100644 --- a/pkg/expr/mathexp/resample.go +++ b/pkg/expr/mathexp/resample.go @@ -72,9 +72,7 @@ func (s Series) Resample(refID string, interval time.Duration, downsampler strin } value = tmp } - if err := resampled.SetPoint(idx, t, value); err != nil { - return resampled, err - } + resampled.SetPoint(idx, t, value) t = t.Add(interval) idx++ } diff --git a/pkg/expr/mathexp/type_series.go b/pkg/expr/mathexp/type_series.go index 8bb44439487..6748116c5fc 100644 --- a/pkg/expr/mathexp/type_series.go +++ b/pkg/expr/mathexp/type_series.go @@ -166,17 +166,15 @@ func (s Series) GetPoint(pointIdx int) (time.Time, *float64) { } // SetPoint sets the time and value on the corresponding vectors at the specified index. -func (s Series) SetPoint(pointIdx int, t time.Time, f *float64) (err error) { +func (s Series) SetPoint(pointIdx int, t time.Time, f *float64) { s.Frame.Fields[seriesTypeTimeIdx].Set(pointIdx, t) s.Frame.Fields[seriesTypeValIdx].Set(pointIdx, f) - return } // AppendPoint appends a point (time/value). -func (s Series) AppendPoint(pointIdx int, t time.Time, f *float64) (err error) { +func (s Series) AppendPoint(t time.Time, f *float64) { s.Frame.Fields[seriesTypeTimeIdx].Append(t) s.Frame.Fields[seriesTypeValIdx].Append(f) - return } // Len returns the length of the series. @@ -214,8 +212,8 @@ func (ss SortSeriesByTime) Len() int { return Series(ss).Len() } func (ss SortSeriesByTime) Swap(i, j int) { iTimeVal, iFVal := Series(ss).GetPoint(i) jTimeVal, jFVal := Series(ss).GetPoint(j) - _ = Series(ss).SetPoint(j, iTimeVal, iFVal) - _ = Series(ss).SetPoint(i, jTimeVal, jFVal) + Series(ss).SetPoint(j, iTimeVal, iFVal) + Series(ss).SetPoint(i, jTimeVal, jFVal) } func (ss SortSeriesByTime) Less(i, j int) bool { diff --git a/public/app/features/expressions/components/Reduce.tsx b/public/app/features/expressions/components/Reduce.tsx index 9ce1ad66034..693a66a5371 100644 --- a/public/app/features/expressions/components/Reduce.tsx +++ b/public/app/features/expressions/components/Reduce.tsx @@ -1,7 +1,7 @@ import React, { FC } from 'react'; import { SelectableValue } from '@grafana/data'; -import { InlineField, InlineFieldRow, Select } from '@grafana/ui'; -import { ExpressionQuery, reducerTypes } from '../types'; +import { InlineField, InlineFieldRow, Input, Select } from '@grafana/ui'; +import { ExpressionQuery, ExpressionQuerySettings, ReducerMode, reducerMode, reducerTypes } from '../types'; interface Props { labelWidth: number; @@ -21,6 +21,49 @@ export const Reduce: FC = ({ labelWidth, onChange, refIds, query }) => { onChange({ ...query, reducer: value.value }); }; + const onSettingsChanged = (settings: ExpressionQuerySettings) => { + onChange({ ...query, settings: settings }); + }; + + const onModeChanged = (value: SelectableValue) => { + let newSettings: ExpressionQuerySettings; + switch (value.value) { + case ReducerMode.ReplaceNonNumbers: + let replaceWithNumber = 0; + if (query.settings?.mode === ReducerMode.ReplaceNonNumbers) { + replaceWithNumber = query.settings?.replaceWithValue ?? 0; + } + newSettings = { + mode: ReducerMode.ReplaceNonNumbers, + replaceWithValue: replaceWithNumber, + }; + break; + default: + newSettings = { + mode: value.value, + }; + } + onSettingsChanged(newSettings); + }; + + const onReplaceWithChanged = (e: React.FormEvent) => { + const value = e.currentTarget.valueAsNumber; + onSettingsChanged({ mode: ReducerMode.ReplaceNonNumbers, replaceWithValue: value ?? 0 }); + }; + + const mode = query.settings?.mode ?? ReducerMode.Strict; + + const replaceWithNumber = () => { + if (mode !== ReducerMode.ReplaceNonNumbers) { + return; + } + return ( + + + + ); + }; + return ( @@ -29,6 +72,10 @@ export const Reduce: FC = ({ labelWidth, onChange, refIds, query }) => { + + {replaceWithNumber()} ); }; diff --git a/public/app/features/expressions/types.ts b/public/app/features/expressions/types.ts index b14ac983e0f..aaaa71faec9 100644 --- a/public/app/features/expressions/types.ts +++ b/public/app/features/expressions/types.ts @@ -24,6 +24,30 @@ export const reducerTypes: Array> = [ { value: ReducerID.last, label: 'Last', description: 'Get the last value' }, ]; +export enum ReducerMode { + Strict = '', // backend API wants an empty string to support "strict" mode + ReplaceNonNumbers = 'replaceNN', + DropNonNumbers = 'dropNN', +} + +export const reducerMode: Array> = [ + { + value: ReducerMode.Strict, + label: 'Strict', + description: 'Result can be NaN if series contains non-numeric data', + }, + { + value: ReducerMode.DropNonNumbers, + label: 'Drop Non-numeric Values', + description: 'Drop NaN, +/-Inf and null from input series before reducing', + }, + { + value: ReducerMode.ReplaceNonNumbers, + label: 'Replace Non-numeric Values', + description: 'Replace NaN, +/-Inf and null with a constant value before reducing', + }, +]; + export const downsamplingTypes: Array> = [ { value: ReducerID.min, label: 'Min', description: 'Fill with the minimum value' }, { value: ReducerID.max, label: 'Max', description: 'Fill with the maximum value' }, @@ -49,7 +73,14 @@ export interface ExpressionQuery extends DataQuery { downsampler?: string; upsampler?: string; conditions?: ClassicCondition[]; + settings?: ExpressionQuerySettings; } + +export interface ExpressionQuerySettings { + mode?: ReducerMode; + replaceWithValue?: number; +} + export interface ClassicCondition { evaluator: { params: number[];