Exemplars UX improvements #5158
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
74e9718 to
147cca9
Compare
mdisibio
reviewed
May 23, 2025
| @@ -228,8 +263,8 @@ func (s *queryRangeSharder) buildBackendRequests(ctx context.Context, tenantID s | |||
| queryHash := hashForQueryRangeRequest(&searchReq) | |||
| colsToJSON := api.NewDedicatedColumnsToJSON() | |||
|
|
|||
| exemplarsPerBlock := s.exemplarsPerShard(uint32(len(metas)), searchReq.Exemplars) | |||
| for _, m := range metas { | |||
| exemplarsPerBlock := s.exemplarsPerShard(metas, searchReq.Exemplars) | |||
There was a problem hiding this comment.
Should we also scale the number of exemplars for generatorRequest based on the time range? I think it sends over the whole limit, which was also a reason exemplars seemed to clump in recent data.
There was a problem hiding this comment.
Good point! With bucket fix, it still should distribute fairly, but by cutting the exemplars parameter, we can reduce the load to generators.
I pushed exemplar splitting between generator and querier in a separate commit. Could you please take a look?
|
@ruslan-mikhailov Do we need to update the docs so people understand the changes you've made in this PR? Maybe this doc? https://grafana.com/docs/tempo/latest/traceql/metrics-queries/#exemplars Also, have you seen this PR? #5129 |
3 tasks
mapno
reviewed
May 27, 2025
Determine the number of requested exemplars from a block based on its time range, not equally
UX improvement
This will reduce load on generators
f7d231c to
4b72208
Compare
|
+ rebase from latest main |
|
+ linter fixes |
833a0e8 to
6b9bafe
Compare
ruslan-mikhailov
added a commit
to ruslan-mikhailov/tempo
that referenced
this pull request
Jun 2, 2025
* TraceQL Metrics: more fair exemplars distribution Determine the number of requested exemplars from a block based on its time range, not equally * TraceQL Metrics: distribute exemplars over time UX improvement * TraceQL Metrics: Parametrise exemplars limit * TraceQL Metrics: pass exemplars limit to combiners * TraceQL Metrics: hard limit exemplars * TraceQL Metrics: restore default behaviour * TraceQL Metrics: e2e test for number of returned exemplars * TraceQL Metrics: state bug in quantile_over_time * Changelog * Stabilise flaky test * nolint for integer overflow * minor linter fixes * TraceQL Metrics: share exemplars between generator and backend This will reduce load on generators * Add comment with link to bug * minor linter fixes
3 tasks
ruslan-mikhailov
added a commit
that referenced
this pull request
Jun 2, 2025
* TraceQL Metrics: more fair exemplars distribution Determine the number of requested exemplars from a block based on its time range, not equally * TraceQL Metrics: distribute exemplars over time UX improvement * TraceQL Metrics: Parametrise exemplars limit * TraceQL Metrics: pass exemplars limit to combiners * TraceQL Metrics: hard limit exemplars * TraceQL Metrics: restore default behaviour * TraceQL Metrics: e2e test for number of returned exemplars * TraceQL Metrics: state bug in quantile_over_time * Changelog * Stabilise flaky test * nolint for integer overflow * minor linter fixes * TraceQL Metrics: share exemplars between generator and backend This will reduce load on generators * Add comment with link to bug * minor linter fixes
knylander-grafana
pushed a commit
to knylander-grafana/tempo-doc-work
that referenced
this pull request
Jun 2, 2025
* TraceQL Metrics: more fair exemplars distribution Determine the number of requested exemplars from a block based on its time range, not equally * TraceQL Metrics: distribute exemplars over time UX improvement * TraceQL Metrics: Parametrise exemplars limit * TraceQL Metrics: pass exemplars limit to combiners * TraceQL Metrics: hard limit exemplars * TraceQL Metrics: restore default behaviour * TraceQL Metrics: e2e test for number of returned exemplars * TraceQL Metrics: state bug in quantile_over_time * Changelog * Stabilise flaky test * nolint for integer overflow * minor linter fixes * TraceQL Metrics: share exemplars between generator and backend This will reduce load on generators * Add comment with link to bug * minor linter fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does:
a. Change the approach to calculating requested exemplars from frontend to querier. Instead of equal share, calculate it based on block time range. The biggest block will receive more exemplars than a small block
b. Change sampling algorithm by fixing number of sampling buckets
Before the change:
After the change:
Results:
Before:
After:
Which issue(s) this PR fixes:
Fixes #4856
Fixes #4632
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]