-
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
Modify ForceFlush to abort after timeout/cancellation #1757
Conversation
…ntelemetry-go into 1618-ForceFlushAborts
Codecov Report
@@ Coverage Diff @@
## main #1757 +/- ##
=====================================
Coverage 78.6% 78.6%
=====================================
Files 133 133
Lines 7072 7083 +11
=====================================
+ Hits 5560 5569 +9
- Misses 1267 1268 +1
- Partials 245 246 +1
|
var bp testBatchExporter | ||
bsp := sdktrace.NewBatchSpanProcessor(&bp) | ||
|
||
err := bsp.ForceFlush(context.Background()) |
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.
To test if ForceFlush
succeeds it should be verified that if actually flushes any held span batches not just returns a nil
error. Can you extend this test to do that?
This has been a recurring issue with attempting to test this span processor. Even though this test has set the queue size to 0 and the processor is configured to block on a full queue, it looks like it's still possible for the last span to be pulled off the queue and the scheduler to switch back to the test and start I wonder if testing that a non-zero number of spans were exported or that the exported count was within 10 of the expected count might be sufficient here since we simply want to know that |
Modified the method, BatchSpanProcessor.ForceFlush, to abort after configured timeout or cancellation. Should resolve Issue #1618