-
Notifications
You must be signed in to change notification settings - Fork 887
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
Add ExponentialHistogram to Metrics data model #1935
Conversation
@open-telemetry/specs-metrics-approvers this is ready for review. |
…ication into jmacd/expohistomodel
FYI I have posted reference implementations that calculate bucket indexes described by the formulas here. These were captured from the review thread of open-telemetry/opentelemetry-proto#322 here, now available: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @yzhuge!
…ication into jmacd/expohistomodel
@jmacd all looks good now. |
@jmacd "the lower-boundary of index -68 cannot be represented using a double". Similarly, to be bullet proof, we need guard against overflow at the high end of "double": Not sure it is worth mentioning such extreme cases in the spec though. Maybe it is, spec has to be bullet proof. "Should the producer bother to check that an index can be reverse-mapped before using it?" And I assume you are talking about the exponent extraction method. The log method generally does not work well in the subnormal range. I had to skip it in subnormal tests (https://github.com/newrelic-experimental/newrelic-sketch-java/blob/main/src/test/java/com/newrelic/nrsketch/indexer/BucketIndexerTest.java#L400) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming the broken link will be fixed. https://github.com/open-telemetry/opentelemetry-specification/pull/1935/files#r723471163
Co-authored-by: Reiley Yang <reyang@microsoft.com>
…ication into jmacd/expohistomodel
Co-authored-by: Aaron Abbott <aaronabbott@google.com>
Fixes #1932.
Changes
Adds statements describing the ExponentialHistogram data point introduced by open-telemetry/opentelemetry-proto#322.
This is meant to capture details about acceptable precision limits, how to handle out-of-range values for producers and consumers. See open-telemetry/opentelemetry-proto#322 (comment)
Related OTEPs:
https://github.com/open-telemetry/oteps/blob/main/text/0149-exponential-histogram.md