-
Notifications
You must be signed in to change notification settings - Fork 494
Fix MeterProvider destructor warning when Shutdown() called manually #3513
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
base: main
Are you sure you want to change the base?
Conversation
|
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
- Add IsShutdown() method to MeterContext - Add atomic<bool> is_shutdown_ member to track shutdown state - Update MeterProvider destructor to check IsShutdown() before calling Shutdown() - Add test case for shutdown behavior - Follows same pattern as BatchSpanProcessor and SimpleLogRecordProcessor Co-authored-by: lalitb <1196320+lalitb@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3513 +/- ##
==========================================
- Coverage 90.01% 89.97% -0.03%
==========================================
Files 219 219
Lines 7044 7047 +3
==========================================
Hits 6340 6340
- Misses 704 707 +3
🚀 New features to boost your workflow:
|
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.
Per previous comments, clarify whether using generative AI is permissible or not.
Please fix the below format error from CI.
|
Co-authored-by: ThomsonTan <1041795+ThomsonTan@users.noreply.github.com>
... Fixed the formatting error by removing trailing whitespace from the test file. Commit aff62f5 |
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.
Blocking due to missing CLA
Problem
When
MeterProvider::Shutdown()
is called manually, the destructor still callsShutdown()
again, which causesMeterContext
to emit a warning:This creates unnecessary noise in logs when users properly shut down their meter providers manually before destruction.
Root Cause
The
MeterProvider
destructor unconditionally callscontext_->Shutdown()
without checking if shutdown has already been performed:Solution
This PR implements the same pattern used by other components in the codebase like
BatchSpanProcessor
andSimpleLogRecordProcessor
:IsShutdown()
method toMeterContext
- Allows checking shutdown state without side effectsatomic<bool> is_shutdown_
member - Tracks shutdown state independently of the shutdown latchMeterProvider
destructor - Only callsShutdown()
if not already shut downChanges
Before:
After:
Testing
ShutdownTwice
test validates the fixBefore Fix
After Fix
No warnings from destructor when
Shutdown()
was called manually.Fixes #3511.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
esm.ubuntu.com
/usr/lib/apt/methods/https
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.