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.

Sign up for GitHub

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

Update runtime instrumentation to new semantic conventions #5655

Open
MrAlias opened this issue May 23, 2024 · 9 comments
Open

Update runtime instrumentation to new semantic conventions #5655

MrAlias opened this issue May 23, 2024 · 9 comments
Assignees
@dashpole
Labels
area: instrumentation Related to an instrumentation package instrumentation: runtime
Milestone
v1.32.0

Comments

@MrAlias
Copy link
Contributor

MrAlias commented May 23, 2024

open-telemetry/semantic-conventions#981
@MrAlias MrAlias added area: instrumentation Related to an instrumentation package instrumentation: runtime labels May 23, 2024
@dashpole
Copy link
Contributor

Do we want a migration plan? Or do we want to just want to switch to the new conventions?

@MrAlias
Copy link
Contributor Author

MrAlias commented May 23, 2024

+1 to just switching.

@MrAlias
Copy link
Contributor Author

MrAlias commented May 23, 2024 •
edited
Loading

Migration plan:

  • Add the new semantic conventions, these will be used by default
  • Add option to turn on old metrics (in addition to the new which are default)

The old metrics will be supported for 2 releases. We do not plan to make any changes to the old metrics. If there is any CVEs they will be removed prior to the 2 release timeline.

@MrAlias MrAlias added this to the v1.28.0 milestone May 30, 2024
@dashpole dashpole mentioned this issue Jun 9, 2024
Merged
dashpole added a commit that referenced this issue Jun 13, 2024
@dashpole
Prepare for migration to new runtime metrics (#5747)
ae2a4f0 
Part of #5655

This is a refactoring to prepare for the implementation of the new
runtime metrics. It:

* Adds support for `OTEL_GO_X_DEPRECATED_RUNTIME_METRICS`, which can be
set to `true` or `false` to enable/disable the existing runtime metrics.
It initially defaults to `true` while the new metrics are under
development.
* Moves the existing runtime metrics implementation to its own internal
package, deprecatedruntime, to clearly separate it from the new metrics
being added, and to make the eventual removal easier.

This does not change any of the metrics generated, or the public API
surface of the runtime metrics package.
@dashpole dashpole mentioned this issue Jun 18, 2024
Merged
@MrAlias MrAlias modified the milestones: v1.28.0, v1.29.0 Jun 27, 2024
MrAlias pushed a commit that referenced this issue Jul 15, 2024
@dashpole @XSAM
Implement new runtime metrics (#5780)
7b8afe3 
Part of
#5655

Changes:

* Move the configuration options to `options.go` without modification.
* Implements the metrics defined here:
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/runtime/go-metrics.md.
These are still disabled by default while it is under development.
* Add unit tests

Notes:

It doesn't implement `go.schedule.duration`, as the histogram will need
some additional work to figure out.

Based on
prometheus/client_golang#955 (comment),
using go's runtime metrics should is more efficient than reading
memstats.

---------

Co-authored-by: Sam Xie <sam@samxie.me>
@dashpole
Copy link
Contributor

I just need to implement the histogram metric now.

@dashpole
Copy link
Contributor

Some notes from my rough attempt to implement the histogram metric:

@MrAlias
Copy link
Contributor Author

MrAlias commented Jul 17, 2024

  • It seems like the buckets may map to our exponential histogram. We might want to consider using that instead of a fixed-bucket histogram.

"buckets are power-of-2 sized" seems to motivate this 👍

@MrAlias
Copy link
Contributor Author

MrAlias commented Jul 17, 2024 •
edited
Loading

  • It doesn't have a Sum. I'm not sure if we can reasonably GA it without a sum. We may need to ask the Go team to add the Sum.

Sounds like a good ask for the Go team

@dashpole
Copy link
Contributor

"buckets are power-of-2 sized" seems to motivate this 👍

It is powers-of-2 sized, but with linear "sub-buckets" between the power-of-two buckets. I asked, and the buckets are also subject to change over time, so using an exponential histogram seems like not the best idea. Slightly disappointing, but maybe an opportunity to improve in the future.

@dashpole
Copy link
Contributor

dashpole commented Aug 2, 2024

I'm going to track adding the histogram metrics separately: #5974.

@MrAlias MrAlias modified the milestones: v1.29.0, v1.30.0 Aug 8, 2024
luca-filipponi pushed a commit to luca-filipponi/opentelemetry-go-contrib that referenced this issue Aug 9, 2024
@dashpole @XSAM @luca-filipponi
Implement new runtime metrics (open-telemetry#5780)
8d69243 
Part of
open-telemetry#5655

Changes:

* Move the configuration options to `options.go` without modification.
* Implements the metrics defined here:
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/runtime/go-metrics.md.
These are still disabled by default while it is under development.
* Add unit tests

Notes:

It doesn't implement `go.schedule.duration`, as the histogram will need
some additional work to figure out.

Based on
prometheus/client_golang#955 (comment),
using go's runtime metrics should is more efficient than reading
memstats.

---------

Co-authored-by: Sam Xie <sam@samxie.me>
@XSAM XSAM modified the milestones: v1.30.0, v1.31.0 Sep 9, 2024
@MrAlias MrAlias modified the milestones: v1.31.0, v1.32.0 Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dashpole dashpole

Labels
area: instrumentation Related to an instrumentation package instrumentation: runtime
Projects
None yet
Milestone
v1.32.0
Development

No branches or pull requests
3 participants
@dashpole @MrAlias @XSAM