Skip to content

Add query-frontend limit for max length of query expression #4397

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

Merged
merged 4 commits into from
Nov 28, 2024

Conversation

electron0zero
Copy link
Member

@electron0zero electron0zero commented Nov 28, 2024

What this PR does:

In order to protect tempo from massive queries, it's helpful to put a cap on the max query length.

Having a limit on the size of the query saves us from being DOSed by issuing huge queries against tempo.

Added query_frontend.max_query_expression_size_bytes config param for query frontends, defaults to 128 KiB.

this can be configured to a value lower of higher by setting it in query_frontend config section like:

query_frontend:
  max_query_expression_size_bytes: 10000

users will see it as an error like this:
image

Loki also limits the size of query expression to 128kb and Mimir has a configurable limit with same name (see grafana/mimir#4604), and both were added for same reasons.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@electron0zero electron0zero enabled auto-merge (squash) November 28, 2024 15:27
Copy link
Contributor

@mapno mapno left a comment

Choose a reason for hiding this comment

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

I prefer to have 0 disable the limit, but I'm to merge this PR 👍

@electron0zero electron0zero merged commit f2f9fc7 into grafana:main Nov 28, 2024
17 checks passed
@electron0zero electron0zero deleted the max_query_expr branch November 28, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants