-
Notifications
You must be signed in to change notification settings - Fork 184
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
Add experimental go runtime metrics semantic conventions #981
Conversation
59b9652
to
4b258a1
Compare
4b258a1
to
2b2aeb8
Compare
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 with some non-blocking clarification questions.
Can this doc be made publicly accessible? |
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.
Thanks for raising this @dashpole . But as mentioned by others, the document you linked hasn't been made public yet. It might also make sense to allow some time for discussing the contents of the document in the Go community. (But since your PR here uses "status: experimental" I guess I don't feel strongly about it)
I don't believe it can be. Feel free to request access. |
I'm hoping this will help provide feedback on the proposal. These conventions will be kept experimental until the proposal is finalized. |
b4d6740
to
63b3e16
Compare
I have posted the proposal on the Go issue tracker; no need to look at the doc anymore. Please comment at golang/go#67120. |
a0a8155
to
c24db68
Compare
1d49f7d
to
d470474
Compare
@reyang anything else needed for this to merge? |
Nope, you're all set 👍 |
Fixes #535
Changes
Add experimental runtime metrics based on the proposed set of recommended metrics from the Go team: golang/go#67120
Please direct feedback on the set of metrics on the issue above. If the recommended set is changed, I will update these conventions. Feedback on the OTel representation of those metrics is welcome.
Alternatives considered
Separate metrics for memory usage
Instead of a single go.memory.usage metric, we could have one metric for the total, and other metrics for the stack and released amounts. Separate metrics would potentially be more "future proof", if a new sub-set of memory that overlapped with released or stack memory was ever introduced. I chose the existing design because using labels is easier to visualize, and understand for users.
go.gc.* instead of go.memory.gc.*
This PR uses
go.memory.gc.*
. Instead, we could usego.gc.*
, similar to java'sjvm.gc.duration
. I went this direction because it is garbage collecting memory, and I felt it was most appropriate to compare the GC target to thego.memory.usage
andgo.memory.limit
metrics.Different naming for "total" memory metrics
/gc/heap/allocs:bytes
and/gc/heap/allocs:objects
from the go runtime are both cumulative totals for number and size of allocations made. I've opted to name themgo.memory.allocated
andgo.memory.allocations
to be concise, but users may thinkgo.memory.allocated
sounds like the same thing asgo.memory.usage
. Some alternative names I considered:go.memory.size_allocated
andgo.memory.objects_allocated
Note that we cannot use the
count
ortotal
suffixes per https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#naming-rules-for-counters-and-updowncountersgo.config.gomemlimit instead of go.memory.limit
To be consistent with the other configuration metric,
go.config.gogc
, we could name the memory limitgo.config.gomemlimit
. I've opted for thego.memory.limit
naming because it aligns withjvm.memory.limit
, and because it helps users understand that they should compare memory usage with the limit.@open-telemetry/go-approvers @felixge @mknyszek