-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix circular dependency in Performance Counters on Mac #44661
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
Conversation
...pentelemetry-exporter/azure/monitor/opentelemetry/exporter/_performance_counters/_manager.py
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Show resolved
Hide resolved
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 fixes a circular dependency issue in Performance Counters on Mac and other platforms where process I/O counters are unavailable. When _PerformanceCountersManager logged a warning about unavailable I/O counters, it triggered the _PerformanceCountersLogRecordProcessor, which in turn tried to access the not-yet-initialized singleton, causing an infinite lock.
Changes:
- Reordered pipeline setup to initialize metrics (with Performance Counters) before tracing and logging
- Removed non-actionable warning log for unavailable I/O counters on Mac/Linux
- Added test coverage to verify metrics setup occurs before logging and tracing setup
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| azure/monitor/opentelemetry/_configure.py | Moved metrics pipeline setup before tracing and logging to ensure Performance Counters are initialized before processors |
| azure/monitor/opentelemetry/exporter/_performance_counters/_manager.py | Removed non-actionable warning log for unavailable Process I/O Rate counter |
| tests/test_configure.py | Added test assertions to verify metrics setup order precedes logging and tracing |
| CHANGELOG.md | Documented bug fix with placeholder PR number |
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py
Outdated
Show resolved
Hide resolved
…metry/_configure.py Co-authored-by: Copilot <[email protected]>
Description
Issue: langchain-ai/langchain-azure#234
On Mac, the I/O performance counter is not available. This was a known limitation, but the warning log it produced in _PerformanceCountersManager led to a circular dependency. As a log, it triggered the _PerformanceCountersLogRecordProcessor which in turn relied on _PerformanceCountersManager. This led to an infinite lock on the singleton.
To solve this, I moved the Metrics pipeline set up to before Logs and Tracing. This way, the Performance Counters are fully set up before the _PerformanceCountersLogRecordProcessor and _PerformanceCountersSpanProcessor are added. I also removed the log itself because it is not useful as a warning. If a customer is on a Mac or another OS that does not have process I/O, there is nothing for them to do. They will still be able to get all the other Performance Counters.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines