search

opentracing-contrib / java-vertx-web

OpenTracing instrumentation for Vert.x web package
Apache License 2.0
21 stars 17 forks source link

Bug: explicitly ignore active span #9

Closed fzakaria closed 6 years ago

fzakaria commented 6 years ago

I was just reviewing the code -- and I think there may be a bug. When you create a new Span in the SpanHandler I believe the code should be ignoreActiveSpan since by default the spans get registered with the ActiveScopeManager (defaults to ThreadLocal variable) which in an event loop model, will be no good.

Something akin to 

tracer.buildSpan(operationName)
                        .ignoreActiveSpan() //this is super important since we are not thread-per-request
                        .startManual()
pavolloffay commented 6 years ago

Hi @fzakaria,

the current code also calls asChildOf(extractedContext) which forces ignoreActiveSpan however if the extracted context is null then it can cause problems. It should be fixed, thanks.

Span span = tracer.buildSpan(routingContext.request().method().toString())
                .asChildOf(extractedContext)
                .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
                .startManual();
fzakaria commented 6 years ago

Not sure what the code you included is, but it should .ignoreActiveSpan() ? If there's a fix already, you can just reference the commit and close the issue.

thanks!

pavolloffay commented 6 years ago

The code is copied from master/head. The fix is not there at the moment. Do you want to create a PR for it?