From 5e2d1c1464752ac662164bdc5f191493292f0dae Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Fri, 12 Jun 2015 14:21:01 +0200 Subject: [PATCH] Deprecate `keeping_extra`, rename it to `keep_common`. `keep_common` is more in line with the function name `drop_common_labels()` terminology-wise, and also more in line with `group_left`/`group_right` (no `...ing` verb suffix). We could also go the full way and call it `keep_common_labels`. That would have the benefit of being even more consistent with the function `drop_common_labels()` and would be more explanatory, but it also seems quite long. --- promql/lex.go | 5 +++-- promql/lex_test.go | 5 ++++- promql/parse.go | 8 ++++---- promql/parse_test.go | 14 +++++++------- promql/printer.go | 2 +- promql/printer_test.go | 2 +- promql/testdata/legacy.test | 12 ++++++------ 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/promql/lex.go b/promql/lex.go index 801c793c8..646116cac 100644 --- a/promql/lex.go +++ b/promql/lex.go @@ -145,7 +145,7 @@ const ( itemWith itemSummary itemDescription - itemKeepingExtra + itemKeepCommon itemOffset itemBy itemOn @@ -177,7 +177,8 @@ var key = map[string]itemType{ "description": itemDescription, "offset": itemOffset, "by": itemBy, - "keeping_extra": itemKeepingExtra, + "keeping_extra": itemKeepCommon, + "keep_common": itemKeepCommon, "on": itemOn, "group_left": itemGroupLeft, "group_right": itemGroupRight, diff --git a/promql/lex_test.go b/promql/lex_test.go index 7ef474033..da8b93a47 100644 --- a/promql/lex_test.go +++ b/promql/lex_test.go @@ -222,7 +222,10 @@ var tests = []struct { expected: []item{{itemAlert, 0, "alert"}}, }, { input: "keeping_extra", - expected: []item{{itemKeepingExtra, 0, "keeping_extra"}}, + expected: []item{{itemKeepCommon, 0, "keeping_extra"}}, + }, { + 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 883aeab20..afd38904d 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -651,8 +651,8 @@ func (p *parser) labels() clientmodel.LabelNames { // aggrExpr parses an aggregation expression. // -// () [by ] [keeping_extra] -// [by ] [keeping_extra] () +// () [by ] [keep_common] +// [by ] [keep_common] () // func (p *parser) aggrExpr() *AggregateExpr { const ctx = "aggregation" @@ -671,7 +671,7 @@ func (p *parser) aggrExpr() *AggregateExpr { grouping = p.labels() modifiersFirst = true } - if p.peek().typ == itemKeepingExtra { + if p.peek().typ == itemKeepCommon { p.next() keepExtra = true modifiersFirst = true @@ -689,7 +689,7 @@ func (p *parser) aggrExpr() *AggregateExpr { p.next() grouping = p.labels() } - if p.peek().typ == itemKeepingExtra { + if p.peek().typ == itemKeepCommon { p.next() keepExtra = true } diff --git a/promql/parse_test.go b/promql/parse_test.go index c50b8413c..1e8bb7159 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -716,7 +716,7 @@ var testExpr = []struct { Grouping: clientmodel.LabelNames{"foo"}, }, }, { - input: "sum by (foo) keeping_extra (some_metric)", + input: "sum by (foo) keep_common (some_metric)", expected: &AggregateExpr{ Op: itemSum, KeepExtraLabels: true, @@ -729,7 +729,7 @@ var testExpr = []struct { Grouping: clientmodel.LabelNames{"foo"}, }, }, { - input: "sum (some_metric) by (foo,bar) keeping_extra", + input: "sum (some_metric) by (foo,bar) keep_common", expected: &AggregateExpr{ Op: itemSum, KeepExtraLabels: true, @@ -754,7 +754,7 @@ var testExpr = []struct { Grouping: clientmodel.LabelNames{"foo"}, }, }, { - input: "COUNT by (foo) keeping_extra (some_metric)", + input: "COUNT by (foo) keep_common (some_metric)", expected: &AggregateExpr{ Op: itemCount, Expr: &VectorSelector{ @@ -767,7 +767,7 @@ var testExpr = []struct { KeepExtraLabels: true, }, }, { - input: "MIN (some_metric) by (foo) keeping_extra", + input: "MIN (some_metric) by (foo) keep_common", expected: &AggregateExpr{ Op: itemMin, Expr: &VectorSelector{ @@ -839,13 +839,13 @@ var testExpr = []struct { fail: true, errMsg: "no valid expression found", }, { - input: "MIN keeping_extra (some_metric) by (foo)", + input: "MIN keep_common (some_metric) by (foo)", fail: true, errMsg: "could not parse remaining input \"by (foo)\"...", }, { - input: "MIN by(test) (some_metric) keeping_extra", + input: "MIN by(test) (some_metric) keep_common", fail: true, - errMsg: "could not parse remaining input \"keeping_extra\"...", + errMsg: "could not parse remaining input \"keep_common\"...", }, // Test function calls. { diff --git a/promql/printer.go b/promql/printer.go index 1543cfe59..5bdf42bba 100644 --- a/promql/printer.go +++ b/promql/printer.go @@ -179,7 +179,7 @@ func (node *AggregateExpr) String() string { if len(node.Grouping) > 0 { format := "%s BY (%s)" if node.KeepExtraLabels { - format += " KEEPING_EXTRA" + format += " KEEP_COMMON" } return fmt.Sprintf(format, aggrString, node.Grouping) } diff --git a/promql/printer_test.go b/promql/printer_test.go index 87c4330ab..9a86c96db 100644 --- a/promql/printer_test.go +++ b/promql/printer_test.go @@ -28,7 +28,7 @@ func TestExprString(t *testing.T) { in: `sum(task:errors:rate10s{job="s"}) BY (code)`, }, { - in: `sum(task:errors:rate10s{job="s"}) BY (code) KEEPING_EXTRA`, + in: `sum(task:errors:rate10s{job="s"}) BY (code) KEEP_COMMON`, }, } diff --git a/promql/testdata/legacy.test b/promql/testdata/legacy.test index 62b141565..63e077b66 100644 --- a/promql/testdata/legacy.test +++ b/promql/testdata/legacy.test @@ -37,7 +37,7 @@ 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) KEEPING_EXTRA +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 @@ -411,15 +411,15 @@ eval instant at 50m sum by (group) (http_requests{job="api-server"}) {group="canary"} 700 {group="production"} 300 -# Test alternative "by"-clause order with "keeping_extra". -eval instant at 50m sum by (group) keeping_extra (http_requests{job="api-server"}) +# 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 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) keeping_extra (http_requests{job="api-server"})) by (job) +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 http_requests{group="canary"} and http_requests{instance="0"} @@ -657,10 +657,10 @@ load 1h testmetric{testlabel="1"} 1 1 testmetric{testlabel="2"} 2 _ -eval instant at 0h sum(testmetric) keeping_extra +eval instant at 0h sum(testmetric) keep_common {} 3 -eval instant at 1h sum(testmetric) keeping_extra +eval instant at 1h sum(testmetric) keep_common {testlabel="1"} 1 clear