Optimizes SampleExpr to remove unnessary line_format.

When doing count_over_time and rate(), it doesn't make sense to have a line_format since you're counting line, you don't really care about what the line contains. At the same time this set the foundation of for further optimization, like parsing only required labels during label parsing. I've also found a bug where `topk by(foo) (1...` where not correctly parsed. Signed-off-by: Cyril Tovena <[email protected]>
grafana · Dec 10, 2020 · 5a874a4 · 5a874a4
1 parent 3f99a07
commit 5a874a4
Show file tree

Hide file tree

Showing 6 changed files with 447 additions and 287 deletions.
diff --git a/pkg/logql/ast_test.go b/pkg/logql/ast_test.go
@@ -66,6 +66,7 @@ func Test_SampleExpr_String(t *testing.T) {
 		`sum(count_over_time({job="mysql"} | logfmt [5m]))`,
 		`sum(count_over_time({job="mysql"} | regexp "(?P<foo>foo|bar)" [5m]))`,
 		`topk(10,sum(rate({region="us-east1"}[5m])) by (name))`,
+		`topk by (name)(10,sum(rate({region="us-east1"}[5m])))`,
 		`avg( rate( ( {job="nginx"} |= "GET" ) [10s] ) ) by (region)`,
 		`avg(min_over_time({job="nginx"} |= "GET" | unwrap foo[10s])) by (region)`,
 		`sum by (cluster) (count_over_time({job="mysql"}[5m]))`,

diff --git a/pkg/logql/engine.go b/pkg/logql/engine.go
@@ -207,6 +207,11 @@ func (q *query) evalSample(ctx context.Context, expr SampleExpr) (promql_parser.
 		return nil, err
 	}
 
+	expr, err = optimizeSampleExpr(expr)
+	if err != nil {
+		return nil, err
+	}
+
 	stepEvaluator, err := q.evaluator.StepEvaluator(ctx, q.evaluator, expr, q.params)
 	if err != nil {
 		return nil, err

diff --git a/pkg/logql/expr.y b/pkg/logql/expr.y
@@ -170,6 +170,7 @@ vectorAggregationExpr:
     // Aggregations with 2 arguments.
     | vectorOp OPEN_PARENTHESIS NUMBER COMMA metricExpr CLOSE_PARENTHESIS                 { $$ = mustNewVectorAggregationExpr($5, $1, nil, &$3) }
     | vectorOp OPEN_PARENTHESIS NUMBER COMMA metricExpr CLOSE_PARENTHESIS grouping        { $$ = mustNewVectorAggregationExpr($5, $1, $7, &$3) }
+    | vectorOp grouping OPEN_PARENTHESIS NUMBER COMMA metricExpr CLOSE_PARENTHESIS        { $$ = mustNewVectorAggregationExpr($6, $1, $2, &$4) }
     ;
 
 labelReplaceExpr: