Initial implementation of a global Tracer.

[carlosalberto]

Hey all,

A little while ago I felt tempted to try out an actual global Tracer for Python, similar to what Java has - and I ended up with this simple approach. Didn't want to work too much on it before getting some initial feedback/approval, as I remember this was a slightly polemical issue ;)

The current approach:

• Exposes a proxy Tracer that forwards calls to an actual registered instance, falling back to the noop Tracer.
• Can be registered through opentracing.init_global_tracer() (liked the Javascript name more than the Java one ;) ) and fetched by opentracing.get_global_tracer().
• opentracing.tracer gets, by default, assigned to it opentracing.get_global_tracer(), to keep backwards compatibility. This way people could do an incremental update - and work just fine if they keep the old get & replace opentracing.tracer behavior:

# This would *still* work, albeit in kind of deprecated mode.
opentracing.tracer = MyTracer()
opentracing.tracer.start_span(...)

# However, users would be informed to do the assignation, from now on,
# through init_global_tracer(), and keep the rest of their code
# untouched.
opentracing.init_global_tracer(MyTracer())
opentracing.tracer.start_span(...)

# Else, simply use:
opentracing.init_global_tracer(MyTracer())
opentracing.get_global_tracer().start_span(...)

Thoughts? If approved, will add docs & a few more tests, etc ;)

@yurishkuro @palazzem @pglombardo @tedsuo

[yurishkuro]

So this is adding a delegation - what problem with the existing approach does it solve?

[yurishkuro]

not sure of the value of employing the lock here but not in the other methods that reference the same _tracer var.

[carlosalberto]

Oh, it's a similar take as the one used in Java - doing locks for setting it, but trying to keep stuff light on the global Tracer retrieval. Ideally I'd use lock on all the places, but that would be an over-kill for every time this global tracer is fetched.

[carlosalberto]

Hey @yurishkuro thanks for the review.

So the idea is to have something slightly less fragile than what we have now, and have people fetch the global Tracer themselves if they want - for reference, in opentracing/__init__.py it's written:

# Note: it should be accessed via 'opentracing.tracer', not via
# 'from opentracing import tracer', the latter seems to take a copy.
tracer = Tracer()

So with this approach, users could just grab it and forget about this detail (if they want) ;) Of course, the advantages may not be as many as what we have now, thus the call for feedback.

[palazzem]

While the problem is clear to me, I'm not sure we need a Proxy class. Can't we simply store a _TRACER with/without the Lock (let's chat in the other thread about it) so that when you get_global_tracer() you just return the tracer instance? same applies when you init_global_tracer(tracer) where you change the _TRACER instance.

My reasoning is:

• you should get the same result
• you avoid code duplication that is needed everytime we change something in the underlying Tracer implementation (new API, name change, etc...)

This will avoid a singleton class that is not very common in Python. Of course tell me if I'm missing something.

[carlosalberto]

I would be fine with this slightly simpler approach as well - in this case, we would need to 'deprecate' the old opentracing.tracer usage, and advise users to use opentracing.get_global_tracer() instead (under the hood, we could still be using opentracing.tracer, for legacy code, etc).

from opentracing import get_global_tracer(), init_global_tracer()

# previously opentracing.tracer
my_lib_init_tracing(get_global_tracer())

Would that be fine with you?

[palazzem]

Yes I think that would work! But of course this is a suggestion. Happy to hear what others think about it!

[yurishkuro]

Sgtm, but I'd name them global_tracer() and set_global_tracer()

[carlosalberto]

@yurishkuro Sweet ;) Will update the PR!

[palazzem]

Agreed!

[palazzem]

I got the reasoning here, and I agree that having this constraint is not ideal. In general, would be great that users don't have to think when it's the right time for the import.

[carlosalberto]

Updated 👍 Let me know @yurishkuro @palazzem

[carlosalberto]

Since these changes have been sitting here for a few days now, think is it ok to merge @yurishkuro ? Let me know ;)