From d411a7d81044a62205c4058f50a4db5e3f054679 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 8 Apr 2014 16:14:35 +0200 Subject: [PATCH] Allow reversing vector and scalar arguments in binops. This allows putting a scalar as the first argument of a binary operator in which the second argument is a vector: For example, 1 / http_requests_total ...will output a vector in which every sample value is 1 divided by the respective input vector element. This even works for filter binary operators now: 1 == http_requests_total Returns a vector with all values set to 1 for every element in http_requests_total whose initial value was 1. Note: For filter binary operators, the resulting values are always taken from the left-hand-side of the operation, no matter whether the scalar or the vector argument is the left-hand-side. That is, 1 != http_requests_total ...will set all result vector sample values to 1, although these are exactly the sample elements that were != 1 in the input vector. If you want to just filter elements without changing their sample values, you still need to do: http_requests_total != 1 The new filter form is a bit exotic, and so probably won't be used often. But it was easier to implement it than disallow it completely or change its behavior. Change-Id: Idd083f2bd3a1219ba1560cf4ace42f5b82e797a5 --- rules/ast/ast.go | 30 ++++++++++++++++++++---------- rules/rules_test.go | 31 +++++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/rules/ast/ast.go b/rules/ast/ast.go index 42391d284..a0dbab33b 100644 --- a/rules/ast/ast.go +++ b/rules/ast/ast.go @@ -195,11 +195,12 @@ type ( vector VectorNode } - // VectorArithExpr represents an arithmetic expression of - // vector type. + // VectorArithExpr represents an arithmetic expression of vector type. At + // least one of the two operand Nodes must be a VectorNode. The other may be + // a VectorNode or ScalarNode. Both criteria are checked at runtime. VectorArithExpr struct { opType BinOpType - lhs VectorNode + lhs Node rhs Node } ) @@ -639,9 +640,20 @@ func labelsEqual(labels1, labels2 clientmodel.Metric) bool { // Eval implements the VectorNode interface and returns the result of // the expression. func (node *VectorArithExpr) Eval(timestamp clientmodel.Timestamp, view *viewAdapter) Vector { - lhs := node.lhs.Eval(timestamp, view) result := Vector{} - if node.rhs.Type() == SCALAR { + if node.lhs.Type() == SCALAR && node.rhs.Type() == VECTOR { + lhs := node.lhs.(ScalarNode).Eval(timestamp, view) + rhs := node.rhs.(VectorNode).Eval(timestamp, view) + for _, rhsSample := range rhs { + value, keep := evalVectorBinop(node.opType, lhs, rhsSample.Value) + if keep { + rhsSample.Value = value + result = append(result, rhsSample) + } + } + return result + } else if node.lhs.Type() == VECTOR && node.rhs.Type() == SCALAR { + lhs := node.lhs.(VectorNode).Eval(timestamp, view) rhs := node.rhs.(ScalarNode).Eval(timestamp, view) for _, lhsSample := range lhs { value, keep := evalVectorBinop(node.opType, lhsSample.Value, rhs) @@ -651,7 +663,8 @@ func (node *VectorArithExpr) Eval(timestamp clientmodel.Timestamp, view *viewAda } } return result - } else if node.rhs.Type() == VECTOR { + } else if node.lhs.Type() == VECTOR && node.rhs.Type() == VECTOR { + lhs := node.lhs.(VectorNode).Eval(timestamp, view) rhs := node.rhs.(VectorNode).Eval(timestamp, view) for _, lhsSample := range lhs { for _, rhsSample := range rhs { @@ -804,9 +817,6 @@ func NewArithExpr(opType BinOpType, lhs Node, rhs Node) (Node, error) { if !nodesHaveTypes(Nodes{lhs, rhs}, []ExprType{SCALAR, VECTOR}) { return nil, errors.New("binary operands must be of vector or scalar type") } - if lhs.Type() == SCALAR && rhs.Type() == VECTOR { - return nil, errors.New("left side of vector binary operation must be of vector type") - } if opType == AND || opType == OR { if lhs.Type() == SCALAR || rhs.Type() == SCALAR { @@ -817,7 +827,7 @@ func NewArithExpr(opType BinOpType, lhs Node, rhs Node) (Node, error) { if lhs.Type() == VECTOR || rhs.Type() == VECTOR { return &VectorArithExpr{ opType: opType, - lhs: lhs.(VectorNode), + lhs: lhs, rhs: rhs, }, nil } diff --git a/rules/rules_test.go b/rules/rules_test.go index 2267ac421..9119bebb8 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -209,6 +209,22 @@ func TestExpressions(t *testing.T) { }, fullRanges: 0, intervalRanges: 8, + }, { + expr: `2 - SUM(http_requests) BY (job)`, + output: []string{ + `http_requests{job="api-server"} => -998 @[%v]`, + `http_requests{job="app-server"} => -2598 @[%v]`, + }, + fullRanges: 0, + intervalRanges: 8, + }, { + expr: `1000 / SUM(http_requests) BY (job)`, + output: []string{ + `http_requests{job="api-server"} => 1 @[%v]`, + `http_requests{job="app-server"} => 0.38461538461538464 @[%v]`, + }, + fullRanges: 0, + intervalRanges: 8, }, { expr: `SUM(http_requests) BY (job) - 2`, output: []string{ @@ -240,6 +256,13 @@ func TestExpressions(t *testing.T) { }, fullRanges: 0, intervalRanges: 8, + }, { + expr: `1000 < SUM(http_requests) BY (job)`, + output: []string{ + `http_requests{job="app-server"} => 1000 @[%v]`, + }, + fullRanges: 0, + intervalRanges: 8, }, { expr: `SUM(http_requests) BY (job) <= 1000`, output: []string{ @@ -398,14 +421,14 @@ func TestExpressions(t *testing.T) { // Empty expressions shouldn"t parse. expr: ``, shouldFail: true, - }, { - // Subtracting a vector from a scalar is not supported. - expr: `1 - http_requests`, - shouldFail: true, }, { // Interval durations can"t be in quotes. expr: `http_requests["1m"]`, shouldFail: true, + }, { + // Binop arguments need to be scalar or vector. + expr: `http_requests - http_requests[1m]`, + shouldFail: true, }, { expr: `http_requests{group!="canary"}`, output: []string{