Skip to content

disable deprecated runtime metrics by default#7418

Merged
pellared merged 17 commits intoopen-telemetry:mainfrom
dprotaso:disable-deprecated-metrics
Jun 6, 2025
Merged

disable deprecated runtime metrics by default#7418
pellared merged 17 commits intoopen-telemetry:mainfrom
dprotaso:disable-deprecated-metrics

Conversation

@dprotaso
Copy link
Copy Markdown
Contributor

@dprotaso dprotaso commented Jun 2, 2025

Part of #5655

@dprotaso dprotaso requested review from a team, dashpole and dmathieu as code owners June 2, 2025 18:03
@dprotaso dprotaso force-pushed the disable-deprecated-metrics branch from 8e5e5e4 to a24e85e Compare June 2, 2025 18:27
Copy link
Copy Markdown
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

I think we also need to update/reorganize the package documentation in

// Package runtime implements the conventional runtime metrics specified by OpenTelemetry.
to reflect that the new metrics are always produced, and that the env var re-enables the old ones. We should probably flip the docs and list the old ones first, and new ones second.

We should also make it so that we still enable the new metrics even when the legacy ones are re-enabled. I think we shouldn't return in this statement anymore:

return deprecatedruntime.Start(meter, c.MinimumReadMemStatsInterval)

Comment thread instrumentation/runtime/internal/x/x.go Outdated
@dashpole
Copy link
Copy Markdown
Contributor

dashpole commented Jun 2, 2025

This needs a changelog entry.

@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 2, 2025

@dashpole where do those go?

@dprotaso dprotaso force-pushed the disable-deprecated-metrics branch from 34e4b0a to 2faa4bb Compare June 2, 2025 18:33
@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 2, 2025

ok @dashpole changes are done PTAL

Comment thread instrumentation/runtime/doc.go Outdated
Comment thread instrumentation/runtime/runtime.go Outdated
Comment thread CHANGELOG.md Outdated
@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 2, 2025

So if this merges when is the next release that this would be a part of?

@dprotaso dprotaso force-pushed the disable-deprecated-metrics branch from 60f8147 to 070c7df Compare June 2, 2025 18:51
@dashpole
Copy link
Copy Markdown
Contributor

dashpole commented Jun 2, 2025

The last release was 2 weeks ago, so probably in the next week. We do releases as needed, but generally every 2 weeks

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2025

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.3%. Comparing base (b0a27cc) to head (14e77d6).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
instrumentation/runtime/runtime.go 0.0% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7418     +/-   ##
=======================================
- Coverage   82.3%   82.3%   -0.1%     
=======================================
  Files        205     205             
  Lines      17967   17967             
=======================================
- Hits       14801   14791     -10     
- Misses      2731    2738      +7     
- Partials     435     438      +3     
Files with missing lines Coverage Δ
instrumentation/runtime/internal/x/x.go 100.0% <100.0%> (ø)
instrumentation/runtime/runtime.go 68.7% <0.0%> (-0.7%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pellared
Copy link
Copy Markdown
Member

pellared commented Jun 2, 2025

@dprotaso dprotaso force-pushed the disable-deprecated-metrics branch from d1e2d35 to aad4477 Compare June 2, 2025 22:33
@dprotaso dprotaso requested a review from dashpole June 2, 2025 22:34
Comment thread CHANGELOG.md Outdated
@dprotaso dprotaso force-pushed the disable-deprecated-metrics branch from 0127214 to 220e866 Compare June 4, 2025 15:12
@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 4, 2025

lint failure seems unrelated

Error: R] https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/cloud_provider/aws/ecs/ | Network error: error sending request for url (https://opentelemetry.io/docs/specs/semconv/resource/) Maybe a certificate error?

@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 4, 2025

is codecov/patch check blocking?

@dashpole
Copy link
Copy Markdown
Contributor

dashpole commented Jun 4, 2025

No

@dashpole
Copy link
Copy Markdown
Contributor

dashpole commented Jun 4, 2025

@open-telemetry/go-approvers any concerns with enabling new go runtime metrics by default, and disabling the old ones?

Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Comment thread CHANGELOG.md Outdated
dprotaso and others added 2 commits June 5, 2025 13:33
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 5, 2025

@pellared made the changes let me know if you need anything else

Comment thread CHANGELOG.md
Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

LGTM

@dashpole, can you double-check? I think we could create an issue to remove OTEL_GO_X_DEPRECATED_RUNTIME_METRICS in v1.38.0.

dprotaso and others added 2 commits June 5, 2025 14:31
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Copy link
Copy Markdown
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

@dashpole, PTAL at my suggestions, feel free to edit them and apply 😉

Comment thread instrumentation/runtime/internal/x/README.md Outdated
Comment thread CHANGELOG.md Outdated
dashpole and others added 2 commits June 5, 2025 14:44
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@pellared pellared merged commit 26ecd83 into open-telemetry:main Jun 6, 2025
27 of 28 checks passed
@dprotaso dprotaso deleted the disable-deprecated-metrics branch June 8, 2025 22:42
@dprotaso
Copy link
Copy Markdown
Contributor Author

dprotaso commented Jun 8, 2025

Thanks everyone - when's the next release planned?

@dashpole
Copy link
Copy Markdown
Contributor

dashpole commented Jun 9, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants