Add without aggregator modifier.

This has the advantage that the user doesn't need
to list all labels they want to keep (as with "by")
but without having to worry about inconsistent labels
as when there's only one time series (as with "keeping_common").

Almost all aggregation should use this rather than the existing
two options as it's much less error prone and easier to maintain
due to not having to always add in "job" plus whatever other common
job-level labels you have like "region".
This commit is contained in:
Brian Brazil 2016-02-07 18:03:16 +00:00
parent b7ef0b45e8
commit 9d0112d7cf
9 changed files with 104 additions and 7 deletions

View File

@ -104,6 +104,7 @@ type AggregateExpr struct {
Op itemType // The used aggregation operation.
Expr Expr // The vector expression over which is aggregated.
Grouping model.LabelNames // The labels by which to group the vector.
Without bool // Whether to drop the given labels rather than keep them.
KeepExtraLabels bool // Whether to keep extra labels common among result elements.
}

View File

@ -619,7 +619,7 @@ func (ev *evaluator) eval(expr Expr) model.Value {
switch e := expr.(type) {
case *AggregateExpr:
vector := ev.evalVector(e.Expr)
return ev.aggregation(e.Op, e.Grouping, e.KeepExtraLabels, vector)
return ev.aggregation(e.Op, e.Grouping, e.Without, e.KeepExtraLabels, vector)
case *BinaryExpr:
lhs := ev.evalOneOf(e.LHS, model.ValScalar, model.ValVector)
@ -1039,12 +1039,25 @@ type groupedAggregation struct {
}
// aggregation evaluates an aggregation operation on a vector.
func (ev *evaluator) aggregation(op itemType, grouping model.LabelNames, keepExtra bool, vec vector) vector {
func (ev *evaluator) aggregation(op itemType, grouping model.LabelNames, without bool, keepExtra bool, vec vector) vector {
result := map[uint64]*groupedAggregation{}
for _, sample := range vec {
groupingKey := model.SignatureForLabels(sample.Metric.Metric, grouping...)
withoutMetric := sample.Metric
if without {
for _, l := range grouping {
withoutMetric.Del(l)
}
withoutMetric.Del(model.MetricNameLabel)
}
var groupingKey uint64
if without {
groupingKey = uint64(withoutMetric.Metric.Fingerprint())
} else {
groupingKey = model.SignatureForLabels(sample.Metric.Metric, grouping...)
}
groupedResult, ok := result[groupingKey]
// Add a new group if it doesn't exist.
@ -1053,6 +1066,8 @@ func (ev *evaluator) aggregation(op itemType, grouping model.LabelNames, keepExt
if keepExtra {
m = sample.Metric
m.Del(model.MetricNameLabel)
} else if without {
m = withoutMetric
} else {
m = metric.Metric{
Metric: model.Metric{},

View File

@ -158,6 +158,7 @@ const (
itemKeepCommon
itemOffset
itemBy
itemWithout
itemOn
itemGroupLeft
itemGroupRight
@ -192,6 +193,7 @@ var key = map[string]itemType{
"annotations": itemAnnotations,
"offset": itemOffset,
"by": itemBy,
"without": itemWithout,
"keeping_extra": itemKeepCommon,
"keep_common": itemKeepCommon,
"on": itemOn,

View File

@ -261,6 +261,9 @@ var tests = []struct {
}, {
input: "by",
expected: []item{{itemBy, 0, "by"}},
}, {
input: "without",
expected: []item{{itemWithout, 0, "without"}},
}, {
input: "on",
expected: []item{{itemOn, 0, "on"}},

View File

@ -732,11 +732,14 @@ func (p *parser) aggrExpr() *AggregateExpr {
p.errorf("expected aggregation operator but got %s", agop)
}
var grouping model.LabelNames
var keepExtra bool
var keepExtra, without bool
modifiersFirst := false
if p.peek().typ == itemBy {
if t := p.peek().typ; t == itemBy || t == itemWithout {
if t == itemWithout {
without = true
}
p.next()
grouping = p.labels()
modifiersFirst = true
@ -752,10 +755,13 @@ func (p *parser) aggrExpr() *AggregateExpr {
p.expect(itemRightParen, ctx)
if !modifiersFirst {
if p.peek().typ == itemBy {
if t := p.peek().typ; t == itemBy || t == itemWithout {
if len(grouping) > 0 {
p.errorf("aggregation must only contain one grouping clause")
}
if t == itemWithout {
without = true
}
p.next()
grouping = p.labels()
}
@ -765,10 +771,15 @@ func (p *parser) aggrExpr() *AggregateExpr {
}
}
if keepExtra && without {
p.errorf("cannot use 'keep_common' with 'without'")
}
return &AggregateExpr{
Op: agop.typ,
Expr: e,
Grouping: grouping,
Without: without,
KeepExtraLabels: keepExtra,
}
}

View File

@ -869,6 +869,32 @@ var testExpr = []struct {
},
Grouping: model.LabelNames{"foo"},
},
}, {
input: "sum without (foo) (some_metric)",
expected: &AggregateExpr{
Op: itemSum,
Without: true,
Expr: &VectorSelector{
Name: "some_metric",
LabelMatchers: metric.LabelMatchers{
{Type: metric.Equal, Name: model.MetricNameLabel, Value: "some_metric"},
},
},
Grouping: model.LabelNames{"foo"},
},
}, {
input: "sum (some_metric) without (foo)",
expected: &AggregateExpr{
Op: itemSum,
Without: true,
Expr: &VectorSelector{
Name: "some_metric",
LabelMatchers: metric.LabelMatchers{
{Type: metric.Equal, Name: model.MetricNameLabel, Value: "some_metric"},
},
},
Grouping: model.LabelNames{"foo"},
},
}, {
input: "stddev(some_metric)",
expected: &AggregateExpr{
@ -920,6 +946,18 @@ var testExpr = []struct {
input: "MIN by(test) (some_metric) keep_common",
fail: true,
errMsg: "could not parse remaining input \"keep_common\"...",
}, {
input: `sum (some_metric) without (test) keep_common`,
fail: true,
errMsg: "cannot use 'keep_common' with 'without'",
}, {
input: `sum (some_metric) without (test) by (test)`,
fail: true,
errMsg: "could not parse remaining input \"by (test)\"...",
}, {
input: `sum without (test) (some_metric) by (test)`,
fail: true,
errMsg: "could not parse remaining input \"by (test)\"...",
},
// Test function calls.
{

View File

@ -137,7 +137,12 @@ func (es Expressions) String() (s string) {
func (node *AggregateExpr) String() string {
aggrString := fmt.Sprintf("%s(%s)", node.Op, node.Expr)
if len(node.Grouping) > 0 {
format := "%s BY (%s)"
var format string
if node.Without {
format = "%s WITHOUT (%s)"
} else {
format = "%s BY (%s)"
}
if node.KeepExtraLabels {
format += " KEEP_COMMON"
}

View File

@ -30,6 +30,9 @@ func TestExprString(t *testing.T) {
{
in: `sum(task:errors:rate10s{job="s"}) BY (code) KEEP_COMMON`,
},
{
in: `sum(task:errors:rate10s{job="s"}) WITHOUT (instance)`,
},
{
in: `up > BOOL 0`,
},

View File

@ -8,6 +8,10 @@ load 5m
http_requests{job="app-server", instance="0", group="canary"} 0+70x10
http_requests{job="app-server", instance="1", group="canary"} 0+80x10
load 5m
foo{job="api-server", instance="0", region="europe"} 0+90x10
foo{job="api-server"} 0+100x10
# Simple sum.
eval instant at 50m SUM BY (group) (http_requests{job="api-server"})
{group="canary"} 700
@ -28,6 +32,18 @@ eval instant at 50m count by (group) (http_requests{job="api-server"})
{group="canary"} 2
{group="production"} 2
# Simple without.
eval instant at 50m sum without (instance) (http_requests{job="api-server"})
{group="canary",job="api-server"} 700
{group="production",job="api-server"} 300
# 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
{group="production",job="api-server"} 300
{region="europe",job="api-server"} 900
{job="api-server"} 1000
# Lower-cased aggregation operators should work too.
eval instant at 50m sum(http_requests) by (job) + min(http_requests) by (job) + max(http_requests) by (job) + avg(http_requests) by (job)
{job="app-server"} 4550
@ -45,6 +61,7 @@ eval instant at 50m sum(sum by (group) keep_common (http_requests{job="api-serve
{job="api-server"} 1000
# Standard deviation and variance.
eval instant at 50m stddev(http_requests)
{} 229.12878474779
@ -61,6 +78,7 @@ eval instant at 50m stdvar by (instance)(http_requests)
{instance="1"} 50000
# Regression test for missing separator byte in labelsToGroupingKey.
clear
load 5m
@ -72,6 +90,7 @@ eval instant at 50m sum(label_grouping_test) by (a, b)
{a="aa", b="bb"} 100
# Tests for min/max.
clear
load 5m