open-telemetry
Histogram Record operation should be explicit about not recording negative values
What are you trying to achieve?
I'm trying to make sure that all the languages are consistent with regards to the implementation of the explicit bucket histogram.
The specification should be clear about the expected behavior of implementations when a negative number is passed to the Record operation of a Histogram.
Right now it says:
The amount of the Measurement, which MUST be a non-negative numeric value.
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#record
What did you expect to see?
I expect the text to be clear.
Suggestion:
The amount of the Measurement, which MUST be a non-negative numeric value. Negative numbers MUST be ignored and an optional error message might be reported.
Additional context.
As @reyang pointed out, this might be a bigger issue since Exponential histograms do support negative numbers. Maybe a solution is to move the requirements with regards to the parameter of the Record operation of Histograms to the SDK metrics specification, so that we can have a definition for Explicit Bucket histograms and another for Exponential Histograms?
The suggested clarification is welcome. As for the additional context, I feel that support for negative observations is a post-1.0 issue to be addressed.
I think for 1.0, Histogram instruments should explicitly disallow negative measurements, as @jmacd suggests. Please submit a PR if you have time!
@rapphil I agree with @jmacd and think that your suggestion is a good one. Would you be able to submit a PR with that change?
Hi @rapphil we see the language was changed in the spec related to histogram measurements. We have seen no feedback on the issue and we think it'd require more info to get it reconsidered. I'm going to close this issue, but if you think this is still an issue, please feel free to leave a comment and we'll re-open it.
