- Notifications
You must be signed in to change notification settings - Fork134
Stop implicitly emitting deprecated process runtime metrics#932
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
hyperlint-aibot commentedMar 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
PR Change SummaryUpdated system metrics instrumentation to stop emitting deprecated process runtime metrics and introduced new metric names for clarity.
Modified Files
How can I customize these reviews?Check out theHyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
cloudflare-workers-and-pagesbot commentedMar 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Deploying logfire-docs with |
| Latest commit: | b3aa472 |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f614868b.logfire-docs.pages.dev |
| Branch Preview URL: | https://alex-system-metrics-breaking.logfire-docs.pages.dev |
I'll resolve merge conflicts after the base gets merged, or maybe not at all until we want to make a major release. |
…etrics-breaking-changes
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.
Pull Request Overview
This PR removes deprecated process runtime metrics from the default system metrics configuration and replaces them with their newer equivalents. This is part of a major release breaking change to align with OpenTelemetry standards.
- Removes deprecated
process.runtime.*metrics from default configurations - Replaces deprecated metrics with their modern equivalents (e.g.,
process.runtime.cpu.utilization→process.cpu.utilization) - Maintains backward compatibility by allowing deprecated metrics to be explicitly requested
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
logfire/_internal/integrations/system_metrics.py | Removes deprecated metrics from type definitions and configurations, adds logic to explicitly exclude them from FULL_CONFIG |
tests/otel_integrations/test_system_metrics.py | Updates test expectations to reflect removal of deprecated metrics and adds test for explicit deprecated metric usage |
docs/integrations/system-metrics.md | Updates documentation to remove references to deprecated metrics and simplify explanations |
Comments suppressed due to low confidence (2)
tests/otel_integrations/test_system_metrics.py:72
- The test only verifies that the deprecated metric name is collected, but doesn't verify the actual metric functionality or values. Consider adding assertions to verify the metric produces expected values.
logfire.instrument_system_metrics({'process.runtime.cpu.utilization': None}, base=None) # type: ignore
Uh oh!
There was an error while loading.Please reload this page.
Builds on#930
These are breaking changes that should be included in the next major release.