otelcol.exporter.prometheus: Make histogram counts cumulative for Prometheus #4196
Conversation
|
|
So to better understand, this is for cumulative temporality histograms (we're dropping delta temporality histograms for now as we haven't implemented the delta-to-cumulative conversion), but you mean we're fixing the cumulative distribution inside of the buckets, right? |
There was a problem hiding this comment.
Surprising from the OTel side, but it is what it is; LGTM at a first glance.
Let's fix the test and I'll leave the final ✅ to @rfratto who wrote the converter
|
|
||
| sort.Float64s(bounds) | ||
|
|
||
| // Calculate cumulative count value as Prometheus wants it |
There was a problem hiding this comment.
Do you think there's a way to reword here so that it's clearer we don't refer to cumulative temporality but so that we make it clear it's about the way that Prometheus uses cumulative buckets for histograms?
|
Thanks @tpaschalis and @rfratto! I've updated the changelog and resolved conflicts with main. |
* otelcol.exporter.prometheus: Make histogram counts cumulative for Prometheus (#4196) * Use sha256 for rpm signing to allow installation on fips-enabed systems (#4268) * Use sha256 for rpm digest signing to allow installation on fips-enabled systems * add changelog * fix startup order for prometheus.operator.servicemonitors. Fixes #4142 (#4146) * update versions for 0.34.3 * build: update to go 1.20.5 --------- Co-authored-by: Nikola Grcevski <[email protected]> Co-authored-by: kfriedrich123 <[email protected]>
github-actions
bot
added
the
frozen-due-to-age
PR Description
This PR attempts to fix the problem with non-cumulative histograms exported to Prometheus, from the otel metrics collector. The code change follows the approach taken by the opentelemetry-prometheus-exporter as shown here:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/prometheusexporter/collector.go#LL224C76-L224C76
Given a number of histogram buckets, we first sort the buckets by the bucket bounds value, then we calculate the cumulative value for each bucket, instead of using the exact bucket count provided by OTel.
Which issue(s) this PR fixes
Fixes #4193
Notes to the Reviewer
I'd appreciate any feedback on where to add tests for this issue.
PR Checklist