PromQL: Support trailing commas in grouping opts (prometheus#6480)

* PromQL: Support trailing commas in grouping opts Fixes prometheus#6470 Signed-off-by: Tobias Guggenmos <tguggenm@redhat.com>
songjiayang · Dec 20, 2019 · 1e0cd28 · 1e0cd28
1 parent 06b4744
commit 1e0cd28
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 53 deletions.
diff --git a/docs/querying/basics.md b/docs/querying/basics.md
@@ -74,8 +74,8 @@ name:
 
     http_requests_total
 
-It is possible to filter these time series further by appending a set of labels
-to match in curly braces (`{}`).
+It is possible to filter these time series further by appending a comma separated list of label
+matchers in curly braces (`{}`).
 
 This example selects only those time series with the `http_requests_total`
 metric name that also have the `job` label set to `prometheus` and their

diff --git a/docs/querying/operators.md b/docs/querying/operators.md
@@ -205,12 +205,17 @@ or
 
     <aggr-op>([parameter,] <vector expression>) [without|by (<label list>)]
 
-`parameter` is only required for `count_values`, `quantile`, `topk` and
-`bottomk`. `without` removes the listed labels from the result vector, while
+`label list` is a list of unquoted labels that may include a trailing comma, i.e.
+both `(label1, label2)` and `(label1, label2,)` are valid syntax.
+
+`without` removes the listed labels from the result vector, while
 all other labels are preserved the output. `by` does the opposite and drops
 labels that are not listed in the `by` clause, even if their label values are
 identical between all elements of the vector.
 
+`parameter` is only required for `count_values`, `quantile`, `topk` and
+`bottomk`.
+
 `count_values` outputs one time series per unique sample value. Each series has
 an additional label. The name of that label is given by the aggregation
 parameter, and the label value is the unique sample value.  The value of each

diff --git a/promql/generated_parser.y b/promql/generated_parser.y
@@ -225,6 +225,8 @@ label_set_item  :
 grouping_labels :
                 LEFT_PAREN grouping_label_list RIGHT_PAREN
                         { $$ = $2 }
+                | LEFT_PAREN grouping_label_list COMMA RIGHT_PAREN
+                        { $$ = $2 }
                 | LEFT_PAREN RIGHT_PAREN
                         { $$ = []string{} }
                 | error

diff --git a/promql/generated_parser.y.go b/promql/generated_parser.y.go
diff --git a/promql/parse_test.go b/promql/parse_test.go
@@ -1190,6 +1190,30 @@ var testExpr = []struct {
 			},
 			Grouping: []string{},
 		},
+	}, {
+		input: "sum by (foo,bar,)(some_metric)",
+		expected: &AggregateExpr{
+			Op: SUM,
+			Expr: &VectorSelector{
+				Name: "some_metric",
+				LabelMatchers: []*labels.Matcher{
+					mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"),
+				},
+			},
+			Grouping: []string{"foo", "bar"},
+		},
+	}, {
+		input: "sum by (foo,)(some_metric)",
+		expected: &AggregateExpr{
+			Op: SUM,
+			Expr: &VectorSelector{
+				Name: "some_metric",
+				LabelMatchers: []*labels.Matcher{
+					mustLabelMatcher(labels.MatchEqual, string(model.MetricNameLabel), "some_metric"),
+				},
+			},
+			Grouping: []string{"foo"},
+		},
 	}, {
 		input: "topk(5, some_metric)",
 		expected: &AggregateExpr{
@@ -1232,6 +1256,14 @@ var testExpr = []struct {
 		input:  "sum without(==)(some_metric)",
 		fail:   true,
 		errMsg: "unexpected <op:==> in grouping opts, expected label",
+	}, {
+		input:  "sum without(,)(some_metric)",
+		fail:   true,
+		errMsg: `unexpected "," in grouping opts, expected label`,
+	}, {
+		input:  "sum without(foo,,)(some_metric)",
+		fail:   true,
+		errMsg: `unexpected "," in grouping opts, expected label`,
 	}, {
 		input:  `sum some_metric by (test)`,
 		fail:   true,