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

Jump to bottom

runtime/metrics: time histogram sub-bucket ranges are off by a factor of two [1.17 backport] #50734

Closed
gopherbot opened this issue Jan 21, 2022 · 5 comments
Closed

runtime/metrics: time histogram sub-bucket ranges are off by a factor of two [1.17 backport] #50734

gopherbot opened this issue Jan 21, 2022 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.17.8

Comments

@gopherbot
Copy link

@mknyszek requested issue #50732 to be considered for backport to the next 1.17 minor release.

@gopherbot Please open backport issues for Go 1.16 and Go 1.17.

This issues is not really possible to workaround because downstream users are supposed to take the reported bucketing for runtime/metrics histograms at face value, and they're currently just wrong. The fix is also very small, and very safe.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jan 21, 2022
@gopherbot gopherbot mentioned this issue Jan 21, 2022
Closed
@gopherbot gopherbot added this to the Go1.17.7 milestone Jan 21, 2022
@cherrymui cherrymui modified the milestones: Go1.17.7, Go1.17.8 Feb 9, 2022
@gopherbot
Copy link
Author

Change https://go.dev/cl/384621 mentions this issue: [release-branch.go1.17] runtime: simplify histogram buckets considerably

@mknyszek
Copy link
Contributor

This CL has +2, so feel free to land once the cherry-pick is approved.

@mknyszek
Copy link
Contributor

(Or let me know that the cherry-pick is not approved. :))

@dmitshur
Copy link
Contributor

Approving per discussion in a release meeting. This backport applies to both 1.17 and 1.16.

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Feb 16, 2022
@gopherbot
Copy link
Author

Closed by merging 1ba25fa to release-branch.go1.17.

gopherbot pushed a commit that referenced this issue Feb 18, 2022
@mknyszek @dmitshur
[release-branch.go1.17] runtime: simplify histogram buckets considerably
1ba25fa 
There was an off-by-one error in the time histogram buckets calculation
that caused the linear sub-buckets distances to be off by 2x.

The fix was trivial, but in writing tests I realized there was a much
simpler way to express the calculation for the histogram buckets, and
took the opportunity to do that here. The new bucket calculation also
fixes the bug.

For #50732.
Fixes #50734.

Change-Id: Idae89986de1c415ee4e148f778e0e101ca003ade
Reviewed-on: https://go-review.googlesource.com/c/go/+/380094
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 2e9dcb5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/384621
TryBot-Result: Gopher Robot <gobot@golang.org>
@knz knz mentioned this issue May 25, 2022
Closed
@golang golang locked and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.17.8
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @gopherbot @cherrymui