diff --git a/promql/engine.go b/promql/engine.go index 2dd010b88c..d5b4cf801c 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -795,9 +795,8 @@ func (ev *evaluator) vectorBinop(op itemType, lhs, rhs vector, matching *VectorM panic("many-to-many only allowed for set operators") } var ( - result = vector{} - sigf = signatureFunc(matching.Ignoring, matching.On...) - resultLabels = append(matching.On, matching.Include...) + result = vector{} + sigf = signatureFunc(matching.Ignoring, matching.On...) ) // The control flow below handles one-to-one or many-to-one matching. @@ -851,7 +850,7 @@ func (ev *evaluator) vectorBinop(op itemType, lhs, rhs vector, matching *VectorM } else if !keep { continue } - metric := resultMetric(ls.Metric, op, matching.Ignoring, resultLabels...) + metric := resultMetric(ls.Metric, rs.Metric, op, matching) insertedSigs, exists := matchedSigs[sig] if matching.Card == CardOneToOne { @@ -863,7 +862,7 @@ func (ev *evaluator) vectorBinop(op itemType, lhs, rhs vector, matching *VectorM // In many-to-one matching the grouping labels have to ensure a unique metric // for the result vector. Check whether those labels have already been added for // the same matching labels. - insertSig := model.SignatureForLabels(metric.Metric, matching.Include...) + insertSig := uint64(metric.Metric.Fingerprint()) if !exists { insertedSigs = map[uint64]struct{}{} matchedSigs[sig] = insertedSigs @@ -902,21 +901,43 @@ func signatureFunc(ignoring bool, labels ...model.LabelName) func(m metric.Metri // resultMetric returns the metric for the given sample(s) based on the vector // binary operation and the matching options. -func resultMetric(met metric.Metric, op itemType, ignoring bool, labels ...model.LabelName) metric.Metric { - if len(labels) == 0 || ignoring { +func resultMetric(lhs, rhs metric.Metric, op itemType, matching *VectorMatching) metric.Metric { + if len(matching.On)+len(matching.Include) == 0 { if shouldDropMetricName(op) { - met.Del(model.MetricNameLabel) + lhs.Del(model.MetricNameLabel) } - for _, l := range labels { - met.Del(l) + return lhs + } + if matching.Ignoring { + if shouldDropMetricName(op) { + lhs.Del(model.MetricNameLabel) } - return met + if matching.Card == CardOneToOne { + for _, l := range matching.On { + lhs.Del(l) + } + } + for _, ln := range matching.Include { + // Included labels from the `group_x` modifier are taken from the one side with Ignoring. + value := rhs.Metric[ln] + if value != "" { + lhs.Set(ln, rhs.Metric[ln]) + } else { + lhs.Del(ln) + } + } + return lhs } // As we definitely write, creating a new metric is the easiest solution. m := model.Metric{} - for _, ln := range labels { - // Included labels from the `group_x` modifier are taken from the "many"-side. - if v, ok := met.Metric[ln]; ok { + for _, ln := range matching.On { + if v, ok := lhs.Metric[ln]; ok { + m[ln] = v + } + } + for _, ln := range matching.Include { + // Included labels from the `group_x` modifier are taken from the "many"-side with On. + if v, ok := lhs.Metric[ln]; ok { m[ln] = v } } diff --git a/promql/parse.go b/promql/parse.go index ba246d424b..b724b56d59 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -461,7 +461,7 @@ func (p *parser) expr() Expr { returnBool = true } - // Parse ON clause. + // Parse ON/IGNORING clause. if p.peek().typ == itemOn || p.peek().typ == itemIgnoring { if p.peek().typ == itemIgnoring { vecMatching.Ignoring = true @@ -470,24 +470,24 @@ func (p *parser) expr() Expr { vecMatching.On = p.labels() // Parse grouping. - if t := p.peek().typ; t == itemGroupLeft { + if t := p.peek().typ; t == itemGroupLeft || t == itemGroupRight { p.next() - vecMatching.Card = CardManyToOne - vecMatching.Include = p.labels() - } else if t == itemGroupRight { - p.next() - vecMatching.Card = CardOneToMany - vecMatching.Include = p.labels() + if t == itemGroupLeft { + vecMatching.Card = CardManyToOne + } else { + vecMatching.Card = CardOneToMany + } + if p.peek().typ == itemLeftParen { + vecMatching.Include = p.labels() + } else if !vecMatching.Ignoring { + p.errorf("must specify labels in INCLUDE clause when using ON") + } } } - if vecMatching.Ignoring && (vecMatching.Card == CardManyToOne || vecMatching.Card == CardOneToMany) { - p.errorf("IGNORING not permitted with many to one matching") - } - for _, ln := range vecMatching.On { for _, ln2 := range vecMatching.Include { - if ln == ln2 { + if ln == ln2 && !vecMatching.Ignoring { p.errorf("label %q must not occur in ON and INCLUDE clause at once", ln) } } diff --git a/promql/parse_test.go b/promql/parse_test.go index 22a07a138d..db39ece5a1 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -250,14 +250,14 @@ var testExpr = []struct { input: "1 offset 1d", fail: true, errMsg: "offset modifier must be preceded by an instant or range selector", + }, { + input: "a - on(b) group_left d", + fail: true, + errMsg: "must specify labels in INCLUDE clause when using ON", }, { input: "a - on(b) ignoring(c) d", fail: true, errMsg: "parse error at char 11: no valid expression found", - }, { - input: "a - ignoring(b) group_left(c) d", - fail: true, - errMsg: "parse error at char 29: IGNORING not permitted with many to one matching", }, // Vector binary operations. { @@ -590,6 +590,52 @@ var testExpr = []struct { Include: model.LabelNames{"bar"}, }, }, + }, { + input: "foo / ignoring(test,blub) group_left(blub) bar", + expected: &BinaryExpr{ + Op: itemDIV, + 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: CardManyToOne, + On: model.LabelNames{"test", "blub"}, + Include: model.LabelNames{"blub"}, + Ignoring: true, + }, + }, + }, { + input: "foo / ignoring(test,blub) group_left(bar) bar", + expected: &BinaryExpr{ + Op: itemDIV, + 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: CardManyToOne, + On: model.LabelNames{"test", "blub"}, + Include: model.LabelNames{"bar"}, + Ignoring: true, + }, + }, }, { input: "foo - on(test,blub) group_right(bar,foo) bar", expected: &BinaryExpr{ @@ -612,6 +658,29 @@ var testExpr = []struct { Include: model.LabelNames{"bar", "foo"}, }, }, + }, { + input: "foo - ignoring(test,blub) group_right(bar,foo) bar", + expected: &BinaryExpr{ + Op: itemSUB, + 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: CardOneToMany, + On: model.LabelNames{"test", "blub"}, + Include: model.LabelNames{"bar", "foo"}, + Ignoring: true, + }, + }, }, { input: "foo and 1", fail: true, @@ -671,7 +740,7 @@ var testExpr = []struct { }, { input: `http_requests{group="production"} / on(instance) group_left cpu_count{type="smp"}`, fail: true, - errMsg: "unexpected identifier \"cpu_count\" in grouping opts, expected \"(\"", + errMsg: "parse error at char 61: must specify labels in INCLUDE clause when using ON", }, { input: `http_requests{group="production"} + on(instance) group_left(job,instance) cpu_count{type="smp"}`, fail: true, diff --git a/promql/testdata/operators.test b/promql/testdata/operators.test index 1c095ebaeb..4b505374ce 100644 --- a/promql/testdata/operators.test +++ b/promql/testdata/operators.test @@ -223,6 +223,9 @@ load 5m node_cpu{instance="def",job="node",mode="idle"} 8 node_cpu{instance="def",job="node",mode="user"} 2 +load 5m + random{foo="bar"} 1 + # Copy machine role to node variable. eval instant at 5m node_role * on (instance,job) group_left (role) node_var {instance="abc",job="node",role="prometheus"} 2 @@ -230,6 +233,18 @@ eval instant at 5m node_role * on (instance,job) group_left (role) node_var eval instant at 5m node_var * on (instance,job) group_right (role) node_role {instance="abc",job="node",role="prometheus"} 2 +eval instant at 5m node_var * ignoring (role) group_left (role) node_role + {instance="abc",job="node",role="prometheus"} 2 + +eval instant at 5m node_role * ignoring (role) group_right (role) node_var + {instance="abc",job="node",role="prometheus"} 2 + +# Copy machine role to node variable with instrumentation labels. +eval instant at 5m node_cpu * ignoring (role, mode) group_left (role) node_role + {instance="abc",job="node",mode="idle",role="prometheus"} 3 + {instance="abc",job="node",mode="user",role="prometheus"} 1 + + # Ratio of total. eval instant at 5m node_cpu / on (instance,job) group_left (mode) sum by (instance,job)(node_cpu) {instance="abc",job="node",mode="idle"} .75 @@ -243,3 +258,31 @@ eval instant at 5m sum by (mode, job)(node_cpu) / on (job) group_left (mode) sum eval instant at 5m sum(sum by (mode, job)(node_cpu) / on (job) group_left (mode) sum by (job)(node_cpu)) {} 1.0 + + +eval instant at 5m node_cpu / ignoring (mode) group_left sum without (mode)(node_cpu) + {instance="abc",job="node",mode="idle"} .75 + {instance="abc",job="node",mode="user"} .25 + {instance="def",job="node",mode="idle"} .80 + {instance="def",job="node",mode="user"} .20 + +eval instant at 5m node_cpu / ignoring (mode) group_left(dummy) sum without (mode)(node_cpu) + {instance="abc",job="node",mode="idle"} .75 + {instance="abc",job="node",mode="user"} .25 + {instance="def",job="node",mode="idle"} .80 + {instance="def",job="node",mode="user"} .20 + +eval instant at 5m sum without (instance)(node_cpu) / ignoring (mode) group_left sum without (instance, mode)(node_cpu) + {job="node",mode="idle"} 0.7857142857142857 + {job="node",mode="user"} 0.21428571428571427 + +eval instant at 5m sum(sum without (instance)(node_cpu) / ignoring (mode) group_left sum without (instance, mode)(node_cpu)) + {} 1.0 + + +# Copy over label from metric with no matching labels, without having to list cross-job target labels ('job' here). +#eval instant at 5m node_cpu + on(dummy) group_left(foo) random +# {instance="abc",job="node",mode="idle",foo="bar"} 3 +# {instance="abc",job="node",mode="user",foo="bar"} 1 +# {instance="def",job="node",mode="idle",foo="bar"} 8 +# {instance="def",job="node",mode="user",foo="bar"} 2