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

Consider to remove IntHistogram and offer only DoubleHistogram (a.k.a Histogram) #257

Closed
bogdandrutu opened this issue Feb 12, 2021 · 1 comment · Fixed by #270
Closed

Comments

@bogdandrutu
Copy link
Member

bogdandrutu commented Feb 12, 2021

This decision was made in the initial beta release of the metrics, but after using this data model in the collector for some time, I came to the conclusion that they are a bit harmful (I have a bunch of places where I have to deal with conversion for this extra type). By using only the DoubleHistograms we may lose precision for the sum and for the value in the Exemplar (see #256 for details about exemplar). Same argument as described in the Exemplar case can be applied for the sum, and we also can consider the fact that usually when exporting a Histogram users will often look at the percentiles which will be anyway approximations and precision will be lost there regardless of the sum being accurate.

We will keep the IntGuage/IntSum for special cases where people really care about precision for these large numbers.

The deprecation will work this way:

  • Rename current DoubleHistogram to just Histogram.
  • Deprecate IntDouble from the metric types we support.
  • When receiving a deprecated IntHistograms type, the receiver can convert it into the new Histogram, only operation needed is to convert the int64 sum into double. Exemplars will be handled by Consider to remove IntExemplar and offer only DoubleExemplar (a.k.a Exemplar) #256.
  • In the collector we will do this in the OTLP receiver, and will remove access to the deprecated field.

/cc @jmacd @open-telemetry/specs-metrics-approvers

@bogdandrutu bogdandrutu added the enhancement New feature or request label Feb 12, 2021
@bogdandrutu bogdandrutu changed the title Consider to remove IntHistogram and offer only DoubleHistograms (a.k.a Histograms) Consider to remove IntHistogram and offer only DoubleHistogram (a.k.a Histogram) Feb 12, 2021
@bogdandrutu bogdandrutu added area:data-model spec:metrics and removed enhancement New feature or request labels Feb 12, 2021
@jmacd
Copy link
Contributor

jmacd commented Feb 23, 2021

Strong support.

Can you strike this line?

Deprecate IntDouble from the metric types we support.

It seems to conflict with

We will keep the IntGuage/IntSum for special cases where people really care about precision for these large numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants