lambcode opened a new issue #920: brave.flush annotation reported for already 
finished span
URL: https://github.com/apache/incubator-zipkin-brave/issues/920
 
 
   ## Problem
   Brave Version: 5.6.0
   
   I am encountering unexpected behavior when using Tracer.currentSpan() to get 
a span that has previously been finished. It appears that calling currentSpan() 
restarts the span to a degree in that the TraceContext is re-registered with 
PendingSpans. This in turn causes a brave.flush annotation to be reported when 
that TraceContext gets GC'd even though in reality the span has already been 
finished.
   ![Untitled 
Diagram(2)](https://user-images.githubusercontent.com/5820125/59121923-9561a100-8916-11e9-982d-953b5337b597.png)
   
   This could probably be avoided by calling abandon() on the Span that is 
returned from currentSpan(), but in our case sometimes this code lives in 
libraries that are not aware whether or not the current span needs to be 
abandoned() (it was finished previously) or if it will be finished later.
   
   This is also causing odd behavior in zipkin-ui (not lens) such as spans 
going missing presumably because it doesn't expect multiple spans with the same 
spanId that aren't shared spans. (For example I found one span had three 
documents in elasticsearch: a client span, a servier span, and another span 
with the brave.flush annotation)
   
   ## Proposed Fix
   I wonder if instead of removing PendingSpan entries from the HashMap when 
PendingSpans.remove() is invoked, these entries could be marked as finished 
somehow. In this manner PendingSpans will not forget about a TraceContext until 
it has been GC'd meaning that the context can never be registered with pending 
spans more than once.
   
   If this approach is pursued, special care will need to be taken when dealing 
with multiple Tracer instances, as each Tracer has it's own instance of 
PendingSpans that needs to remain in sync.
   
   I have attached a sample project with unit tests for both a single tracer 
and multiple tracer test that exhibits this behavior.
   
[bugjar.zip](https://github.com/apache/incubator-zipkin-brave/files/3267116/bugjar.zip)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to