Optimizes SampleExpr to remove unnecessary line_format. (#3065)

* 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 <cyril.tovena@gmail.com> * For all range vector operations. Except for bytes one which count bytes in lines. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com> * Update pkg/logql/optimize_test.go Co-authored-by: Owen Diehl <ow.diehl@gmail.com> Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
grafana · Jan 6, 2021 · c971e8e · c971e8e
1 parent bd321fb
commit c971e8e
Show file tree

Hide file tree

Showing 6 changed files with 449 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: