Stabilize Prometheus Native Histogram -> OTLP (Exponential) Histogram#4898
Stabilize Prometheus Native Histogram -> OTLP (Exponential) Histogram#4898krajorama wants to merge 8 commits intoopen-telemetry:mainfrom
Conversation
Fixes: open-telemetry#4748 See task list in the issues comments. Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
0a31c07 to
8cf1fd9
Compare
| - `StartTimeUnixNano` is set to the `Start Timestamp` timestamp, if available. | ||
| - `AggregationTemporality` is set to `cumulative`. | ||
|
|
||
| A Native histogram with custom buckets (NHCB) schema (i.e. schema -53) and which |
There was a problem hiding this comment.
I'm still nervous about "leaking" the NHCB concept outside of the prometheus server / prometheus receiver. E.g. SDK bridges from Prom -> OTel would never need to implement this... WDYT about leaving this out of the specification and considering it implicit in the (classic) histogram section.
There was a problem hiding this comment.
Well the prometheusreceiver will receive NHCB from scrape and the prometheusremotewritereceiver can receive them over Remote-Write 2.0 , so I'm not sure we can ignore it. I think this is the correct place for it. At least I would search for it here.
There was a problem hiding this comment.
I'm not that concerned about the prometheus receiver, as we rely on lots if internal implementation details of the prometheus server. But if we do it in PRW 2.0, then the cat is out of the bag (it is part of protocols now).
I still think this would make more sense to document this as part of the histogram section, as translation will largely follow the rules in that section. Maybe something like "In some cases, classic prometheus histograms are represented as Native Histograms with the schema -53 for performance or cost reasons. These are referred to as "Custom Bucket native histograms". Such histograms MUST follow the same rules for classic histograms below."
There was a problem hiding this comment.
I've moved the text to Histograms: 46d0f72
I think it looks terrible. And not consistent with how Prometheus documents NHCB. For Prometheus NHCB is a native histogram. Also not sure how to handle the last sentence in the native histograms: "Native Histograms with Schema outside of the range [-4, 8] and not equal to -53 MUST be dropped."
There was a problem hiding this comment.
Also the histogram part is stable, so I don't know if we can make this change?
There was a problem hiding this comment.
Also the histogram part is stable, so I don't know if we can make this change?
Usually we would add a development header above the new text if we wanted to do that.
But reading it I agree with you. I had imagined that we could say "here is how you get a Prometheus histogram from NHCB", and then just point to the histogram spec. But if we write it as a complete section it does look out of place. Sorry for the churn, you can revert to what you had previously.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> Coded with Claude Sonnet 4.6.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
This reverts commit 46d0f72.
Fixes #4748
Changes
Enacted from #4748 (comment)
Additionally added notes on
CountandSumfor special value cases. Note that the OTel metric data model does not have "MUST" requirements on the special values or if the overallCountmust be equal to the sum of all buckets.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