semantic-conventions icon indicating copy to clipboard operation
semantic-conventions copied to clipboard

Add experimental go runtime metrics semantic conventions

Open dashpole opened this issue 1 year ago • 5 comments

Fixes https://github.com/open-telemetry/semantic-conventions/issues/535

Changes

Add experimental runtime metrics based on the proposed set of recommended metrics from the Go team: https://github.com/golang/go/issues/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 use go.gc.*, similar to java's jvm.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 the go.memory.usage and go.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 them go.memory.allocated and go.memory.allocations to be concise, but users may think go.memory.allocated sounds like the same thing as go.memory.usage. Some alternative names I considered:

  • go.memory.size_allocated and go.memory.objects_allocated

Note that we cannot use the count or total suffixes per https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#naming-rules-for-counters-and-updowncounters

go.config.gomemlimit instead of go.memory.limit

To be consistent with the other configuration metric, go.config.gogc, we could name the memory limit go.config.gomemlimit. I've opted for the go.memory.limit naming because it aligns with jvm.memory.limit, and because it helps users understand that they should compare memory usage with the limit.

@open-telemetry/go-approvers @felixge @mknyszek

dashpole avatar Apr 29 '24 18:04 dashpole

Add experimental runtime metrics based on the proposed set of recommended metrics from the Go team: https://docs.google.com/document/d/1NBeEYxld5lA_UUXoFniBO0a6_ryHYgFC0hgUAjpfdDY/edit?resourcekey=0-NBd-0IZAYTtiPej124rR2w&tab=t.0

Can this doc be made publicly accessible?

trask avatar Apr 29 '24 23:04 trask

Can this doc be made publicly accessible?

I don't believe it can be. Feel free to request access.

dashpole avatar Apr 30 '24 14:04 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'm hoping this will help provide feedback on the proposal. These conventions will be kept experimental until the proposal is finalized.

dashpole avatar Apr 30 '24 14:04 dashpole

I have posted the proposal on the Go issue tracker; no need to look at the doc anymore. Please comment at https://github.com/golang/go/issues/67120.

mknyszek avatar Apr 30 '24 20:04 mknyszek

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: dashpole / name: David Ashpole (3781bae6165b02f30d039b20c45fd920a67a0505, 998b8ced138864a20d64939ad5f1ffc908681dcb, 79dad69aeae532a267c7ee56cf4bed108517e9c6, 79493afcb851c8e24f9f6f6e5fd21850236a6a92, 16d3a0aa3c22dfecdf76ba8be6a5def8ab0f9b81, bde0f9b58247a37ef9d8bf237583c22757a3b8d1, c527c4b64d19424b07ed25da43dedfb091b34084, 66a99653119dc203e150e22043c583f8d6e04ea0, 9d185f252a7e16983b20d8145f04eaad821af865, 9929f8e8237179eb35b721c6b249c1dc6b7a1f64, 65fca8d4a2aaa21fbe4cf97602ba97c21e9f1055, bf32e4f2397ae2c6c864782a984d0f0e5f6dd538, a7083523ce2c061bde5aab959f6a00901fae9b84, 7ecfa39b6b432e3019319b87b7d0eb0b73d0dc2a, eed3414fcb9f9de88f413d8a813cb4c451fc41b9, d1ad50f4e671866841ba8e514f77febf68a6c9b0, 09aeb7786ba244ccee1ee876c4d55d27756381ce, 32e4ebe1f5d7aa2965564f6d2fd405a019b70fce, c456bb3aec3bccc8357f642bad4785d60f56b0c8, 7a4fb6dfefbd6693f83067acea5278c775e2b716)

@reyang anything else needed for this to merge?

dashpole avatar May 23 '24 16:05 dashpole

@reyang anything else needed for this to merge?

Nope, you're all set 👍

reyang avatar May 23 '24 16:05 reyang