-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove locking from Jaeger exporter shutdown/export #1807
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1807 +/- ##
=======================================
- Coverage 78.4% 78.4% -0.1%
=======================================
Files 135 135
Lines 7236 7249 +13
=======================================
+ Hits 5679 5686 +7
- Misses 1307 1313 +6
Partials 250 250
|
BenchmarkFrom #1805 :
From this branch with a cherry-pick of that commit:
This looks to add 3 allocs/op and not a definitive ns/op difference. This seems like a decent trade-off given the ability to hook into the context. Additionally, this (allocs and time) will be reduced much more in the ultimate destination of #1803. |
Replace the use of a lock to guard a state variable for shutdown of the Exporter with a channel. Additionally, hook this new channel into the context used during
ExportSpans
so if the exporter is shutdown in the middle of a call to this function it will cancel the context and flush.Part of #1803 and needed for #1799.