diff --git a/promql/ast.go b/promql/ast.go index fa9812fdd..3f929bbe2 100644 --- a/promql/ast.go +++ b/promql/ast.go @@ -231,9 +231,9 @@ type VectorMatching struct { // MatchingLabels contains the labels which define equality of a pair of // elements from the vectors. MatchingLabels model.LabelNames - // Ignoring excludes the given label names from matching, - // rather than only using them. - Ignoring bool + // On includes the given label names from matching, + // rather than excluding them. + On bool // Include contains additional labels that should be included in // the result from the side with the lower cardinality. Include model.LabelNames diff --git a/promql/engine.go b/promql/engine.go index 8a7ad6d66..3a9fc8fac 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -728,7 +728,7 @@ func (ev *evaluator) vectorAnd(lhs, rhs vector, matching *VectorMatching) vector if matching.Card != CardManyToMany { panic("set operations must only use many-to-many matching") } - sigf := signatureFunc(matching.Ignoring, matching.MatchingLabels...) + sigf := signatureFunc(matching.On, matching.MatchingLabels...) var result vector // The set of signatures for the right-hand side vector. @@ -751,7 +751,7 @@ func (ev *evaluator) vectorOr(lhs, rhs vector, matching *VectorMatching) vector if matching.Card != CardManyToMany { panic("set operations must only use many-to-many matching") } - sigf := signatureFunc(matching.Ignoring, matching.MatchingLabels...) + sigf := signatureFunc(matching.On, matching.MatchingLabels...) var result vector leftSigs := map[uint64]struct{}{} @@ -773,7 +773,7 @@ func (ev *evaluator) vectorUnless(lhs, rhs vector, matching *VectorMatching) vec if matching.Card != CardManyToMany { panic("set operations must only use many-to-many matching") } - sigf := signatureFunc(matching.Ignoring, matching.MatchingLabels...) + sigf := signatureFunc(matching.On, matching.MatchingLabels...) rightSigs := map[uint64]struct{}{} for _, rs := range rhs { @@ -796,7 +796,7 @@ func (ev *evaluator) vectorBinop(op itemType, lhs, rhs vector, matching *VectorM } var ( result = vector{} - sigf = signatureFunc(matching.Ignoring, matching.MatchingLabels...) + sigf = signatureFunc(matching.On, matching.MatchingLabels...) ) // The control flow below handles one-to-one or many-to-one matching. @@ -882,9 +882,9 @@ func (ev *evaluator) vectorBinop(op itemType, lhs, rhs vector, matching *VectorM } // signatureFunc returns a function that calculates the signature for a metric -// based on the provided labels. If ignoring, then the given labels are ignored instead. -func signatureFunc(ignoring bool, labels ...model.LabelName) func(m metric.Metric) uint64 { - if len(labels) == 0 || ignoring { +// ignoring the provided labels. If on, then the given labels are only used instead. +func signatureFunc(on bool, labels ...model.LabelName) func(m metric.Metric) uint64 { + if !on { return func(m metric.Metric) uint64 { tmp := m.Metric.Clone() for _, l := range labels { @@ -905,10 +905,7 @@ func resultMetric(lhs, rhs metric.Metric, op itemType, matching *VectorMatching) if shouldDropMetricName(op) { lhs.Del(model.MetricNameLabel) } - if len(matching.MatchingLabels)+len(matching.Include) == 0 { - return lhs - } - if matching.Ignoring { + if !matching.On { if matching.Card == CardOneToOne { for _, l := range matching.MatchingLabels { lhs.Del(l) diff --git a/promql/parse.go b/promql/parse.go index e2e03b18f..da9c383b3 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -463,8 +463,8 @@ func (p *parser) expr() Expr { // Parse ON/IGNORING clause. if p.peek().typ == itemOn || p.peek().typ == itemIgnoring { - if p.peek().typ == itemIgnoring { - vecMatching.Ignoring = true + if p.peek().typ == itemOn { + vecMatching.On = true } p.next() vecMatching.MatchingLabels = p.labels() @@ -485,7 +485,7 @@ func (p *parser) expr() Expr { for _, ln := range vecMatching.MatchingLabels { for _, ln2 := range vecMatching.Include { - if ln == ln2 && !vecMatching.Ignoring { + if ln == ln2 && vecMatching.On { p.errorf("label %q must not occur in ON and GROUP clause at once", ln) } } @@ -671,14 +671,16 @@ func (p *parser) labels() model.LabelNames { p.expect(itemLeftParen, ctx) labels := model.LabelNames{} - for { - id := p.expect(itemIdentifier, ctx) - labels = append(labels, model.LabelName(id.val)) + if p.peek().typ != itemRightParen { + for { + id := p.expect(itemIdentifier, ctx) + labels = append(labels, model.LabelName(id.val)) - if p.peek().typ != itemComma { - break + if p.peek().typ != itemComma { + break + } + p.next() } - p.next() } p.expect(itemRightParen, ctx) diff --git a/promql/parse_test.go b/promql/parse_test.go index 4b9791fe2..5e65bfd98 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -471,12 +471,14 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardOneToMany, MatchingLabels: model.LabelNames{"baz", "buz"}, + On: true, Include: model.LabelNames{"test"}, }, }, VectorMatching: &VectorMatching{ Card: CardOneToOne, MatchingLabels: model.LabelNames{"foo"}, + On: true, }, }, }, { @@ -498,6 +500,7 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardOneToOne, MatchingLabels: model.LabelNames{"test", "blub"}, + On: true, }, }, }, { @@ -519,6 +522,7 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardManyToOne, MatchingLabels: model.LabelNames{"test", "blub"}, + On: true, }, }, }, { @@ -540,6 +544,29 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardManyToMany, MatchingLabels: model.LabelNames{"test", "blub"}, + On: true, + }, + }, + }, { + input: "foo and on() bar", + expected: &BinaryExpr{ + Op: itemLAND, + LHS: &VectorSelector{ + Name: "foo", + LabelMatchers: metric.LabelMatchers{ + {Type: metric.Equal, Name: model.MetricNameLabel, Value: "foo"}, + }, + }, + RHS: &VectorSelector{ + Name: "bar", + LabelMatchers: metric.LabelMatchers{ + {Type: metric.Equal, Name: model.MetricNameLabel, Value: "bar"}, + }, + }, + VectorMatching: &VectorMatching{ + Card: CardManyToMany, + MatchingLabels: model.LabelNames{}, + On: true, }, }, }, { @@ -561,7 +588,27 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardManyToMany, MatchingLabels: model.LabelNames{"test", "blub"}, - Ignoring: true, + }, + }, + }, { + input: "foo and ignoring() bar", + expected: &BinaryExpr{ + Op: itemLAND, + LHS: &VectorSelector{ + Name: "foo", + LabelMatchers: metric.LabelMatchers{ + {Type: metric.Equal, Name: model.MetricNameLabel, Value: "foo"}, + }, + }, + RHS: &VectorSelector{ + Name: "bar", + LabelMatchers: metric.LabelMatchers{ + {Type: metric.Equal, Name: model.MetricNameLabel, Value: "bar"}, + }, + }, + VectorMatching: &VectorMatching{ + Card: CardManyToMany, + MatchingLabels: model.LabelNames{}, }, }, }, { @@ -583,6 +630,7 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardManyToMany, MatchingLabels: model.LabelNames{"bar"}, + On: true, }, }, }, { @@ -604,6 +652,7 @@ var testExpr = []struct { VectorMatching: &VectorMatching{ Card: CardManyToOne, MatchingLabels: model.LabelNames{"test", "blub"}, + On: true, Include: model.LabelNames{"bar"}, }, }, @@ -627,7 +676,6 @@ var testExpr = []struct { Card: CardManyToOne, MatchingLabels: model.LabelNames{"test", "blub"}, Include: model.LabelNames{"blub"}, - Ignoring: true, }, }, }, { @@ -650,7 +698,6 @@ var testExpr = []struct { Card: CardManyToOne, MatchingLabels: model.LabelNames{"test", "blub"}, Include: model.LabelNames{"bar"}, - Ignoring: true, }, }, }, { @@ -673,6 +720,7 @@ var testExpr = []struct { Card: CardOneToMany, MatchingLabels: model.LabelNames{"test", "blub"}, Include: model.LabelNames{"bar", "foo"}, + On: true, }, }, }, { @@ -695,7 +743,6 @@ var testExpr = []struct { Card: CardOneToMany, MatchingLabels: model.LabelNames{"test", "blub"}, Include: model.LabelNames{"bar", "foo"}, - Ignoring: true, }, }, }, { @@ -1142,6 +1189,18 @@ var testExpr = []struct { }, Grouping: model.LabelNames{"foo"}, }, + }, { + input: "sum by ()(some_metric)", + expected: &AggregateExpr{ + Op: itemSum, + Expr: &VectorSelector{ + Name: "some_metric", + LabelMatchers: metric.LabelMatchers{ + {Type: metric.Equal, Name: model.MetricNameLabel, Value: "some_metric"}, + }, + }, + Grouping: model.LabelNames{}, + }, }, { input: `sum some_metric by (test)`, fail: true, @@ -1150,10 +1209,6 @@ var testExpr = []struct { input: `sum (some_metric) by test`, fail: true, errMsg: "unexpected identifier \"test\" in grouping opts, expected \"(\"", - }, { - input: `sum (some_metric) by ()`, - fail: true, - errMsg: "unexpected \")\" in grouping opts, expected identifier", }, { input: `sum (some_metric) by test`, fail: true, diff --git a/promql/printer.go b/promql/printer.go index 5d395bf6b..8d93af29e 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -160,10 +160,10 @@ func (node *BinaryExpr) String() string { matching := "" vm := node.VectorMatching if vm != nil && len(vm.MatchingLabels) > 0 { - if vm.Ignoring { - matching = fmt.Sprintf(" IGNORING(%s)", vm.MatchingLabels) - } else { + if vm.On { matching = fmt.Sprintf(" ON(%s)", vm.MatchingLabels) + } else { + matching = fmt.Sprintf(" IGNORING(%s)", vm.MatchingLabels) } if vm.Card == CardManyToOne || vm.Card == CardOneToMany { matching += " GROUP_" diff --git a/promql/testdata/aggregators.test b/promql/testdata/aggregators.test index de4dd1e83..78bf5192d 100644 --- a/promql/testdata/aggregators.test +++ b/promql/testdata/aggregators.test @@ -37,6 +37,19 @@ eval instant at 50m sum without (instance) (http_requests{job="api-server"}) {group="canary",job="api-server"} 700 {group="production",job="api-server"} 300 +# Empty by. +eval instant at 50m sum by () (http_requests{job="api-server"}) + {} 1000 + +# No by/without. +eval instant at 50m sum (http_requests{job="api-server"}) + {} 1000 + +# Empty without. +eval instant at 50m sum without () (http_requests{job="api-server",group="production"}) + {group="production",job="api-server",instance="0"} 100 + {group="production",job="api-server",instance="1"} 200 + # Without with mismatched and missing labels. Do not do this. eval instant at 50m sum without (instance) (http_requests{job="api-server"} or foo) {group="canary",job="api-server"} 700 diff --git a/promql/testdata/operators.test b/promql/testdata/operators.test index 8169ff6f4..2075e77d7 100644 --- a/promql/testdata/operators.test +++ b/promql/testdata/operators.test @@ -329,3 +329,21 @@ eval instant at 5m node_cpu > on(job, instance) group_left(target) (threshold or node_cpu{instance="abc",job="node",mode="user",target="a@b.com"} 1 node_cpu{instance="def",job="node",mode="idle"} 8 node_cpu{instance="def",job="node",mode="user"} 2 + +clear + +load 5m + random{foo="bar"} 2 + metricA{baz="meh"} 3 + metricB{baz="meh"} 4 + +# On with no labels, for metrics with no common labels. +eval instant at 5m random + on() metricA + {} 5 + +# Ignoring with no labels is the same as no ignoring. +eval instant at 5m metricA + ignoring() metricB + {baz="meh"} 7 + +eval instant at 5m metricA + metricB + {baz="meh"} 7