-
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
Use dedicated ShutdownResult for Metric SDK shutdown #2573
Use dedicated ShutdownResult for Metric SDK shutdown #2573
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2573 +/- ##
=======================================
- Coverage 79.6% 79.6% -0.1%
=======================================
Files 118 118
Lines 22473 22486 +13
=======================================
+ Hits 17902 17906 +4
- Misses 4571 4580 +9 ☔ View full report in Codecov by Sentry. |
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 LGTM and follows from our extensive discussions !
if you manage to merge it before the weekend, I could quickly do trace/logger Monday morning.
Thanks @scottgerring . @lalitb could you do another review? |
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.
Nicely done. Thanks.
I'll continue to work on Metrics area only, to avoid conflicts, so you can do logs/trace side. |
Trying to check how the changes discussed in
#2571 and #2564 would look like.
This touches just Metrics's Shutdown path.