zipkin-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-zipkin-brave] lambcode opened a new issue #920: brave.flush annotation reported for already finished span
Date Fri, 07 Jun 2019 18:06:08 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@zipkin.apache.org
For additional commands, e-mail: dev-help@zipkin.apache.org


Mime
View raw message