Navigation Menu

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

runtime,runtime/metrics: various histogram metrics don't follow the API #43329

Closed
mknyszek opened this issue Dec 22, 2020 · 4 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@mknyszek
Copy link
Contributor

The metrics /gc/heap/allocs-by-size:bytes, /gc/heap/frees-by-size:bytes, and /gc/pauses:seconds all have issues where the counts don't correctly line up with the buckets.

For the former two, it's the fact that the size classes have inclusive upper-bounds and exclusive lower-bounds while the API expects the opposite. For the latter, it's that the implicit bottom bucket representing the range of values (-inf, 0) isn't actually treated as a real bucket by the implementation. That bucket ends up containing 0-length durations. And there's an extra bucket at the top.

These are minor issues but should be fixed and tested.

@mknyszek mknyszek added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Dec 22, 2020
@mknyszek mknyszek added this to the Go1.16 milestone Dec 22, 2020
@mknyszek mknyszek self-assigned this Dec 22, 2020
@gopherbot

This comment has been minimized.

@gopherbot
Copy link

Change https://golang.org/cl/279467 mentions this issue: runtime: fix allocs-by-size and frees-by-size buckets

@gopherbot

This comment has been minimized.

@gopherbot
Copy link

Change https://golang.org/cl/279468 mentions this issue: runtime: shift timeHistogram buckets and allow negative durations

gopherbot pushed a commit that referenced this issue Dec 23, 2020
Currently these two metrics are reported incorrectly, going by the
documentation in the runtime/metrics package. We just copy in the
size-class-based values from the runtime wholesale, but those implicitly
have an inclusive upper-bound and exclusive lower-bound (e.g. 48-byte
size class contains objects in the size range (32, 48]) but the API
declares inclusive lower-bounds and exclusive upper-bounds.

Also, the bottom bucket representing (-inf, 1) should always be empty.
Extend the consistency check to verify this.

Updates #43329.

Change-Id: I11b5b062a34e13405ab662d15334bda91f779775
Reviewed-on: https://go-review.googlesource.com/c/go/+/279467
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
@golang golang locked and limited conversation to collaborators Dec 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants