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

Prepare for migration to new runtime metrics #5747

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Jun 9, 2024

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 requested review from MadVikingGod and a team as code owners June 9, 2024 15:52

// BoolFeature is an experimental feature control flag. It provides a uniform way
// to interact with these feature flags and parse their values.
type BoolFeature struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This differs from the "Feature" pattern used elsewhere because I wanted to be able to have a default value of "true". Rather than make the existing pattern more complex to support this, I simplified it to fit this packages' needs.

The value set must be the case-insensitive string of `"true"` to enable the
feature, and `"false"` to disable the feature. All other values are ignored.

[previous runtime metrics conventions]: go.opentelemetry.io/contrib/instrumentation/runtime/internal/deprecatedruntime
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link will not work until after the next release.

Copy link

codecov bot commented Jun 9, 2024

Codecov Report

Attention: Patch coverage is 7.34463% with 164 lines in your changes missing coverage. Please review.

Project coverage is 63.9%. Comparing base (a794a70) to head (8c344a3).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #5747   +/-   ##
=====================================
  Coverage   63.9%   63.9%           
=====================================
  Files        195     197    +2     
  Lines      12267   12285   +18     
=====================================
+ Hits        7849    7862   +13     
- Misses      4196    4201    +5     
  Partials     222     222           
Files Coverage Δ
instrumentation/runtime/internal/x/x.go 100.0% <100.0%> (ø)
instrumentation/runtime/runtime.go 0.0% <0.0%> (ø)
...tion/runtime/internal/deprecatedruntime/runtime.go 0.0% <0.0%> (ø)

Copy link
Contributor

@MadVikingGod MadVikingGod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

Are we going to keep the same scope for the deprecated metrics, and will it be the same as the scope of the new metrics?

@dashpole
Copy link
Contributor Author

I was going to keep the same scope for the deprecated metrics. It doesn't make sense to me to modify the scope if they are going away anyways.

@dashpole dashpole merged commit ae2a4f0 into open-telemetry:main Jun 13, 2024
21 of 23 checks passed
@dashpole dashpole deleted the internal_runtime_metrics branch June 13, 2024 19:46
@MrAlias
Copy link
Contributor

MrAlias commented Jun 15, 2024

Looks like this introduced a broken link. I'll look into submitting a fix.

@MrAlias MrAlias added this to the v1.28.0 milestone Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants