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

Allow histograms to specify negative bucket bounds #156

Closed
x13n opened this issue Jun 6, 2020 · 4 comments · Fixed by #163
Closed

Allow histograms to specify negative bucket bounds #156

x13n opened this issue Jun 6, 2020 · 4 comments · Fixed by #163

Comments

@x13n
Copy link
Contributor

x13n commented Jun 6, 2020

According to the proto documentation, histogram bucket bounds can only be positive, which makes it impossible for OpenTelemetry to be fully compatible with Prometheus, which allows negative bounds as well.

Prometheus receiver doesn't respect this and sends negative bucket bounds down OpenTelemetry collector pipelines anyway. This can lead to bugs, as processors/exporters can make incorrect assumptions about the data (in fact, at least Stackdriver exporter already does).

This is a request to extend OpenTelemetry data model to treat first histogram bucket as (-inf, buckets[0]) instead of [0, buckets[0]).

/cc @bogdandrutu @rghetia @nilebox @mayurkale22

@nilebox
Copy link
Member

nilebox commented Jun 9, 2020

Should this be covered by the spec?
e.g. open-telemetry/oteps#89, which mentions compatibility with Prometheus histograms.

@bogdandrutu
Copy link
Member

Please draft a PR.

@jmacd
Copy link
Contributor

jmacd commented Jun 17, 2020

Should this be covered by the spec?

I think of this as an OTLP issue, not a spec issue.

x13n added a commit to x13n/opentelemetry-proto that referenced this issue Jul 3, 2020
x13n added a commit to x13n/opentelemetry-proto that referenced this issue Jul 3, 2020
x13n added a commit to x13n/opentelemetry-proto that referenced this issue Jul 6, 2020
@x13n
Copy link
Contributor Author

x13n commented Jul 10, 2020

I drafted a PR, please take a look: #163

bogdandrutu added a commit that referenced this issue Jul 17, 2020
Fixes #156

@nilebox @bogdandrutu @jmacd

Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants