enhancement: use frontend MaxExemplars config as single source of truth for exemplar limits#6515
Conversation
| // newBucketSet creates a new bucket set for the given time range | ||
| // start and end are in nanoseconds | ||
| func newBucketSet(exemplars uint32, start, end uint64) *limitedBucketSet { | ||
| if exemplars > maxExemplars || exemplars == 0 { |
There was a problem hiding this comment.
Removing this can be dangerous. This code can be reached from the queriers without passing by the clamping code in the frontend.
There was a problem hiding this comment.
Previously I assume all requests sent to queriers are from frontend. What else requests are you referring to here?
My thinking is to cap the exemplars at the earliest stage, e.g. frontend, before it being used by downstream codes.
There was a problem hiding this comment.
Discussed in person. There are queries sent to querier directly.
Updated the PR to cap the req.Exemplars at the entrance of the traceQL engine, which are CompileMetricsQueryRange and CompileMetricsQueryRangeNonRaw.
737a05b to
97011d8
Compare
pkg/traceql/util.go
Outdated
There was a problem hiding this comment.
This can lead to a division by zero panic now.
For a very small interval and a big number of buckets bucketWidth can be zero.
Later on in the bucket() method we use bucketWidth as the division denominator
Example:
interval = 1000ms
buckets = 1500
bucketWidth = 1000/1500 = 0 since we are using integers.
We need to handle that case
There was a problem hiding this comment.
Wow, thanks for catching this!
| // its not present then we look to see if the user provided `with(exemplars=false)` | ||
| exemplars := int(req.Exemplars) | ||
| if v, ok := expr.Hints.GetInt(HintExemplars, allowUnsafeQueryHints); ok { | ||
| exemplars = v |
There was a problem hiding this comment.
This is overwritting the maxExemplars cap when the hint is used
There was a problem hiding this comment.
This is a separate known issue, will fix in a following up PR if that's possible.
97011d8 to
9977c3e
Compare
…th for exemplar limits (grafana#6515)
|
The backport to To backport manually, run these commands in your terminal: git fetch
git switch --create backport-6515-to-release-v2.10 origin/release-v2.10
git cherry-pick -x 401698df93a3f6337f8aa959bf1868494aca30c9Resolve the conflicts, then add the changes and run git add . && git cherry-pick --continueIf you have the GitHub CLI installed: git push --set-upstream origin backport-6515-to-release-v2.10
PR_BODY=$(gh pr view 6515 --json body --template 'Backport 401698df93a3f6337f8aa959bf1868494aca30c9 from #6515{{ "\n\n---\n\n" }}{{ index . "body" }}')
echo "${PR_BODY}" | gh pr create --title '[release-v2.10] enhancement: use frontend MaxExemplars config as single source of truth for exemplar limits' --body-file - --label 'backport' --label '' --base release-v2.10 --webOr, if you don't have the GitHub CLI installed (we recommend you install it!): git push --set-upstream origin backport-6515-to-release-v2.10And open a pull request where the |
…th for exemplar limits (grafana#6515) (cherry picked from commit 401698d)
What this PR does:
Removed a hardcoded
maxExemplars = 100fromengine_metrics.go. Instead use the configurable limit from frontend for metrics range queries.Without the fix, the clients could never receive more than 100 exemplars regardless of server configuration.
Test:
MaxExemplars = 300in frontend configurationBefore:

After:

Which issue(s) this PR fixes:
Fixes #5166
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]