-
Notifications
You must be signed in to change notification settings - Fork 502
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
fix: Remove mut ref requirement for shutdown LogExporter #2764
fix: Remove mut ref requirement for shutdown LogExporter #2764
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2764 +/- ##
=======================================
- Coverage 79.7% 79.7% -0.1%
=======================================
Files 123 123
Lines 23107 23115 +8
=======================================
Hits 18426 18426
- Misses 4681 4689 +8 ☔ View full report in Codecov by Sentry. 🚀 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.
Looks good for now. The shutdown logic for tonic export can be implemented later.
Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com>
@gruebel Merged this to unblock progress! Please share any feedback on this, and I'll follow up. |
Redoing https://github.com/open-telemetry/opentelemetry-rust/pull/2634/files
LogExporter
shutdown() no longer requiremut ref
. The component that needs mutability should rely on interior mutability. Most of the exporters already use interior mutability.Metric already does not require mutable self on shutdown, this makes Logs consistent with that, and later we need to get Traces too.
Also LogProcessor, LogProvider does not require
mut ref
on shutdown, so this makes exporter aligned as well.Additionally this also fixes #2770