Stabilize Prometheus Native Histogram -> OTLP (Exponential) Histogram#4898
Stabilize Prometheus Native Histogram -> OTLP (Exponential) Histogram#4898krajorama wants to merge 1 commit 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
| are spans, otherwise left at 0. Note that Prometheus Native Histogram | ||
| buckets are indexed by upper boundary while Exponential Histograms are | ||
| indexed by lower boundary, hence the minus one. |
There was a problem hiding this comment.
nit: maybe rephrase this as "The minus one is because Prometheus Native ..."
| - `Sum` is converted to the Exponential Histogram `Sum`. | ||
| [Stale NaN value](https://github.com/prometheus/prometheus/blob/main/model/value/value.go). | ||
| Otherwise, | ||
| - `Count` is converted to Exponential Histogram `Count`. Note that the `Count` |
There was a problem hiding this comment.
// The number of values in the population. Must be
// non-negative. This value must be equal to the sum of the "bucket_counts"
// values in the positive and negative Buckets plus the "zero_count" field.
fixed64 count = 4;
Not sure if we are allowed to have it be greater than the sum of other fields... Maybe we need to actually sum up all of the bucket counts instead of using the count directly?
| use. | ||
| - The Native Histogram may contain overflow buckets. If converted to | ||
| an Exponential Histogram bucket, the overflow bucket would map to values | ||
| outside the IEEE float range. The Native Histogram SHOULD be dropped in |
There was a problem hiding this comment.
Curious why the recommendation is to drop the whole histogram. We should definitely always drop the overflow bucket, though.
| - `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.
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