-
Notifications
You must be signed in to change notification settings - Fork 600
Chore: use shutdown result type in LogExporter trait #2588
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
Chore: use shutdown result type in LogExporter trait #2588
Conversation
| exporter.shutdown(); | ||
| exporter | ||
| .shutdown() | ||
| .map_err(|e| LogError::Other(Box::new(e)))?; |
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.
this should be ShutdownError::InternalFailure?
Same with the else branch here dealing with mutexpoison.
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.
@cijothomas to be clear you're suggesting that we change trait LogProcessor's shutdown signature as well, right?
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.
yes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2588 +/- ##
=======================================
- Coverage 79.5% 79.5% -0.1%
=======================================
Files 118 118
Lines 22480 22498 +18
=======================================
+ Hits 17878 17886 +8
- Misses 4602 4612 +10 ☔ View full report in Codecov by Sentry. |
106e4b4 to
39f3127
Compare
1ef8a02 to
b8ee60c
Compare
b8ee60c to
466c1ec
Compare
Fixes #
Design discussion issue (if applicable) #
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes