From 768d09fd2af923c1b3f0d63ffa5109a32fc2fdfe Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Thu, 21 Apr 2016 18:41:27 +0100 Subject: [PATCH] Change on+group_* to take copy from the one side. If the label doesn't exist on the one side, it's not copied. All labels on the many inside are included, this is a breaking change but likely low impact. --- promql/engine.go | 27 +++++++++++++++------------ promql/testdata/operators.test | 10 +++++----- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index d5b4cf801..141aa630c 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -902,23 +902,20 @@ 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(lhs, rhs metric.Metric, op itemType, matching *VectorMatching) metric.Metric { + if shouldDropMetricName(op) { + lhs.Del(model.MetricNameLabel) + } if len(matching.On)+len(matching.Include) == 0 { - if shouldDropMetricName(op) { - lhs.Del(model.MetricNameLabel) - } return lhs } if matching.Ignoring { - if shouldDropMetricName(op) { - lhs.Del(model.MetricNameLabel) - } 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. + // Included labels from the `group_x` modifier are taken from the "one"-side. value := rhs.Metric[ln] if value != "" { lhs.Set(ln, rhs.Metric[ln]) @@ -930,14 +927,20 @@ func resultMetric(lhs, rhs metric.Metric, op itemType, matching *VectorMatching) } // As we definitely write, creating a new metric is the easiest solution. m := model.Metric{} - for _, ln := range matching.On { - if v, ok := lhs.Metric[ln]; ok { - m[ln] = v + if matching.Card == CardOneToOne { + for _, ln := range matching.On { + if v, ok := lhs.Metric[ln]; ok { + m[ln] = v + } + } + } else { + for k, v := range lhs.Metric { + m[k] = 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 { + // Included labels from the `group_x` modifier are taken from the "one"-side . + if v, ok := rhs.Metric[ln]; ok { m[ln] = v } } diff --git a/promql/testdata/operators.test b/promql/testdata/operators.test index 4b505374c..83ec8879f 100644 --- a/promql/testdata/operators.test +++ b/promql/testdata/operators.test @@ -281,8 +281,8 @@ eval instant at 5m sum(sum without (instance)(node_cpu) / ignoring (mode) group_ # 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 +eval instant at 5m node_cpu + on(dummy) group_left(foo) random*0 + {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