fix(spec/metrics): Histogram sum is optional - #5273
Conversation
The implemented data model has the sum as intentionally optional, see https://github.com/open-telemetry/opentelemetry-proto/blob/4411380f9b4875100f3a07f746182c41fa1a8b5a/opentelemetry/proto/metrics/v1/metrics.proto#L462 https://github.com/open-telemetry/opentelemetry-proto/blob/4411380f9b4875100f3a07f746182c41fa1a8b5a/opentelemetry/proto/metrics/v1/metrics.proto#L547 For both explicit and exponential histograms. Bring the spec in line with reality. Exporters already deal with it sometimes, it's not totally consistent. Noted in open-telemetry#5125 (comment)
33229bf to
4bb703d
Compare
Pull request dashboard statusClosed · refreshed 2026-08-24 14:39 UTC Status above doesn't look right?
|
|
@open-telemetry/specs-metrics-approvers The concern I have with this is that this would imply that SDKs need to support an optional Sum, which (from my ai-assisted audit) is only Javascript today. The other 10 metrics SDKs don't allow the sum to be unset. But I don't think you need this to unblock your other 3 PRs. The protobuf definition is generally a superset of the data model (e.g. includes metric metadata), as confusing as that can be. |
I don't want to make this change to the spec, I don't think there's enough of a use case to justify it. So if we can avoid making the change, I'm all for it. I guess we can just close this PR? Or do we want to get some consensus to this first? |
OTel Rust also fabricates a "0" sum when the sum isn't applicable. And changing that would be breaking change... |
jmacd
left a comment
There was a problem hiding this comment.
Thank you for the historical note. I was aware of the original motivation for sum to be optional in the protobuf, but not the reversal. So, I take it the recommendation is that sum should be recorded, that average value will be correct.
If there was a histogram instrument defined for strictly non-negative values, that would appear to help clarify our semantics.
👍 |
Currently it wouldn't help much with translation to Prometheus. It's unlikely we'd introduce a new metric type for this and for a Prometheus classic histogram to be able to tell if there are negative observations you'd have to define an upper bound that's the largest representable negative number. That's because bucket upper bounds are inclusive, using 0 upper bound wouldn't work - that can mean observing 0. For native histograms it's even worse, the zero bucket is a small range around 0, so observations there can be negative/zero/positive. Maybe we should have native histograms 2.0 (with new reserved schema numbers) where the zero count is counting 0s strictly, so bucket |
|
Closing due to #5273 (comment) and SDK impact concerns. |
The implemented data model has the sum as intentionally optional, see histograms, exponential histograms.
Bring the spec in line with reality.
Exporters already deal with it sometimes, it's not totally consistent.
The Prometheus remote write exporter omits the
..._sumseries whensumis not present in the OTel histograms.Other exporters and native histograms outputs set the output
sumto 0 - effectively making up a value.E.g. exponential histogram leaves the field at 0 if unset, doesn't reject the data point.
Noted in
#5125 (comment)
Questions
sumis unset ?Historical context
There was a lot of debate over the years about what to do with the
sumwhen there are negative observations in a histogram (or summary), see prometheus/prometheus#6669 , and OpenMetrics 1.0 said that if there are negative observations in a histogram, then thesummust not be present (prometheus/OpenMetrics#193), which gave rise to optional sum. However no library actually implemented it and it's excluding legit use cases where there are negative observations. OpenMetrics 2.0 reverses curse to be more in line with implementations: prometheus/docs#2627.In general it's unlikely that anyone actually generates histograms with
sumabsent, so it seems dropping such histograms is fine. If we want to be extra careful we could allow opt-in defaulting thesumto zero.Fixes #
Changes
Please provide a brief description of the changes here.
For non-trivial changes, follow the change proposal process.
CHANGELOG.mdfile updated for non-trivial changes[chore]in the PR title to skip the changelog check