diff --git a/pkg/expr/dataplane_test.go b/pkg/expr/dataplane_test.go index 1b885e7c9ea..fcec6bf390d 100644 --- a/pkg/expr/dataplane_test.go +++ b/pkg/expr/dataplane_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/grafana/dataplane/examples" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" @@ -44,7 +45,7 @@ func TestPassThroughDataplaneExamples(t *testing.T) { func framesPassThroughService(t *testing.T, frames data.Frames) (data.Frames, error) { me := &mockEndpoint{ - Frames: frames, + map[string]backend.DataResponse{"A": {Frames: frames}}, } cfg := setting.NewCfg() diff --git a/pkg/expr/errors.go b/pkg/expr/errors.go index ea8e342a2f0..555aa445fb7 100644 --- a/pkg/expr/errors.go +++ b/pkg/expr/errors.go @@ -2,6 +2,7 @@ package expr import ( "errors" + "fmt" "github.com/grafana/grafana/pkg/util/errutil" ) @@ -13,7 +14,7 @@ var ConversionError = errutil.BadRequest("sse.readDataError").MustTemplate( ), ) -func MakeConversionError(refID string, err error) error { +func makeConversionError(refID string, err error) error { data := errutil.TemplateData{ // Conversion errors should only have meta information in errors Public: map[string]any{ @@ -52,3 +53,41 @@ func MakeQueryError(refID, datasourceUID string, err error) error { return QueryError.Build(data) } + +var depErrStr = "did not execute expression [{{ .Public.refId }}] due to a failure to of the dependent expression or query [{{.Public.depRefId}}]" + +var DependencyError = errutil.NewBase( + errutil.StatusBadRequest, "sse.dependencyError").MustTemplate( + depErrStr, + errutil.WithPublic(depErrStr)) + +func makeDependencyError(refID, depRefID string) error { + data := errutil.TemplateData{ + Public: map[string]interface{}{ + "refId": refID, + "depRefId": depRefID, + }, + Error: fmt.Errorf("did not execute expression %v due to a failure to of the dependent expression or query %v", refID, depRefID), + } + + return DependencyError.Build(data) +} + +var unexpectedNodeTypeErrString = "expected executable node type but got node type [{{ .Public.nodeType }} for refid [{{ .Public.refId}}]" + +var UnexpectedNodeTypeError = errutil.NewBase( + errutil.StatusBadRequest, "sse.unexpectedNodeType").MustTemplate( + unexpectedNodeTypeErrString, + errutil.WithPublic(unexpectedNodeTypeErrString)) + +func makeUnexpectedNodeTypeError(refID, nodeType string) error { + data := errutil.TemplateData{ + Public: map[string]interface{}{ + "refId": refID, + "nodeType": nodeType, + }, + Error: fmt.Errorf("expected executable node type but got node type %v for refId %v", nodeType, refID), + } + + return UnexpectedNodeTypeError.Build(data) +} diff --git a/pkg/expr/graph.go b/pkg/expr/graph.go index ae2773f9282..639ef0c4873 100644 --- a/pkg/expr/graph.go +++ b/pkg/expr/graph.go @@ -44,8 +44,13 @@ type Node interface { ID() int64 // ID() allows the gonum graph node interface to be fulfilled NodeType() NodeType RefID() string - Execute(ctx context.Context, now time.Time, vars mathexp.Vars, s *Service) (mathexp.Results, error) String() string + NeedsVars() []string +} + +type ExecutableNode interface { + Node + Execute(ctx context.Context, now time.Time, vars mathexp.Vars, s *Service) (mathexp.Results, error) } // DataPipeline is an ordered set of nodes returned from DPGraph processing. @@ -67,27 +72,48 @@ func (dp *DataPipeline) execute(c context.Context, now time.Time, s *Service) (m dsNodes = append(dsNodes, node.(*DSNode)) } - if err := executeDSNodesGrouped(c, now, vars, s, dsNodes); err != nil { - return nil, err - } + executeDSNodesGrouped(c, now, vars, s, dsNodes) } for _, node := range *dp { if groupByDSFlag && node.NodeType() == TypeDatasourceNode { continue // already executed via executeDSNodesGrouped } + + // Don't execute nodes that have dependent nodes that have failed + var hasDepError bool + for _, neededVar := range node.NeedsVars() { + if res, ok := vars[neededVar]; ok { + if res.Error != nil { + errResult := mathexp.Results{ + Error: makeDependencyError(node.RefID(), neededVar), + } + vars[node.RefID()] = errResult + hasDepError = true + break + } + } + } + if hasDepError { + continue + } + c, span := s.tracer.Start(c, "SSE.ExecuteNode") span.SetAttributes("node.refId", node.RefID(), attribute.Key("node.refId").String(node.RefID())) - if node.NodeType() == TypeCMDNode { - cmdNode := node.(*CMDNode) - inputRefIDs := cmdNode.Command.NeedsVars() + if len(node.NeedsVars()) > 0 { + inputRefIDs := node.NeedsVars() span.SetAttributes("node.inputRefIDs", inputRefIDs, attribute.Key("node.inputRefIDs").StringSlice(inputRefIDs)) } defer span.End() - res, err := node.Execute(c, now, vars, s) + execNode, ok := node.(ExecutableNode) + if !ok { + return vars, makeUnexpectedNodeTypeError(node.RefID(), node.NodeType().String()) + } + + res, err := execNode.Execute(c, now, vars, s) if err != nil { - return nil, err + res.Error = err } vars[node.RefID()] = res diff --git a/pkg/expr/mathexp/exp.go b/pkg/expr/mathexp/exp.go index 74b3c4e7ef5..9908cdc2cbc 100644 --- a/pkg/expr/mathexp/exp.go +++ b/pkg/expr/mathexp/exp.go @@ -307,7 +307,7 @@ func (e *State) union(aResults, bResults Results, biNode *parse.BinaryNode) []*U } func (e *State) walkBinary(node *parse.BinaryNode) (Results, error) { - res := Results{Values{}} + res := Results{Values: Values{}} ar, err := e.walk(node.Args[0]) if err != nil { return res, err diff --git a/pkg/expr/mathexp/exp_nan_null_val_test.go b/pkg/expr/mathexp/exp_nan_null_val_test.go index 835bd446c1c..c57ad790386 100644 --- a/pkg/expr/mathexp/exp_nan_null_val_test.go +++ b/pkg/expr/mathexp/exp_nan_null_val_test.go @@ -27,114 +27,102 @@ func TestNaN(t *testing.T) { { name: "unary !: Op Number(NaN) is NaN", expr: "! $A", - vars: Vars{"A": Results{[]Value{makeNumber("", nil, NaN)}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("", nil, NaN))}, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{[]Value{makeNumber("", nil, NaN)}}, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "unary -: Op Number(NaN) is NaN", expr: "-$A", - vars: Vars{"A": Results{[]Value{makeNumber("", nil, NaN)}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("", nil, NaN))}, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{[]Value{makeNumber("", nil, NaN)}}, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "binary: Scalar Op(Non-AND/OR) Number(NaN) is NaN", expr: "1 * $A", - vars: Vars{"A": Results{[]Value{makeNumber("", nil, NaN)}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("", nil, NaN))}, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{[]Value{makeNumber("", nil, NaN)}}, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "binary: Scalar Op(AND/OR) Number(NaN) is 0/1", expr: "1 || $A", - vars: Vars{"A": Results{[]Value{makeNumber("", nil, NaN)}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("", nil, NaN))}, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{[]Value{makeNumber("", nil, float64Pointer(1))}}, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(1))), }, { name: "binary: Scalar Op(Non-AND/OR) Series(with NaN value) is NaN)", expr: "1 - $A", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), NaN, - }), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(-1), + "A": resultValuesNoErr( + makeSeries("temp", nil, tp{ + time.Unix(5, 0), float64Pointer(2), }, tp{ time.Unix(10, 0), NaN, }), - }, + ), }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(-1), + }, tp{ + time.Unix(10, 0), NaN, + }), + ), }, { name: "binary: Number Op(Non-AND/OR) Series(with NaN value) is Series with NaN", expr: "$A == $B", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), NaN, - }), - }, - }, - "B": Results{[]Value{makeNumber("", nil, float64Pointer(0))}}, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(0), + "A": resultValuesNoErr( + makeSeries("temp", nil, tp{ + time.Unix(5, 0), float64Pointer(2), }, tp{ time.Unix(10, 0), NaN, }), - }, + ), + "B": resultValuesNoErr(makeNumber("", nil, float64Pointer(0))), }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(0), + }, tp{ + time.Unix(10, 0), NaN, + }), + ), }, { name: "binary: Number(NaN) Op Series(with NaN value) is Series with NaN", expr: "$A + $B", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), NaN, - }), - }, - }, - "B": Results{[]Value{makeNumber("", nil, NaN)}}, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), NaN, + "A": resultValuesNoErr( + makeSeries("temp", nil, tp{ + time.Unix(5, 0), float64Pointer(2), }, tp{ time.Unix(10, 0), NaN, }), - }, + ), + "B": resultValuesNoErr(makeNumber("", nil, NaN)), }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), NaN, + }, tp{ + time.Unix(10, 0), NaN, + }), + ), }, } @@ -200,200 +188,144 @@ func TestNullValues(t *testing.T) { name: "series: unary with a null value in it has a null value in result", expr: "- $A", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(-1), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - { - name: "series: binary with a null value in it has a null value in result", - expr: "$A - $A", - vars: Vars{ - "A": Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(0), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - { - name: "series and scalar: binary with a null value in it has a nil value in result", - expr: "$A - 1", - vars: Vars{ - "A": Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(0), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - { - name: "number: unary ! null number: is null", - expr: "! $A", - vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - }, - { - name: "number: binary null number and null number: is null", - expr: "$A + $A", - vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - }, - { - name: "number: binary non-null number and null number: is null", - expr: "$A * $B", - vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - "B": Results{ - []Value{ - makeNumber("", nil, float64Pointer(1)), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - }, - { - name: "number and series: binary non-null number and series with a null: is null", - expr: "$A * $B", - vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, float64Pointer(1)), - }, - }, - "B": Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ + "A": resultValuesNoErr( makeSeries("", nil, tp{ time.Unix(5, 0), float64Pointer(1), }, tp{ time.Unix(10, 0), nil, }), - }, + ), }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(-1), + }, tp{ + time.Unix(10, 0), nil, + }), + ), + }, + { + name: "series: binary with a null value in it has a null value in result", + expr: "$A - $A", + vars: Vars{ + "A": resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(1), + }, tp{ + time.Unix(10, 0), nil, + }), + ), + }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(0), + }, tp{ + time.Unix(10, 0), nil, + }), + ), + }, + { + name: "series and scalar: binary with a null value in it has a nil value in result", + expr: "$A - 1", + vars: Vars{ + "A": resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(1), + }, tp{ + time.Unix(10, 0), nil, + }), + ), + }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(0), + }, tp{ + time.Unix(10, 0), nil, + }), + ), + }, + { + name: "number: unary ! null number: is null", + expr: "! $A", + vars: Vars{ + "A": resultValuesNoErr(makeNumber("", nil, nil)), + }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr(makeNumber("", nil, nil)), + }, + { + name: "number: binary null number and null number: is null", + expr: "$A + $A", + vars: Vars{ + "A": resultValuesNoErr(makeNumber("", nil, nil)), + }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr(makeNumber("", nil, nil)), + }, + { + name: "number: binary non-null number and null number: is null", + expr: "$A * $B", + vars: Vars{ + "A": resultValuesNoErr(makeNumber("", nil, nil)), + "B": resultValuesNoErr(makeNumber("", nil, float64Pointer(1))), + }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr(makeNumber("", nil, nil)), + }, + { + name: "number and series: binary non-null number and series with a null: is null", + expr: "$A * $B", + vars: Vars{ + "A": resultValuesNoErr(makeNumber("", nil, float64Pointer(1))), + "B": resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(1), + }, tp{ + time.Unix(10, 0), nil, + }), + ), + }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(1), + }, tp{ + time.Unix(10, 0), nil, + }), + ), }, { name: "number and series: binary null number and series with non-null and null: is null and null", expr: "$A * $B", vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, - "B": Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, - }, - newErrIs: assert.NoError, - execErrIs: assert.NoError, - results: Results{ - []Value{ + "A": resultValuesNoErr(makeNumber("", nil, nil)), + "B": resultValuesNoErr( makeSeries("", nil, tp{ - time.Unix(5, 0), nil, + time.Unix(5, 0), float64Pointer(1), }, tp{ time.Unix(10, 0), nil, }), - }, + ), }, + newErrIs: assert.NoError, + execErrIs: assert.NoError, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), nil, + }, tp{ + time.Unix(10, 0), nil, + }), + ), }, } @@ -442,7 +374,7 @@ func TestNoData(t *testing.T) { "!$A", "-$A", } - vars := Vars{"A": Results{[]Value{NewNoData()}}} + vars := Vars{"A": resultValuesNoErr(NewNoData())} for _, expr := range unaryOps { t.Run(fmt.Sprintf("op: %s", expr), func(t *testing.T) { e, err := New(expr) @@ -459,8 +391,8 @@ func TestNoData(t *testing.T) { makeVars := func(a, b Value) Vars { return Vars{ - "A": Results{[]Value{a}}, - "B": Results{[]Value{b}}, + "A": resultValuesNoErr(a), + "B": resultValuesNoErr(b), } } diff --git a/pkg/expr/mathexp/exp_scalar_no_test.go b/pkg/expr/mathexp/exp_scalar_no_test.go index 79cd492caf9..338da57fae4 100644 --- a/pkg/expr/mathexp/exp_scalar_no_test.go +++ b/pkg/expr/mathexp/exp_scalar_no_test.go @@ -25,7 +25,7 @@ func TestScalarExpr(t *testing.T) { newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - Results: Results{[]Value{NewScalar("", float64Pointer(1.0))}}, + Results: resultValuesNoErr(NewScalar("", float64Pointer(1.0))), }, { name: "unary: scalar", @@ -34,7 +34,7 @@ func TestScalarExpr(t *testing.T) { newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - Results: Results{[]Value{NewScalar("", float64Pointer(0.0))}}, + Results: resultValuesNoErr(NewScalar("", float64Pointer(0.0))), }, { name: "binary: scalar Op scalar", @@ -43,7 +43,7 @@ func TestScalarExpr(t *testing.T) { newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - Results: Results{[]Value{NewScalar("", float64Pointer(2.0))}}, + Results: resultValuesNoErr(NewScalar("", float64Pointer(2.0))), }, { name: "binary: scalar Op scalar - divide by zero", @@ -52,25 +52,25 @@ func TestScalarExpr(t *testing.T) { newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - Results: Results{[]Value{NewScalar("", float64Pointer(math.Inf(1)))}}, + Results: resultValuesNoErr(NewScalar("", float64Pointer(math.Inf(1)))), }, { name: "binary: scalar Op number", expr: "1 + $A", - vars: Vars{"A": Results{[]Value{makeNumber("temp", nil, float64Pointer(2.0))}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("temp", nil, float64Pointer(2.0)))}, newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - Results: Results{[]Value{makeNumber("", nil, float64Pointer(3.0))}}, + Results: resultValuesNoErr(makeNumber("", nil, float64Pointer(3.0))), }, { name: "binary: number Op Scalar", expr: "$A - 3", - vars: Vars{"A": Results{[]Value{makeNumber("temp", nil, float64Pointer(2.0))}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("temp", nil, float64Pointer(2.0)))}, newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - Results: Results{[]Value{makeNumber("", nil, float64Pointer(-1))}}, + Results: resultValuesNoErr(makeNumber("", nil, float64Pointer(-1))), }, } @@ -101,20 +101,20 @@ func TestNumberExpr(t *testing.T) { { name: "binary: number Op Scalar", expr: "$A / $A", - vars: Vars{"A": Results{[]Value{makeNumber("temp", nil, float64Pointer(2.0))}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("temp", nil, float64Pointer(2.0)))}, newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - results: Results{[]Value{makeNumber("", nil, float64Pointer(1))}}, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(1))), }, { name: "unary: number", expr: "- $A", - vars: Vars{"A": Results{[]Value{makeNumber("temp", nil, float64Pointer(2.0))}}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("temp", nil, float64Pointer(2.0)))}, newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - results: Results{[]Value{makeNumber("", nil, float64Pointer(-2.0))}}, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(-2.0))), }, { name: "binary: Scalar Op Number (Number will nil val) returns nil", @@ -122,8 +122,8 @@ func TestNumberExpr(t *testing.T) { newErrIs: assert.NoError, execErrIs: assert.NoError, resultIs: assert.Equal, - vars: Vars{"A": Results{[]Value{makeNumber("", nil, nil)}}}, - results: Results{[]Value{makeNumber("", nil, nil)}}, + vars: Vars{"A": resultValuesNoErr(makeNumber("", nil, nil))}, + results: resultValuesNoErr(makeNumber("", nil, nil)), }, } diff --git a/pkg/expr/mathexp/exp_series_test.go b/pkg/expr/mathexp/exp_series_test.go index 5cc5703fbef..cb226e60337 100644 --- a/pkg/expr/mathexp/exp_series_test.go +++ b/pkg/expr/mathexp/exp_series_test.go @@ -25,15 +25,13 @@ func TestSeriesExpr(t *testing.T) { vars: aSeries, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ // Not sure about preservering names... - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), float64Pointer(1), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", nil, tp{ // Not sure about preservering names... + time.Unix(5, 0), float64Pointer(1), + }, tp{ + time.Unix(10, 0), float64Pointer(1), + }), + ), }, { name: "binary scalar Op series", @@ -41,15 +39,13 @@ func TestSeriesExpr(t *testing.T) { vars: aSeries, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ // Not sure about preservering names... - time.Unix(5, 0), float64Pointer(100), - }, tp{ - time.Unix(10, 0), float64Pointer(99), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", nil, tp{ // Not sure about preservering names... + time.Unix(5, 0), float64Pointer(100), + }, tp{ + time.Unix(10, 0), float64Pointer(99), + }), + ), }, { name: "binary series Op scalar", @@ -57,15 +53,13 @@ func TestSeriesExpr(t *testing.T) { vars: aSeries, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ // Not sure about preservering names... - time.Unix(5, 0), float64Pointer(100), - }, tp{ - time.Unix(10, 0), float64Pointer(99), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", nil, tp{ // Not sure about preservering names... + time.Unix(5, 0), float64Pointer(100), + }, tp{ + time.Unix(10, 0), float64Pointer(99), + }), + ), }, { name: "series Op series", @@ -73,15 +67,13 @@ func TestSeriesExpr(t *testing.T) { vars: aSeries, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ // Not sure about preservering names... - time.Unix(5, 0), float64Pointer(4), - }, tp{ - time.Unix(10, 0), float64Pointer(2), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", nil, tp{ // Not sure about preservering names... + time.Unix(5, 0), float64Pointer(4), + }, tp{ + time.Unix(10, 0), float64Pointer(2), + }), + ), }, { name: "series Op number", @@ -89,15 +81,13 @@ func TestSeriesExpr(t *testing.T) { vars: aSeriesbNumber, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", data.Labels{"id": "1"}, tp{ - time.Unix(5, 0), float64Pointer(9), - }, tp{ - time.Unix(10, 0), float64Pointer(8), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", data.Labels{"id": "1"}, tp{ + time.Unix(5, 0), float64Pointer(9), + }, tp{ + time.Unix(10, 0), float64Pointer(8), + }), + ), }, { name: "number Op series", @@ -105,15 +95,13 @@ func TestSeriesExpr(t *testing.T) { vars: aSeriesbNumber, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", data.Labels{"id": "1"}, tp{ - time.Unix(5, 0), float64Pointer(9), - }, tp{ - time.Unix(10, 0), float64Pointer(8), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", data.Labels{"id": "1"}, tp{ + time.Unix(5, 0), float64Pointer(9), + }, tp{ + time.Unix(10, 0), float64Pointer(8), + }), + ), }, { name: "series Op series with label union", @@ -121,20 +109,18 @@ func TestSeriesExpr(t *testing.T) { vars: twoSeriesSets, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", data.Labels{"sensor": "a", "turbine": "1"}, tp{ - time.Unix(5, 0), float64Pointer(6 * .5), - }, tp{ - time.Unix(10, 0), float64Pointer(8 * .2), - }), - makeSeries("", data.Labels{"sensor": "b", "turbine": "1"}, tp{ - time.Unix(5, 0), float64Pointer(10 * .5), - }, tp{ - time.Unix(10, 0), float64Pointer(16 * .2), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", data.Labels{"sensor": "a", "turbine": "1"}, tp{ + time.Unix(5, 0), float64Pointer(6 * .5), + }, tp{ + time.Unix(10, 0), float64Pointer(8 * .2), + }), + makeSeries("", data.Labels{"sensor": "b", "turbine": "1"}, tp{ + time.Unix(5, 0), float64Pointer(10 * .5), + }, tp{ + time.Unix(10, 0), float64Pointer(16 * .2), + }), + ), }, // Length of resulting series is A when A + B. However, only points where the time matches // for A and B are added to the result @@ -142,35 +128,29 @@ func TestSeriesExpr(t *testing.T) { name: "series Op series with sparse time join", expr: "$A + $B", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("temp", data.Labels{}, tp{ - time.Unix(5, 0), float64Pointer(1), - }, tp{ - time.Unix(10, 0), float64Pointer(2), - }), - }, - }, - "B": Results{ - []Value{ - makeSeries("efficiency", data.Labels{}, tp{ - time.Unix(5, 0), float64Pointer(3), - }, tp{ - time.Unix(9, 0), float64Pointer(4), - }), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", data.Labels{}, tp{ + time.Unix(5, 0), float64Pointer(1), + }, tp{ + time.Unix(10, 0), float64Pointer(2), + }), + ), + "B": resultValuesNoErr( + makeSeries("efficiency", data.Labels{}, tp{ + time.Unix(5, 0), float64Pointer(3), + }, tp{ + time.Unix(9, 0), float64Pointer(4), + }), + ), }, newErrIs: assert.NoError, execErrIs: assert.NoError, - results: Results{ - []Value{ - makeSeries("", nil, tp{ // Not sure about preserving names... - time.Unix(5, 0), float64Pointer(4), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", nil, tp{ // Not sure about preserving names... + time.Unix(5, 0), float64Pointer(4), + }), + ), }, } diff --git a/pkg/expr/mathexp/exp_test.go b/pkg/expr/mathexp/exp_test.go index 85936c2ca74..fbe6bf5c205 100644 --- a/pkg/expr/mathexp/exp_test.go +++ b/pkg/expr/mathexp/exp_test.go @@ -48,59 +48,56 @@ func boolPointer(b bool) *bool { return &b } +func resultValuesNoErr(v ...Value) Results { + return Results{ + Values: v, + Error: nil, + } +} + var aSeries = Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), float64Pointer(1), - }), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", nil, tp{ + time.Unix(5, 0), float64Pointer(2), + }, tp{ + time.Unix(10, 0), float64Pointer(1), + }), + ), } var aSeriesbNumber = Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), float64Pointer(1), - }), - }, - }, - "B": Results{ - []Value{ - makeNumber("volt", data.Labels{"id": "1"}, float64Pointer(7)), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", nil, tp{ + time.Unix(5, 0), float64Pointer(2), + }, tp{ + time.Unix(10, 0), float64Pointer(1), + }), + ), + "B": resultValuesNoErr( + makeNumber("volt", data.Labels{"id": "1"}, float64Pointer(7)), + ), } var twoSeriesSets = Vars{ - "A": Results{ - []Value{ - makeSeries("temp", data.Labels{"sensor": "a", "turbine": "1"}, tp{ - time.Unix(5, 0), float64Pointer(6), - }, tp{ - time.Unix(10, 0), float64Pointer(8), - }), - makeSeries("temp", data.Labels{"sensor": "b", "turbine": "1"}, tp{ - time.Unix(5, 0), float64Pointer(10), - }, tp{ - time.Unix(10, 0), float64Pointer(16), - }), - }, - }, - "B": Results{ - []Value{ - makeSeries("efficiency", data.Labels{"turbine": "1"}, tp{ - time.Unix(5, 0), float64Pointer(.5), - }, tp{ - time.Unix(10, 0), float64Pointer(.2), - }), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", data.Labels{"sensor": "a", "turbine": "1"}, tp{ + time.Unix(5, 0), float64Pointer(6), + }, tp{ + time.Unix(10, 0), float64Pointer(8), + }), + makeSeries("temp", data.Labels{"sensor": "b", "turbine": "1"}, tp{ + time.Unix(5, 0), float64Pointer(10), + }, tp{ + time.Unix(10, 0), float64Pointer(16), + }), + ), + "B": resultValuesNoErr( + makeSeries("efficiency", data.Labels{"turbine": "1"}, tp{ + time.Unix(5, 0), float64Pointer(.5), + }, tp{ + time.Unix(10, 0), float64Pointer(.2), + }), + ), } // NaN is just to make the calls a little cleaner, the one diff --git a/pkg/expr/mathexp/funcs_test.go b/pkg/expr/mathexp/funcs_test.go index ab0b14f6e36..56d8e41d520 100644 --- a/pkg/expr/mathexp/funcs_test.go +++ b/pkg/expr/mathexp/funcs_test.go @@ -23,16 +23,12 @@ func TestAbsFunc(t *testing.T) { name: "abs on number", expr: "abs($A)", vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, float64Pointer(-7)), - }, - }, + "A": resultValuesNoErr(makeNumber("", nil, float64Pointer(-7))), }, newErrIs: require.NoError, execErrIs: require.NoError, resultIs: require.Equal, - results: Results{[]Value{makeNumber("", nil, float64Pointer(7))}}, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(7))), }, { name: "abs on scalar", @@ -41,34 +37,30 @@ func TestAbsFunc(t *testing.T) { newErrIs: require.NoError, execErrIs: require.NoError, resultIs: require.Equal, - results: Results{[]Value{NewScalar("", float64Pointer(1.0))}}, + results: resultValuesNoErr(NewScalar("", float64Pointer(1.0))), }, { name: "abs on series", expr: "abs($A)", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(-2), - }, tp{ - time.Unix(10, 0), float64Pointer(-1), - }), - }, - }, + "A": resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(-2), + }, tp{ + time.Unix(10, 0), float64Pointer(-1), + }), + ), }, newErrIs: require.NoError, execErrIs: require.NoError, resultIs: require.Equal, - results: Results{ - []Value{ - makeSeries("", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), float64Pointer(1), - }), - }, - }, + results: resultValuesNoErr( + makeSeries("", nil, tp{ + time.Unix(5, 0), float64Pointer(2), + }, tp{ + time.Unix(10, 0), float64Pointer(1), + }), + ), }, { name: "abs on string - should error", @@ -101,51 +93,39 @@ func TestIsNumberFunc(t *testing.T) { name: "is_number on number type with real number value", expr: "is_number($A)", vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, float64Pointer(6)), - }, - }, + "A": resultValuesNoErr(makeNumber("", nil, float64Pointer(6))), }, - results: Results{[]Value{makeNumber("", nil, float64Pointer(1))}}, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(1))), }, { name: "is_number on number type with null value", expr: "is_number($A)", vars: Vars{ - "A": Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, + "A": resultValuesNoErr(makeNumber("", nil, nil)), }, - results: Results{[]Value{makeNumber("", nil, float64Pointer(0))}}, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(0))), }, { name: "is_number on on series", expr: "is_number($A)", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("", nil, - tp{time.Unix(5, 0), float64Pointer(5)}, - tp{time.Unix(10, 0), nil}, - tp{time.Unix(15, 0), float64Pointer(math.NaN())}, - tp{time.Unix(20, 0), float64Pointer(math.Inf(-1))}, - tp{time.Unix(25, 0), float64Pointer(math.Inf(0))}), - }, - }, - }, - results: Results{ - []Value{ + "A": resultValuesNoErr( makeSeries("", nil, - tp{time.Unix(5, 0), float64Pointer(1)}, - tp{time.Unix(10, 0), float64Pointer(0)}, - tp{time.Unix(15, 0), float64Pointer(0)}, - tp{time.Unix(20, 0), float64Pointer(0)}, - tp{time.Unix(25, 0), float64Pointer(0)}), - }, + tp{time.Unix(5, 0), float64Pointer(5)}, + tp{time.Unix(10, 0), nil}, + tp{time.Unix(15, 0), float64Pointer(math.NaN())}, + tp{time.Unix(20, 0), float64Pointer(math.Inf(-1))}, + tp{time.Unix(25, 0), float64Pointer(math.Inf(0))}), + ), }, + results: resultValuesNoErr( + makeSeries("", nil, + tp{time.Unix(5, 0), float64Pointer(1)}, + tp{time.Unix(10, 0), float64Pointer(0)}, + tp{time.Unix(15, 0), float64Pointer(0)}, + tp{time.Unix(20, 0), float64Pointer(0)}, + tp{time.Unix(25, 0), float64Pointer(0)}), + ), }, } for _, tt := range tests { diff --git a/pkg/expr/mathexp/reduce_test.go b/pkg/expr/mathexp/reduce_test.go index e4576e555d7..2f9c7a2a81f 100644 --- a/pkg/expr/mathexp/reduce_test.go +++ b/pkg/expr/mathexp/reduce_test.go @@ -12,23 +12,19 @@ import ( ) var seriesWithNil = Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), nil, - }), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", nil, tp{ + time.Unix(5, 0), float64Pointer(2), + }, tp{ + time.Unix(10, 0), nil, + }), + ), } var seriesEmpty = Vars{ - "A": Results{ - []Value{ - makeSeries("temp", nil), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", nil), + ), } func TestSeriesReduce(t *testing.T) { @@ -56,11 +52,7 @@ func TestSeriesReduce(t *testing.T) { vars: aSeries, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(3)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(3))), }, { name: "sum series with a nil value", @@ -69,11 +61,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesWithNil, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "sum empty series", @@ -82,11 +70,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesEmpty, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(0)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(0))), }, { name: "mean series with a nil value", @@ -95,11 +79,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesWithNil, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "mean empty series", @@ -108,11 +88,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesEmpty, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "min series with a nil value", @@ -121,11 +97,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesWithNil, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "min empty series", @@ -134,11 +106,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesEmpty, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "max series with a nil value", @@ -147,11 +115,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesWithNil, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "max empty series", @@ -160,11 +124,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesEmpty, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "mean series", @@ -173,11 +133,7 @@ func TestSeriesReduce(t *testing.T) { vars: aSeries, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(1.5)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(1.5))), }, { name: "count empty series", @@ -186,34 +142,24 @@ func TestSeriesReduce(t *testing.T) { vars: seriesEmpty, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(0)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(0))), }, { name: "mean series with labels", red: "mean", varToReduce: "A", vars: Vars{ - "A": Results{ - []Value{ - makeSeries("temp", data.Labels{"host": "a"}, tp{ - time.Unix(5, 0), float64Pointer(2), - }, tp{ - time.Unix(10, 0), float64Pointer(1), - }), - }, - }, + "A": resultValuesNoErr( + makeSeries("temp", data.Labels{"host": "a"}, tp{ + time.Unix(5, 0), float64Pointer(2), + }, tp{ + time.Unix(10, 0), float64Pointer(1), + }), + ), }, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", data.Labels{"host": "a"}, float64Pointer(1.5)), - }, - }, + results: resultValuesNoErr(makeNumber("", data.Labels{"host": "a"}, float64Pointer(1.5))), }, { name: "last empty series", @@ -222,11 +168,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesEmpty, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, NaN), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, NaN)), }, { name: "last null series", @@ -235,11 +177,7 @@ func TestSeriesReduce(t *testing.T) { vars: seriesWithNil, errIs: require.NoError, resultsIs: require.Equal, - results: Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, nil)), }, } @@ -267,15 +205,13 @@ 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}), - }, - }, + "A": resultValuesNoErr( + 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) { @@ -291,88 +227,56 @@ func TestSeriesReduceDropNN(t *testing.T) { red: "sum", varToReduce: "A", vars: aSeries, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(3)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(2))), }, { name: "dropNN: sum empty series", red: "sum", varToReduce: "A", vars: seriesEmpty, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(0)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(2))), }, { name: "DropNN: mean empty series", red: "mean", varToReduce: "A", vars: seriesEmpty, - results: Results{ - []Value{ - makeNumber("", nil, nil), - }, - }, + results: resultValuesNoErr(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), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, nil)), }, { name: "DropNN: count empty series", red: "count", varToReduce: "A", vars: seriesEmpty, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(0)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(1))), }, } @@ -410,88 +314,56 @@ func TestSeriesReduceReplaceNN(t *testing.T) { red: "sum", varToReduce: "A", vars: aSeries, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(3)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(replaceWith+2))), }, { name: "replaceNN: sum empty series", red: "sum", varToReduce: "A", vars: seriesEmpty, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(0)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer((2+replaceWith)/2e0))), }, { name: "replaceNN: mean empty series", red: "mean", varToReduce: "A", vars: seriesEmpty, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(replaceWith)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(replaceWith))), }, { name: "replaceNN: count empty series", red: "count", varToReduce: "A", vars: seriesEmpty, - results: Results{ - []Value{ - makeNumber("", nil, float64Pointer(0)), - }, - }, + results: resultValuesNoErr(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)), - }, - }, + results: resultValuesNoErr(makeNumber("", nil, float64Pointer(2))), }, } diff --git a/pkg/expr/mathexp/types.go b/pkg/expr/mathexp/types.go index 48ad72fac4b..38a975caed8 100644 --- a/pkg/expr/mathexp/types.go +++ b/pkg/expr/mathexp/types.go @@ -10,6 +10,7 @@ import ( // Results is a container for Value interfaces. type Results struct { Values Values + Error error } // Values is a slice of Value interfaces diff --git a/pkg/expr/ml.go b/pkg/expr/ml.go index 27b9df49e0f..3deca1714a3 100644 --- a/pkg/expr/ml.go +++ b/pkg/expr/ml.go @@ -49,6 +49,11 @@ func (m *MLNode) NodeType() NodeType { return TypeMLNode } +// NodeType returns the data pipeline node type. +func (m *MLNode) NeedsVars() []string { + return []string{} +} + // Execute initializes plugin API client, executes a ml.Command and then converts the result of the execution. // Returns non-empty mathexp.Results if evaluation was successful. Returns QueryError if command execution failed func (m *MLNode) Execute(ctx context.Context, now time.Time, _ mathexp.Vars, s *Service) (r mathexp.Results, e error) { diff --git a/pkg/expr/ml/node.go b/pkg/expr/ml/node.go index 096ce77d239..771a19e4644 100644 --- a/pkg/expr/ml/node.go +++ b/pkg/expr/ml/node.go @@ -39,7 +39,7 @@ func UnmarshalCommand(query []byte, appURL string) (Command, error) { var expr CommandConfiguration err := json.Unmarshal(query, &expr) if err != nil { - return nil, fmt.Errorf("failed to unmarshall Machine learning command: %w", err) + return nil, fmt.Errorf("failed to unmarshal Machine learning command: %w", err) } if len(expr.Type) == 0 { return nil, fmt.Errorf("required field 'type' is not specified or empty. Should be one of [%s]", Outlier) diff --git a/pkg/expr/ml/node_test.go b/pkg/expr/ml/node_test.go index e62faff0aa3..3e3c2da7fd7 100644 --- a/pkg/expr/ml/node_test.go +++ b/pkg/expr/ml/node_test.go @@ -82,7 +82,7 @@ func TestUnmarshalCommand(t *testing.T) { "data": 1, } }), - err: "failed to unmarshall Machine learning command", + err: "failed to unmarshal Machine learning command", }, { name: "field 'config' is missing", @@ -96,7 +96,7 @@ func TestUnmarshalCommand(t *testing.T) { config: updateJson(outlierQuery, func(cmd map[string]interface{}) { cmd["intervalMs"] = "test" }), - err: "failed to unmarshall Machine learning command", + err: "failed to unmarshal Machine learning command", }, { name: "field 'config.datasource_uid' is not specified", diff --git a/pkg/expr/nodes.go b/pkg/expr/nodes.go index f9601bccfd9..f03bd77c01d 100644 --- a/pkg/expr/nodes.go +++ b/pkg/expr/nodes.go @@ -85,6 +85,10 @@ func (gn *CMDNode) NodeType() NodeType { return TypeCMDNode } +func (gn *CMDNode) NeedsVars() []string { + return gn.Command.NeedsVars() +} + // Execute runs the node and adds the results to vars. If the node requires // other nodes they must have already been executed and their results must // already by in vars. @@ -151,6 +155,11 @@ func (dn *DSNode) NodeType() NodeType { return TypeDatasourceNode } +// NodeType returns the data pipeline node type. +func (dn *DSNode) NeedsVars() []string { + return []string{} +} + func (s *Service) buildDSNode(dp *simple.DirectedGraph, rn *rawNode, req *Request) (*DSNode, error) { if rn.TimeRange == nil { return nil, fmt.Errorf("time range must be specified for refID %s", rn.RefID) @@ -196,7 +205,7 @@ func (s *Service) buildDSNode(dp *simple.DirectedGraph, rn *rawNode, req *Reques // executeDSNodesGrouped groups datasource node queries by the datasource instance, and then sends them // in a single request with one or more queries to the datasource. -func executeDSNodesGrouped(ctx context.Context, now time.Time, vars mathexp.Vars, s *Service, nodes []*DSNode) (e error) { +func executeDSNodesGrouped(ctx context.Context, now time.Time, vars mathexp.Vars, s *Service, nodes []*DSNode) { type dsKey struct { uid string // in theory I think this all I need for the key, but rather be safe id int64 @@ -209,13 +218,16 @@ func executeDSNodesGrouped(ctx context.Context, now time.Time, vars mathexp.Vars } for _, nodeGroup := range byDS { - if err := func() error { + func() { ctx, span := s.tracer.Start(ctx, "SSE.ExecuteDatasourceQuery") defer span.End() firstNode := nodeGroup[0] pCtx, err := s.pCtxProvider.GetWithDataSource(ctx, firstNode.datasource.Type, firstNode.request.User, firstNode.datasource) if err != nil { - return err + for _, dn := range nodeGroup { + vars[dn.refID] = mathexp.Results{Error: datasources.ErrDataSourceNotFound} + } + return } logger := logger.FromContext(ctx).New("datasourceType", firstNode.datasource.Type, @@ -243,9 +255,9 @@ func executeDSNodesGrouped(ctx context.Context, now time.Time, vars mathexp.Vars }) } - responseType := "unknown" - respStatus := "success" - defer func() { + instrument := func(e error, rt string) { + respStatus := "success" + responseType := rt if e != nil { responseType = "error" respStatus = "failure" @@ -258,32 +270,35 @@ func executeDSNodesGrouped(ctx context.Context, now time.Time, vars mathexp.Vars logger.Debug("Data source queried", "responseType", responseType) useDataplane := strings.HasPrefix(responseType, "dataplane-") s.metrics.dsRequests.WithLabelValues(respStatus, fmt.Sprintf("%t", useDataplane), firstNode.datasource.Type).Inc() - }() + } resp, err := s.dataService.QueryData(ctx, req) if err != nil { - return MakeQueryError(firstNode.refID, firstNode.datasource.UID, err) + for _, dn := range nodeGroup { + vars[dn.refID] = mathexp.Results{Error: MakeQueryError(firstNode.refID, firstNode.datasource.UID, err)} + } + instrument(err, "") + return } for _, dn := range nodeGroup { dataFrames, err := getResponseFrame(resp, dn.refID) if err != nil { - return MakeQueryError(dn.refID, dn.datasource.UID, err) + vars[dn.refID] = mathexp.Results{Error: MakeQueryError(dn.refID, dn.datasource.UID, err)} + instrument(err, "") + return } var result mathexp.Results - responseType, result, err = convertDataFramesToResults(ctx, dataFrames, dn.datasource.Type, s, logger) + responseType, result, err := convertDataFramesToResults(ctx, dataFrames, dn.datasource.Type, s, logger) if err != nil { - return MakeConversionError(dn.refID, err) + result.Error = makeConversionError(dn.RefID(), err) } + instrument(err, responseType) vars[dn.refID] = result } - return nil - }(); err != nil { - return err - } + }() } - return nil } // Execute runs the node and adds the results to vars. If the node requires @@ -346,7 +361,7 @@ func (dn *DSNode) Execute(ctx context.Context, now time.Time, _ mathexp.Vars, s var result mathexp.Results responseType, result, err = convertDataFramesToResults(ctx, dataFrames, dn.datasource.Type, s, logger) if err != nil { - err = MakeConversionError(dn.refID, err) + err = makeConversionError(dn.refID, err) } return result, err } diff --git a/pkg/expr/service.go b/pkg/expr/service.go index 63fca9e83a4..ffd7e3ffd66 100644 --- a/pkg/expr/service.go +++ b/pkg/expr/service.go @@ -109,6 +109,7 @@ func (s *Service) ExecutePipeline(ctx context.Context, now time.Time, pipeline D for refID, val := range vars { res.Responses[refID] = backend.DataResponse{ Frames: val.Values.AsDataFrames(refID), + Error: val.Error, } } return res, nil diff --git a/pkg/expr/service_test.go b/pkg/expr/service_test.go index b415ad0267e..f4e1d347543 100644 --- a/pkg/expr/service_test.go +++ b/pkg/expr/service_test.go @@ -3,6 +3,7 @@ package expr import ( "context" "encoding/json" + "fmt" "sort" "testing" "time" @@ -21,6 +22,7 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util/errutil" ) func TestService(t *testing.T) { @@ -29,7 +31,9 @@ func TestService(t *testing.T) { data.NewField("value", data.Labels{"test": "label"}, []*float64{fp(2)})) me := &mockEndpoint{ - Frames: []*data.Frame{dsDF}, + Responses: map[string]backend.DataResponse{ + "A": {Frames: data.Frames{dsDF}}, + }, } pCtxProvider := plugincontext.ProvideService(nil, &pluginstore.FakePluginStore{ @@ -110,18 +114,82 @@ func TestService(t *testing.T) { } } +func TestDSQueryError(t *testing.T) { + me := &mockEndpoint{ + Responses: map[string]backend.DataResponse{ + "A": {Error: fmt.Errorf("womp womp")}, + "B": {Frames: data.Frames{}}, + }, + } + + pCtxProvider := plugincontext.ProvideService(nil, &pluginstore.FakePluginStore{ + PluginList: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "test"}}, + }, + }, &datafakes.FakeDataSourceService{}, nil) + + s := Service{ + cfg: setting.NewCfg(), + dataService: me, + pCtxProvider: pCtxProvider, + features: &featuremgmt.FeatureManager{}, + tracer: tracing.InitializeTracerForTest(), + metrics: newMetrics(nil), + } + + queries := []Query{ + { + RefID: "A", + DataSource: &datasources.DataSource{ + OrgID: 1, + UID: "test", + Type: "test", + }, + JSON: json.RawMessage(`{ "datasource": { "uid": "1" }, "intervalMs": 1000, "maxDataPoints": 1000 }`), + TimeRange: AbsoluteTimeRange{ + From: time.Time{}, + To: time.Time{}, + }, + }, + { + RefID: "B", + DataSource: dataSourceModel(), + JSON: json.RawMessage(`{ "datasource": { "uid": "__expr__", "type": "__expr__"}, "type": "math", "expression": "$A * 2" }`), + }, + { + RefID: "C", + DataSource: dataSourceModel(), + JSON: json.RawMessage(`{ "datasource": { "uid": "__expr__", "type": "__expr__"}, "type": "math", "expression": "42" }`), + }, + } + + req := &Request{Queries: queries, User: &user.SignedInUser{}} + + pl, err := s.BuildPipeline(req) + require.NoError(t, err) + + resp, err := s.ExecutePipeline(context.Background(), time.Now(), pl) + require.NoError(t, err) + + var utilErr errutil.Error + require.ErrorContains(t, resp.Responses["A"].Error, "womp womp") + require.ErrorAs(t, resp.Responses["B"].Error, &utilErr) + require.ErrorIs(t, utilErr, DependencyError) + require.Equal(t, fp(42), resp.Responses["C"].Frames[0].Fields[0].At(0)) +} + func fp(f float64) *float64 { return &f } type mockEndpoint struct { - Frames data.Frames + Responses map[string]backend.DataResponse } func (me *mockEndpoint) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { resp := backend.NewQueryDataResponse() - resp.Responses["A"] = backend.DataResponse{ - Frames: me.Frames, + for _, ref := range req.Queries { + resp.Responses[ref.RefID] = me.Responses[ref.RefID] } return resp, nil } diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index ba5beb78ae1..5e554c70434 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -23,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util/errutil" ) var logger = log.New("ngalert.eval") @@ -134,6 +135,9 @@ type ExecutionResults struct { // Results contains the results of all queries, reduce and math expressions Results map[string]data.Frames + // Errors contains a map of RefIDs that returned an error + Errors map[string]error + // NoData contains the DatasourceUID for RefIDs that returned no data. NoData map[string]string @@ -323,6 +327,7 @@ type NumberValueCapture struct { Value *float64 } +//nolint:gocyclo func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.QueryDataResponse) ExecutionResults { // captures contains the values of all instant queries and expressions for each dimension captures := make(map[string]map[data.Fingerprint]NumberValueCapture) @@ -349,6 +354,16 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q result := ExecutionResults{Results: make(map[string]data.Frames)} for refID, res := range execResp.Responses { + if res.Error != nil { + if result.Errors == nil { + result.Errors = make(map[string]error) + } + result.Errors[refID] = res.Error + if refID == c.Condition { + result.Error = res.Error + } + } + // There are two possible frame formats for No Data: // // 1. A response with no frames @@ -431,6 +446,29 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q } } + // If the error of the condition is an Error that indicates the condition failed + // because one of its dependent query or expressions failed, then we follow + // the dependency chain to an error that is not a dependency error. + if len(result.Errors) > 0 && result.Error != nil { + if errors.Is(result.Error, expr.DependencyError) { + var utilError errutil.Error + e := result.Error + for { + errors.As(e, &utilError) + depRefID := utilError.PublicPayload["depRefId"].(string) + depError, ok := result.Errors[depRefID] + if !ok { + return result + } + if !errors.Is(depError, expr.DependencyError) { + result.Error = depError + return result + } + e = depError + } + } + } + return result } diff --git a/public/app/features/alerting/unified/components/expressions/Expression.tsx b/public/app/features/alerting/unified/components/expressions/Expression.tsx index 45d125558b0..54fcf984a94 100644 --- a/public/app/features/alerting/unified/components/expressions/Expression.tsx +++ b/public/app/features/alerting/unified/components/expressions/Expression.tsx @@ -4,7 +4,7 @@ import React, { FC, useCallback, useState } from 'react'; import { DataFrame, dateTimeFormat, GrafanaTheme2, isTimeSeriesFrames, LoadingState, PanelData } from '@grafana/data'; import { Stack } from '@grafana/experimental'; -import { AutoSizeInput, Button, clearButtonStyles, IconButton, useStyles2 } from '@grafana/ui'; +import { AutoSizeInput, Badge, Button, clearButtonStyles, IconButton, useStyles2 } from '@grafana/ui'; import { ClassicConditions } from 'app/features/expressions/components/ClassicConditions'; import { Math } from 'app/features/expressions/components/Math'; import { Reduce } from 'app/features/expressions/components/Reduce'; @@ -302,6 +302,10 @@ const Header: FC = ({
{getExpressionLabel(queryType)}
+ {/* when we have an evaluation error, we show a badge next to "set as alert condition" */} + {!alertCondition && error && ( + + )} onSetCondition(query.refId)} enabled={alertCondition} diff --git a/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx b/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx index 69f7ad7a69b..f8c507d2334 100644 --- a/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx @@ -9,7 +9,7 @@ import { AlertQuery } from 'app/types/unified-alerting-dto'; import { Expression } from '../expressions/Expression'; -import { errorFromSeries, warningFromSeries } from './util'; +import { errorFromPreviewData, warningFromSeries } from './util'; interface Props { condition: string | null; @@ -45,7 +45,7 @@ export const ExpressionsEditor = ({ const data = panelData[query.refId]; const isAlertCondition = condition === query.refId; - const error = isAlertCondition && data ? errorFromSeries(data.series) : undefined; + const error = data ? errorFromPreviewData(data) : undefined; const warning = isAlertCondition && data ? warningFromSeries(data.series) : undefined; return ( diff --git a/public/app/features/alerting/unified/components/rule-editor/QueryRows.tsx b/public/app/features/alerting/unified/components/rule-editor/QueryRows.tsx index 426d879ce85..bffc5b74301 100644 --- a/public/app/features/alerting/unified/components/rule-editor/QueryRows.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/QueryRows.tsx @@ -18,7 +18,7 @@ import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto'; import { AlertQueryOptions, EmptyQueryWrapper, QueryWrapper } from './QueryWrapper'; -import { errorFromSeries, getThresholdsForQueries } from './util'; +import { errorFromPreviewData, getThresholdsForQueries } from './util'; interface Props { // The query configuration @@ -161,9 +161,7 @@ export class QueryRows extends PureComponent { state: LoadingState.NotStarted, }; const dsSettings = this.getDataSourceSettings(query); - - const isAlertCondition = this.props.condition === query.refId; - const error = isAlertCondition ? errorFromSeries(data.series) : undefined; + const error = data ? errorFromPreviewData(data) : undefined; if (!dsSettings) { return ( diff --git a/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx b/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx index e5e24829dcf..f7323d738bb 100644 --- a/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx @@ -14,7 +14,7 @@ import { } from '@grafana/data'; import { Stack } from '@grafana/experimental'; import { DataQuery } from '@grafana/schema'; -import { GraphTresholdsStyleMode, Icon, InlineField, Input, Tooltip, useStyles2 } from '@grafana/ui'; +import { Badge, GraphTresholdsStyleMode, Icon, InlineField, Input, Tooltip, useStyles2 } from '@grafana/ui'; import { QueryEditorRow } from 'app/features/query/components/QueryEditorRow'; import { AlertQuery } from 'app/types/unified-alerting-dto'; @@ -119,6 +119,8 @@ export const QueryWrapper = ({ minInterval: queryOptions.minInterval, }; + const isAlertCondition = condition === query.refId; + return ( @@ -132,9 +134,12 @@ export const QueryWrapper = ({ onSetCondition(query.refId)} - enabled={condition === query.refId} + enabled={isAlertCondition} error={error} /> + {!isAlertCondition && error && ( + + )} ); } diff --git a/public/app/features/alerting/unified/components/rule-editor/util.ts b/public/app/features/alerting/unified/components/rule-editor/util.ts index ab798d83996..db18a60a49b 100644 --- a/public/app/features/alerting/unified/components/rule-editor/util.ts +++ b/public/app/features/alerting/unified/components/rule-editor/util.ts @@ -110,6 +110,16 @@ export function errorFromSeries(series: DataFrame[]): Error | undefined { return error; } +export function errorFromPreviewData(data: PanelData): Error | undefined { + // give preference to QueryErrors + if (data.errors?.length) { + return new Error(data.errors[0].message); + } + + // if none, return errors from series + return errorFromSeries(data.series); +} + export function warningFromSeries(series: DataFrame[]): Error | undefined { const notices = series[0]?.meta?.notices ?? []; const warning = notices.find((notice) => notice.severity === 'warning')?.text; diff --git a/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts b/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts index 324083d74b2..5177063eaf0 100644 --- a/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts +++ b/public/app/features/alerting/unified/state/AlertingQueryRunner.test.ts @@ -45,6 +45,7 @@ describe('AlertingQueryRunner', () => { A: { annotations: [], state: LoadingState.Done, + errors: [], series: [ expectDataFrameWithValues({ time: [1620051612238, 1620051622238, 1620051632238], @@ -60,6 +61,7 @@ describe('AlertingQueryRunner', () => { B: { annotations: [], state: LoadingState.Done, + errors: [], series: [ expectDataFrameWithValues({ time: [1620051612238, 1620051622238], @@ -136,6 +138,7 @@ describe('AlertingQueryRunner', () => { A: { annotations: [], state: LoadingState.Done, + errors: [], series: [ expectDataFrameWithValues({ time: [1620051612238, 1620051622238, 1620051632238], @@ -151,6 +154,7 @@ describe('AlertingQueryRunner', () => { B: { annotations: [], state: LoadingState.Done, + errors: [], series: [ expectDataFrameWithValues({ time: [1620051612238, 1620051622238], diff --git a/public/app/features/alerting/unified/state/AlertingQueryRunner.ts b/public/app/features/alerting/unified/state/AlertingQueryRunner.ts index e4f4ad23e7e..45950f25202 100644 --- a/public/app/features/alerting/unified/state/AlertingQueryRunner.ts +++ b/public/app/features/alerting/unified/state/AlertingQueryRunner.ts @@ -24,6 +24,8 @@ import { AlertQuery } from 'app/types/unified-alerting-dto'; import { getTimeRangeForExpression } from '../utils/timeRange'; export interface AlertingQueryResult { + error?: string; + status?: number; // HTTP status error frames: DataFrameJSON[]; } @@ -183,10 +185,16 @@ const mapToPanelData = ( const results: Record = {}; for (const [refId, result] of Object.entries(data.results)) { + const { error, status, frames = [] } = result; + + // extract errors from the /eval results + const errors = error ? [{ message: error, refId, status }] : []; + results[refId] = { + errors, timeRange: dataByQuery[refId].timeRange, state: LoadingState.Done, - series: result.frames.map(dataFrameFromJSON), + series: frames.map(dataFrameFromJSON), }; }