Skip to content
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

HistogramValue.Bucket.Exemplar is inadequate #81

Closed
jmacd opened this issue Dec 10, 2019 · 0 comments · Fixed by #193
Closed

HistogramValue.Bucket.Exemplar is inadequate #81

jmacd opened this issue Dec 10, 2019 · 0 comments · Fixed by #193
Labels
release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics

Comments

@jmacd
Copy link
Contributor

jmacd commented Dec 10, 2019

The HistogramValue Bucket supports one exemplar per bucket, with exemplar values being restricted to string-valued attributes. This is particularly restrictive, for a number of reasons:

  1. It's not possible to provide exemplars for non-histograms
  2. It's not possible to provide more than one exemplar per bucket
  3. It requires converting certain data types to a string, which will require additional specification

I would remove this support entirely until a full set of requirements can be drafted. (1) It's meaningful and reasonable to support encoding exemplars for any kind of metric instrument. (2) It's certainly useful to provide more than one exemplar per instrument/value-range, as a means of conveying a sample distribution, (3) There is a common request (e.g., open-telemetry/opentelemetry-specification#381, https://kccncna19.sched.com/event/UaXX/deep-linking-metrics-and-traces-with-opentelemetry-openmetrics-and-m3-rob-skillington-chronosphere) to include trace context in these exemplars, and forcing them to be encoded as string-attribute lists is onerous (and inefficient)--probably trace context should be specialized.

@jmacd jmacd added bug Something isn't working spec:metrics and removed bug Something isn't working labels Dec 12, 2019
MrAlias added a commit to MrAlias/opentelemetry-proto that referenced this issue Apr 15, 2020
Update metric descriptor to contain information on the data qualities.
This includes:

 - The meaning of the time interval measurements were made over is now
   described by the `interval` value.
 - The monotonic nature of the data is captured if known.
 - If the data values are restricted to a subset of numbers.

The `SummaryDataPoint` and `HistogramDataPoint` were merged into a
unified `Distribution` message that contains statistics about the
observed population of values.

Adds support for multiple histogram bucket definitions and adds a linear
bucket definition message.

Includes an explicit minimum and maximum for a `Distribution` (open-telemetry#122).

Removes the exemplar code (open-telemetry#81). This can be added back in as a more
generalized case now given the new `Data` structure of the Metrics. But,
it has been left for a separate PR.

Unifies the common parts of the previous `*DataPoints` into a unified
`Data` message.

This is not complete. It likely needs better names and it certainly
needs comments.

Related to open-telemetry#125
Fixes to open-telemetry#81
@bogdandrutu bogdandrutu added release:required-for-ga Must be resolved before GA release, or nice to have before GA and removed release:required-for-ga labels Jul 30, 2020
@andrewhsu andrewhsu added this to Required/Allowed for GA, resolved. in GA Spec Burndown Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics
Projects
No open projects
GA Spec Burndown
  
Required/Allowed for GA, resolved.
2 participants