Skip to content

Commit

Permalink
Optimizes SampleExpr to remove unnessary line_format.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
cyriltovena committed Dec 10, 2020
1 parent 3f99a07 commit 5a874a4
Show file tree
Hide file tree
Showing 6 changed files with 447 additions and 287 deletions.
1 change: 1 addition & 0 deletions pkg/logql/ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]))`,
Expand Down
5 changes: 5 additions & 0 deletions pkg/logql/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/logql/expr.y
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 5a874a4

Please sign in to comment.