From 3b89616d82110b2c843cd87bc9a1da4fb8c0bd34 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Thu, 23 Jun 2016 17:49:22 +0100 Subject: [PATCH] Allow on, ignoring, by and without wit empty laberls. This offers new semantics in allowing on() for matching two single-element vectors with no known common labels. Previosuly this was often done using on(dummy). This also allows making it explict that you meant to do an aggregation without labels via by(). Fixes #1597. --- promql/engine.go | 7 ++-- promql/parse.go | 14 ++++---- promql/parse_test.go | 59 +++++++++++++++++++++++++++++--- promql/testdata/aggregators.test | 13 +++++++ promql/testdata/operators.test | 18 ++++++++++ 5 files changed, 96 insertions(+), 15 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 92da4a41d..3a9fc8fac 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -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. +// 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 len(labels) == 0 || !on { + if !on { return func(m metric.Metric) uint64 { tmp := m.Metric.Clone() for _, l := range labels { @@ -905,9 +905,6 @@ 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.On { if matching.Card == CardOneToOne { for _, l := range matching.MatchingLabels { diff --git a/promql/parse.go b/promql/parse.go index 057237dae..da9c383b3 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -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 16be4382c..5e65bfd98 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -547,6 +547,28 @@ var testExpr = []struct { 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, + }, + }, }, { input: "foo and ignoring(test,blub) bar", expected: &BinaryExpr{ @@ -568,6 +590,27 @@ var testExpr = []struct { MatchingLabels: model.LabelNames{"test", "blub"}, }, }, + }, { + 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{}, + }, + }, }, { input: "foo unless on(bar) baz", expected: &BinaryExpr{ @@ -1146,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, @@ -1154,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/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