Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogQL: Vector and Range Vector Aggregation. #654

Merged
merged 9 commits into from
Sep 4, 2019

Conversation

cyriltovena
Copy link
Contributor

This adds vector and range vector aggregation. I've exposed two new endpoint /api/v1/query for instant query and /api/v1/query_range for query over a range of time.

I've kept the old endpoint for range logs at /api/prom/query because new endpoints have different response structure and this would break Grafana integration. /cc @davkal

Examples of new query possible:

  • count_over_time({job="mysql"}[5m]) total count of log line recorded over the last 5m
  • rate(({job="mysql"} |= "error")[1m]) rate of log line containing errors over the last minute.
  • avg(rate(({app="nginx"} |= "GET")[1m])) by (cluster) average rate of http GET requests by cluster.

For now the step evaluation is dictated by the range vector iterator as this is the only one we required and the root AST node is responsible for giving back the result. This might need to change for extract or unary operator.

Performance wise, the biggest bottleneck is retrieving all sample and deduping them. I've also notice a lot of GC while doing large queries but this is due to the fact that we're throwing away all log line very quickly.

Documentation is up to date, next steps will be to work on the extract function allowing to extract sample from log line.

pkg/logql/expr.y Outdated Show resolved Hide resolved
@cyriltovena
Copy link
Contributor Author

@tomwilkie This is ready to review a final time. I've added a bunch of tests around LogQL evaluation and everything looks good.

There are scalar and more complex arithmetic operations still not done, or even the extract function but I'd like to start with this first. Overall I think the current structure will support those two easily.

🙏

Examples:

```bash
$ curl -G -s "http://localhost:3100/api/v1/query_range" --data-urlencode 'query=sum(rate({job="varlogs"}[10m])) by (level)' --data-urlencode 'step=300' | jq
Copy link
Contributor Author

@cyriltovena cyriltovena Aug 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use /api/loki/api/v1/query_range ? @tomwilkie @slim-bean

Requirement :

  • we need a unique prefix to the path
  • we can point golang client at this endpoint.
  • should be similar to cortex api

@CLAassistant
Copy link

CLAassistant commented Aug 30, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

docs/api.md Outdated Show resolved Hide resolved
pkg/logproto/logproto.proto Show resolved Hide resolved
pkg/logproto/logproto.proto Show resolved Hide resolved
@tomwilkie tomwilkie removed their request for review September 3, 2019 14:48
cyriltovena and others added 8 commits September 3, 2019 14:18
- adds avg,min,max,top,bottomk,stddev,stdvar,count
- updates api documentation
- adds tests

Improve yacc & go lexer to understand duration
Remove support for regexp in all queries
Clean up querier and logselector
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
@cyriltovena
Copy link
Contributor Author

I'm trying to retrofit regexp query string.

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

- keep supporting regex for old endpoint.
- use reserved keyword for removed property in grpc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants