BraveScopeManager.active() and opentracing-jaxrs

[jmif]

Hi there,

We're having some issues with closing Scopes / Spans with opentracing-jaxrs. This is related to opentracing-contrib/java-jaxrs#87 and I'm hoping to get your input on the issue with respect to BrowserScopeManager / BrowserScopeManager.active()

At a high level (more details in referenced issue) the JAXRS filter tries to close the current scope at the end of the request and it grabs the current scope from the BraveTracer's BraveScopeManager here.

It then attempts to close the scope however the Scope returned from the BraveScopeManager has a noop close method. This results in the scope not being closed and being used across requests.

Is this the expected behavior of active() (seems to be given the comment on the method)? If so what is the best way to get the active scope and close it in this context?

[codefromthecrypt]

please file a bug to the jaxrs code. not only is that style of inteception problematic (why brave moved to servlet filter + decoration for jax-rs), but it is also unnecessary to use implicits to close a scope. using an attribute is more reliable.

[codefromthecrypt]

Here's the last code we used to ensure you don't have to use implicits to close a scope openzipkin/brave@9d98d56#diff-27b317da912256b195ad886eac72f9f1L72 and here's why we removed that code in general https://github.com/openzipkin/brave/tree/master/instrumentation/jaxrs2#why-dont-we-use-containerresponsefilter

[pavolloffay]

Agree that it's safer to pass the span around in attribute. Actually the scope in this case.

Is there any reason why BraveScopeManager.active().close() is noop? I don't think it complies with OpenTracing API. It will cause problems in other use-cases.

[pavolloffay]

This is also related to opentracing/opentracing-java#267 and

[codefromthecrypt]

the opentracing api is both wrong and underspecified. refer to reactive streams for an example of a properly specified api. it should be a bug to close a scope from an alternate thread. the api is wrong as it encourages doing this.

[codefromthecrypt]

while still thrashing there seems to be some hope that the api will be corrected opentracing/opentracing-java#267 (comment)

[rogueai]

Hi guys, I'd like to add to this issue both to try to understand an issue I'm facing, and to figure out a proper fix for that.
I'm using spring messaging in my project, and it seems their OpenTracingChannelIntercpetor tries to close the scope after the message is sent, see OpenTracingChannelInterceptor code here
So it looks like this is allowed by the OpenTracing spec, now the fact that in brave opentracing this is a No-op, that leaves my spans open across messages and I never get any tracing for them.
I tried to put a fix for that by caching the scopes in the interceptor when the spans start, but it does look like a hack to me, plus I'm worried about messing up the scopes.

Do you have suggestions on that? Can I achieve the same result without overriding their interceptor, or perhaps there's a better way to handle that?

Thanks.

[codefromthecrypt]

file a bug on their side. opentracing isn't a real spec

…

[codefromthecrypt]

sadly it looks like 0.32 still exposes (hence implies tracking of) the current scope instance opentracing/opentracing-java#267

[codefromthecrypt]

opentracing 0.33 removes this method