Skip to content

Quantile summaries#170

Merged
NelsonVides merged 4 commits intomasterfrom
quantile_summaries
Feb 26, 2025
Merged

Quantile summaries#170
NelsonVides merged 4 commits intomasterfrom
quantile_summaries

Conversation

@NelsonVides
Copy link
Copy Markdown
Member

@NelsonVides NelsonVides commented Feb 25, 2025

Fixes #159 and #146

As in the last commit:

Note that the underlying algorithm for quantiles is not mergeable,
hence, it requires a linear strategy. However, precisely because it is
not generally mergeable, and it is also expensive, Prometheus generally
recommends using histograms on the client side, and it is left to the
Prometheus server to compute, merge, and aggregate, histograms,
recreating any required quantiles.
It is left for the future to implement mergeable summaries, an idea
would be a DDSketch algorithm or any variant thereof.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/metrics/prometheus_quantile_summary.erl 92.30% <100.00%> (+0.30%) ⬆️
src/metrics/prometheus_summary.erl 96.00% <ø> (ø)
...eunit/metric/prometheus_quantile_summary_tests.erl 98.53% <100.00%> (+0.07%) ⬆️

mikpe
mikpe previously approved these changes Feb 25, 2025
Comment thread rebar.config Outdated
DenysGonchar
DenysGonchar previously approved these changes Feb 26, 2025
Note that the underlying algorithm for quantiles is not mergeable,
hence, it requires a linear strategy. However, precisely because it is
not generally mergeable, and it is also expensive, Prometheus generally
recommends using histograms on the client side, and it is left to the
Prometheus server to compute, merge, and aggregate, histograms,
recreating any required quantiles.

It is left for the future to implement mergeable summaries, an idea
would be a DDSketch algorithm or any variant thereof.
@NelsonVides NelsonVides dismissed stale reviews from DenysGonchar and mikpe via f1331e6 February 26, 2025 14:27
Copy link
Copy Markdown

@lhoguin lhoguin left a comment

Choose a reason for hiding this comment

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

This review only validates that RabbitMQ should continue to work after this change. I have not evaluated the technical merits of quantile summaries (I'm out of my league there).

@NelsonVides NelsonVides merged commit d28dee8 into master Feb 26, 2025
@NelsonVides NelsonVides deleted the quantile_summaries branch February 26, 2025 17:35
@NelsonVides NelsonVides mentioned this pull request Mar 24, 2025
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.

Crashes with quantile_summary metrics and multiple schedulers Reading quantile summary metrics fails intermittently when no labels are used

4 participants