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

Fixes 500 when query is outside of max_query_lookback #4902

Merged
merged 3 commits into from
Dec 9, 2021

Conversation

cyriltovena
Copy link
Contributor

We were using cortex middleware to apply this limit, but when the max_query_lookback was enforced it was generating
a reponse that is not known to Loki.

And since Loki frontend support many other type of reponses (Series,Labels,...) I had to fork the code in Loki.

I added a regression tests and also enforce the limit for labels API which was for some reason omitted.

Signed-off-by: Cyril Tovena cyril.tovena@gmail.com

Checklist

  • [] Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

We were using cortex middleware to apply this limit, but when the max_query_lookback was enforced it was generating
a reponse that is not known to Loki.

And since Loki frontend support many other type of reponses (Series,Labels,...) I had to fork the code in Loki.

I added a regression tests and also enforce the limit for labels API which was for some reason omitted.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
@cyriltovena cyriltovena requested a review from a team as a code owner December 9, 2021 11:53
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit a47a76c into grafana:main Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants