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
Merge sparsehistogram branch into master #63
Merged
Merged
Conversation
This file contains 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
Signed-off-by: beorn7 <beorn@grafana.com>
This follows what looks to be the winning proposal in open-telemetry/oteps#149 See more detail in upcoming commit for prometheus/client_golang. Signed-off-by: beorn7 <beorn@grafana.com>
Switch to base 2 and powers of 2 for resolution
Note that this is only an extension of the proto spec. Both generators and consumers of the protobuf still need changes to make use of these changes. Gauge histograms measure current distributions. For one, they are inspired by the GaugeHistogram type introducted by OpenMetrics, see https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#gaugehistogram They are also handled in the same way as OpenMetrics does it, by using a new MetricType enum field GAUGE_HISTOGRAM, but not changing anything else, i.e. for both regular and gauge histograms, the same Histogram message type is used. The other reason why we need gauge histograms comes from PromQL: If you `rate` a histogram (which is possible with the new sparse histograms as 1st class data type), the result is a gauge histogram. A rate'd histogram can be created by a recording rule and then stored in the TSDB. From there, it can be exposed by federation, so we need to be able to represent it in the exposition format. Float histograms are histograms where all counts (count of observations, counts in each bucket, zero bucket count) are floating point numbers rather than integer numbers. They are rarely needed for direct instrumentation. Use cases are weighted histograms or timing histograms, see kubernetes/kubernetes#109277 for a real-world example. However, float histograms happen all the time as results of PromQL expressions. Following the same line of argument as above, those float histograms can end up in the TSDB via recording rules, which means they can be exposed via federation. Note that float histograms are implicitly supported by the original Prometheus text format, as this format simply uses floating point numbers for all sample values. OpenMetrics has avoided this ambiguity and has specified integers for bucket counts and the count of observations in a histogram, which means it needs to be extended to support float histograms, similar to how this commit extends the original Prometheus protobuf format. Signed-off-by: beorn7 <beorn@grafana.com>
Signed-off-by: beorn7 <beorn@grafana.com>
Add float histograms and gauge histograms to proto spec
This is slightly more efficient on the wire, and it matches the way the corresponding Go types are structured (`histogram.Histogram` and `histogram.FloatHistogram`). This follow the change for the remote-write protobuf in prometheus/prometheus#11011 . This commit also moves away from the "sparse histogram" naming in lieu of the preferred name "native histogram". Note that this is deliberately an incompatible change of the proto spec. Keeping compatibility would cause some hassle but not much gain because we haven't published the proto spec in any release yet and always marked it as experimental. Compatibility to the released proto spec (without native histograms) is kept, of course. Signed-off-by: beorn7 <beorn@grafana.com>
Flatten the buckets of native histograms
Signed-off-by: beorn7 <beorn@grafana.com>
I think I should also update the README.md to let people know about the use case for native histograms. |
Signed-off-by: beorn7 <beorn@grafana.com>
Done. |
codesome
approved these changes
Oct 18, 2022
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.
While Native Histograms are still an experimental feature, I believe we can merge their proto spec into the master branch at this point here in this repository (which doesn't have a notion of feature flags or similar).
Note that this also adds support for gauge histograms and float histograms to conventional histograms.