diff --git a/promql/ast.go b/promql/ast.go index 44e2fdd32..8871982ed 100644 --- a/promql/ast.go +++ b/promql/ast.go @@ -99,12 +99,11 @@ type Expressions []Expr // AggregateExpr represents an aggregation operation on a Vector. type AggregateExpr struct { - Op itemType // The used aggregation operation. - Expr Expr // The Vector expression over which is aggregated. - Param Expr // Parameter used by some aggregators. - Grouping []string // The labels by which to group the Vector. - Without bool // Whether to drop the given labels rather than keep them. - KeepCommonLabels bool // Whether to keep common labels among result elements. + Op itemType // The used aggregation operation. + Expr Expr // The Vector expression over which is aggregated. + Param Expr // Parameter used by some aggregators. + Grouping []string // The labels by which to group the Vector. + Without bool // Whether to drop the given labels rather than keep them. } // BinaryExpr represents a binary expression between two child expressions. diff --git a/promql/engine.go b/promql/engine.go index 41f9238a1..a2d6ae819 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -679,7 +679,7 @@ func (ev *evaluator) eval(expr Expr) Value { switch e := expr.(type) { case *AggregateExpr: Vector := ev.evalVector(e.Expr) - return ev.aggregation(e.Op, e.Grouping, e.Without, e.KeepCommonLabels, e.Param, Vector) + return ev.aggregation(e.Op, e.Grouping, e.Without, e.Param, Vector) case *BinaryExpr: lhs := ev.evalOneOf(e.LHS, ValueTypeScalar, ValueTypeVector) @@ -1234,7 +1234,7 @@ type groupedAggregation struct { } // aggregation evaluates an aggregation operation on a Vector. -func (ev *evaluator) aggregation(op itemType, grouping []string, without bool, keepCommon bool, param Expr, vec Vector) Vector { +func (ev *evaluator) aggregation(op itemType, grouping []string, without bool, param Expr, vec Vector) Vector { result := map[uint64]*groupedAggregation{} var k int64 @@ -1282,9 +1282,7 @@ func (ev *evaluator) aggregation(op itemType, grouping []string, without bool, k if !ok { var m labels.Labels - if keepCommon { - m = lb.Del(labels.MetricName).Labels() - } else if without { + if without { m = metric } else { m = make(labels.Labels, 0, len(grouping)) @@ -1319,10 +1317,6 @@ func (ev *evaluator) aggregation(op itemType, grouping []string, without bool, k } continue } - // Add the sample to the existing group. - if keepCommon { - group.labels = intersection(group.labels, s.Metric) - } switch op { case itemSum: diff --git a/promql/lex.go b/promql/lex.go index efc0b11e8..8a4d83b05 100644 --- a/promql/lex.go +++ b/promql/lex.go @@ -187,7 +187,6 @@ const ( itemFor itemLabels itemAnnotations - itemKeepCommon itemOffset itemBy itemWithout @@ -227,7 +226,6 @@ var key = map[string]itemType{ "offset": itemOffset, "by": itemBy, "without": itemWithout, - "keep_common": itemKeepCommon, "on": itemOn, "ignoring": itemIgnoring, "group_left": itemGroupLeft, diff --git a/promql/lex_test.go b/promql/lex_test.go index 34da242f5..1c49d19bd 100644 --- a/promql/lex_test.go +++ b/promql/lex_test.go @@ -251,9 +251,6 @@ var tests = []struct { { input: "alert", expected: []item{{itemAlert, 0, "alert"}}, - }, { - input: "keep_common", - expected: []item{{itemKeepCommon, 0, "keep_common"}}, }, { input: "if", expected: []item{{itemIf, 0, "if"}}, diff --git a/promql/parse.go b/promql/parse.go index f4672a092..7008b148a 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -703,8 +703,8 @@ func (p *parser) labels() []string { // aggrExpr parses an aggregation expression. // -// () [by ] [keep_common] -// [by ] [keep_common] () +// () [by|without ] +// [by|without ] () // func (p *parser) aggrExpr() *AggregateExpr { const ctx = "aggregation" @@ -714,7 +714,7 @@ func (p *parser) aggrExpr() *AggregateExpr { p.errorf("expected aggregation operator but got %s", agop) } var grouping []string - var keepCommon, without bool + var without bool modifiersFirst := false @@ -726,11 +726,6 @@ func (p *parser) aggrExpr() *AggregateExpr { grouping = p.labels() modifiersFirst = true } - if p.peek().typ == itemKeepCommon { - p.next() - keepCommon = true - modifiersFirst = true - } p.expect(itemLeftParen, ctx) var param Expr @@ -752,23 +747,14 @@ func (p *parser) aggrExpr() *AggregateExpr { p.next() grouping = p.labels() } - if p.peek().typ == itemKeepCommon { - p.next() - keepCommon = true - } - } - - if keepCommon && without { - p.errorf("cannot use 'keep_common' with 'without'") } return &AggregateExpr{ - Op: agop.typ, - Expr: e, - Param: param, - Grouping: grouping, - Without: without, - KeepCommonLabels: keepCommon, + Op: agop.typ, + Expr: e, + Param: param, + Grouping: grouping, + Without: without, } } diff --git a/promql/parse_test.go b/promql/parse_test.go index 659715c7a..edfb8cb1e 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -1068,32 +1068,6 @@ var testExpr = []struct { }, Grouping: []string{"foo"}, }, - }, { - input: "sum by (foo) keep_common (some_metric)", - expected: &AggregateExpr{ - Op: itemSum, - KeepCommonLabels: true, - Expr: &VectorSelector{ - Name: "some_metric", - LabelMatchers: []*labels.Matcher{ - mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), - }, - }, - Grouping: []string{"foo"}, - }, - }, { - input: "sum (some_metric) by (foo,bar) keep_common", - expected: &AggregateExpr{ - Op: itemSum, - KeepCommonLabels: true, - Expr: &VectorSelector{ - Name: "some_metric", - LabelMatchers: []*labels.Matcher{ - mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), - }, - }, - Grouping: []string{"foo", "bar"}, - }, }, { input: "avg by (foo)(some_metric)", expected: &AggregateExpr{ @@ -1106,32 +1080,6 @@ var testExpr = []struct { }, Grouping: []string{"foo"}, }, - }, { - input: "COUNT by (foo) keep_common (some_metric)", - expected: &AggregateExpr{ - Op: itemCount, - Expr: &VectorSelector{ - Name: "some_metric", - LabelMatchers: []*labels.Matcher{ - mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), - }, - }, - Grouping: []string{"foo"}, - KeepCommonLabels: true, - }, - }, { - input: "MIN (some_metric) by (foo) keep_common", - expected: &AggregateExpr{ - Op: itemMin, - Expr: &VectorSelector{ - Name: "some_metric", - LabelMatchers: []*labels.Matcher{ - mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"), - }, - }, - Grouping: []string{"foo"}, - KeepCommonLabels: true, - }, }, { input: "max by (foo)(some_metric)", expected: &AggregateExpr{ @@ -1264,17 +1212,13 @@ var testExpr = []struct { fail: true, errMsg: "no valid expression found", }, { - input: "MIN keep_common (some_metric) by (foo)", + input: "MIN keep_common (some_metric)", fail: true, - errMsg: "could not parse remaining input \"by (foo)\"...", + errMsg: "parse error at char 5: unexpected identifier \"keep_common\" in aggregation, expected \"(\"", }, { - input: "MIN by(test) (some_metric) keep_common", + input: "MIN (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, diff --git a/promql/printer.go b/promql/printer.go index 6140a2531..709d8cb98 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -148,9 +148,6 @@ func (node *AggregateExpr) String() string { } aggrString = fmt.Sprintf(format, aggrString, strings.Join(node.Grouping, ", ")) } - if node.KeepCommonLabels { - aggrString += " KEEP_COMMON" - } return aggrString } diff --git a/promql/printer_test.go b/promql/printer_test.go index 2ecb88988..9a828067c 100644 --- a/promql/printer_test.go +++ b/promql/printer_test.go @@ -63,12 +63,6 @@ func TestExprString(t *testing.T) { { in: `sum(task:errors:rate10s{job="s"}) BY (code)`, }, - { - in: `sum(task:errors:rate10s{job="s"}) BY (code) KEEP_COMMON`, - }, - { - in: `sum(task:errors:rate10s{job="s"}) KEEP_COMMON`, - }, { in: `sum(task:errors:rate10s{job="s"}) WITHOUT (instance)`, }, diff --git a/promql/testdata/aggregators.test b/promql/testdata/aggregators.test index 2f590e456..40b66d606 100644 --- a/promql/testdata/aggregators.test +++ b/promql/testdata/aggregators.test @@ -42,7 +42,7 @@ 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"}) +eval instant at 50m sum(http_requests{job="api-server"}) {} 1000 # Empty without. @@ -62,16 +62,16 @@ eval instant at 50m sum(http_requests) by (job) + min(http_requests) by (job) + {job="app-server"} 4550 {job="api-server"} 1750 -# Test alternative "by"-clause order with "keep_common". -eval instant at 50m sum by (group) keep_common (http_requests{job="api-server"}) - {group="canary", job="api-server"} 700 - {group="production", job="api-server"} 300 +# Test alternative "by"-clause order. +eval instant at 50m sum by (group) (http_requests{job="api-server"}) + {group="canary"} 700 + {group="production"} 300 # Test both alternative "by"-clause orders in one expression. # Public health warning: stick to one form within an expression (or even # in an organization), or risk serious user confusion. -eval instant at 50m sum(sum by (group) keep_common (http_requests{job="api-server"})) by (job) - {job="api-server"} 1000 +eval instant at 50m sum(sum by (group) (http_requests{job="api-server"})) by (job) + {} 1000 diff --git a/promql/testdata/legacy.test b/promql/testdata/legacy.test index 607c2819c..daa1e473e 100644 --- a/promql/testdata/legacy.test +++ b/promql/testdata/legacy.test @@ -45,10 +45,6 @@ eval instant at 50m SUM(http_requests{instance="0"}) BY(job) {job="api-server"} 400 {job="app-server"} 1200 -eval instant at 50m SUM(http_requests{instance="0"}) BY(job) KEEP_COMMON - {instance="0", job="api-server"} 400 - {instance="0", job="app-server"} 1200 - eval instant at 50m SUM(http_requests) BY (job) {job="api-server"} 1000 {job="app-server"} 2600 @@ -353,17 +349,6 @@ eval instant at 50m log10(vector_matching_a - 20) # Matrix tests. -clear -load 1h - testmetric{testlabel="1"} 1 1 - testmetric{testlabel="2"} 2 _ - -eval instant at 0h sum(testmetric) keep_common - {} 3 - -eval instant at 1h sum(testmetric) keep_common - {testlabel="1"} 1 - clear load 1h testmetric{aa="bb"} 1