-
Notifications
You must be signed in to change notification settings - Fork 821
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
docs(sdk-metrics): Update Histogram docs to reflect upper bound inclusivity #4829
docs(sdk-metrics): Update Histogram docs to reflect upper bound inclusivity #4829
Conversation
@@ -49,7 +49,7 @@ export interface Histogram { | |||
* | |||
* buckets: { | |||
* boundaries: [10, 20, 30], | |||
* counts: [3, 3, 1, 2], | |||
* counts: [3, 3, 2, 1], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to mention what spec describe here in the notes maybe, so is clear why the counts are like that. "buckets express the number of values that are greater than their lower bound and less than or equal to their upper bound."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@legendecas I think we discussed in the SIG meeting that this is not the case yet and that we'd change the code to be upper-bound inclusive, right? |
Yes, I fixed up a PR based on this one at #4935. |
5c1ae0a
…sivity (open-telemetry#4829) Co-authored-by: Hector Hernandez <[email protected]>
Which problem is this PR solving?
According to the specs, histogram bucket upper bounds are inclusive. This PR contains a very small change to the docs to reflect that. This might avoid confusing a newcomer to the project.
Fixes # N/A
Short description of the changes
Type of change
Docs only
How Has This Been Tested?
N/A
Checklist: