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 record measurements with negative values #6596

Open
callumj opened this issue Jul 19, 2024 · 2 comments
Open

Allow histograms to record measurements with negative values #6596

callumj opened this issue Jul 19, 2024 · 2 comments
Labels
blocked:spec blocked on open or unresolved spec Feature Request Suggest an idea for this project

Comments

@callumj
Copy link

callumj commented Jul 19, 2024

Describe the bug
Looking at SdkDoubleHistogram.java we block negative values in histograms despite this being supported under exponential histograms (via negativebuckets).

Is this intentional or just a side effect of the standard storage of Histograms not permitting negative values?

Steps to reproduce

  • Emit a negative histogram value
  • Get a WARN
  • Nothing emitted

What did you expect to see?
Negative values supported

What did you see instead?
Warning message

What version and what artifacts are you using?
N/A

Environment
N/A

Additional context
Add any other context about the problem here.

@callumj callumj added the Bug Something isn't working label Jul 19, 2024
@jack-berg
Copy link
Member

Unfortunately this is the expectation today. According to the spec:

The value is expected to be non-negative. This API SHOULD be documented in a way to communicate to users that this value is expected to be non-negative. This API SHOULD NOT validate this value, that is left to implementations of the API.

The reason (if I recall correctly) is that when histogram measurements are restricted to positive values, the resulting sum field on output explicit bucket histogram and exponential bucket histogram points is monotonic, which allows it to be used for computing rates. This doesn't seem like a convincing argument to disallow negative values so I may be missing something.

If this is important to you, I encourage you to open an issue in the spec. A search through spec issues for the term "negative" returns a number of conversations about punting on support for negative values until after stability, so it seems appropriate to raise the conversation once again.

@jack-berg jack-berg added blocked:spec blocked on open or unresolved spec Feature Request Suggest an idea for this project and removed Bug Something isn't working labels Jul 19, 2024
@jack-berg jack-berg changed the title Should negative histogram values be supported when using Ex Allow histograms to record measurements with negative values Jul 19, 2024
@heyams
Copy link
Contributor

heyams commented Aug 16, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked:spec blocked on open or unresolved spec Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

3 participants