opentracing
Allow Tracer access from Span
The Tracer object should be accessible via the Span.
This will be inline with Jaeger's implementation & with the OpenTracing Golang client.
In code that I am integrating Jaeger, it performs a lot of local tracing (inner spans) -- so I've found I've had to pass both the Tracer & the parent Span down every method (it is event loop code not threaded so ScopeManager stuff isn't helpful).
Seems like exposing Tracer would just help this --
I agree on this - and as a matter of fact, the Python API has this (a Span has a reference to its Tracer). I'm wondering if there's some kind of restriction/limitation on why it hasn't been done here?
@carlosalberto its pretty easy -- i'm happy to submit PR although its a line change I think its easy.
The code change is trivial, but Tracer writers must agree on this first (in case there's a reason or situation where they can't add this). @yurishkuro maybe can bring some feedback? ;)
speaking for Jaeger I don't have a problem with it, but it is a breaking change of sorts (breaks existing interface implementations), so we should get more eyes/votes on it.
cc @tedsuo @wu-sheng @felixbarny
Should be an easy upgrade for customers though since it doesn't change/remove any existing API signatures (simply adds to the interface).
The benefit that Jaeger already has an accessor means that its likely even more compliant mixed versions (if a client is running with incorrect dependency versions).
A nit would be to make sure @Override is added to the Jaeger Tracer
For skywalking, this API has no good or bad influence. As an auto instrument agent, we keep the context in the shadow. All instances of skywalking tracer are the same. But accessing tracer from span, means you have multi tracer instances. Is this common? or why need different tracer instances? For different strategy or ?
Trying to get this one moving again, to not end up being stale - feedback requested from @felixbarny @CodingFabian
Let us know ;)
not an issue for us. but whats the rationale behind it? in the end one could argue that you do not need two objects at all and do everything through a single instance
As the OP -- the rationale was that it was tiresome to pass around both Tracer & span downstream when I wanted to create child spans
For context:
- I am in a NIO Java codebase so use of thread locals is not useable
- I could not rely on a singleton Tracer since we spin up multiple services in a single JVM during testing and each need their own Tracer instance (with appropriate process name etc..)
I don't have the need for this but I guess I'd be ok with this change.
@fzakaria is there any open source code of the NIO codebase instrumentation you mentioned? Why does propagating the trace context to the currently executing thread not work in your case? Could you outline the problems you are facing with some (pseudo) code examples?
@felixbarny
At the company I am we leverage vert.x (essentially Netty) -- as NIO framework the codebase cannot leverage ThreadLocal semantics as code execution jumps from event loop -> worker threads often. Most of the codebase as a result has to pass down the calls a common context that in our case includes the current span.
Any inner method may choose then to create a child span (local tracing timing) at which point it would also need the Tracer to do so. I found having to have both (span & tracer) to perform the action of creating a child span cumbersome and this ticket is in the spirit of making it simpler.
@fzakaria Hey, thanks for the latest feedback. Would you mind proposing a PR so we can get this item going?
Perhaps this should go through as an RFC on the spec first so we have better uniformity across languages?
@tylerbenson Oh, I'm genuinely not sure this is something that could need to go in the actual Specification or be simply considered sugar on top. For example, Tracer.activeSpan() is a shortcut, so it doesn't have to be in the spec, but not sure here...