Add Tracer#start_active_scope and deprecate Tracer#start_active_span

[indrekj]

After some discussion in
opentracing/opentracing-python#64 (comment)
and in the cross-language gitter, it was decided that #start_active_scope
makes more sense than #start_active_span because the return type is a
Scope, not a Span. Python and other similar languages will also change it.

[indrekj]

/cc @mwear @itsderek23 @tedsuo @carlosalberto @yurishkuro

[pglombardo]

My thoughts just for the record:

1. I've seen it said multiple times that we'd like to eventually hide/abstract scope from users or at least make it more transparent - but this change puts "scope" in the method name bringing Scopes to the cognitive forefront.

2. start_active_span - to understand the "active" in the method name you have to have an idea of Scopes and what they do/are for. Hence if the return is a scope, you know & understand why that is the return value

3. start_active_scope - This name makes it's clear that the return is a scope which is good, but it raises the question, do I now have to create a span on that scope? Did I just name the scope? I think this is more confusing to new users than the former.

IMO this change introduces more confusion than it solves.

[carlosalberto]

Hey @pglombardo

Thanks for the feedback. My impression is that there's no perfect solution here - so it's a matter of choosing which one is the 'best' (which, well, can also be subjective ;) )

Was wondering if I can quote (or copy & paste) your comment for opentracing/opentracing-python#89 ? ;)

[pglombardo]

Was wondering if I can quote (or copy & paste) your comment for opentracing/opentracing-python#89 ? ;)

If it's helpful, by all means...

[mwear]

I've seen this conversation in progress. Thanks for taking on this update @indrekj.

@carlosalberto @tedsuo Is this decision baked enough that it's ready to go into an official release, or is there a debate ongoing? I would like to merge this if the decision is reasonably final. If not, we could make this available in a separate branch for the time being.

[palazzem]

Hello everyone! I have some thoughts about this change:

• in general, I think that our end goal must be removing the concept of Scopes from our basic usage, unless something advanced is required. For instance, the Datadog API was tracer.trace(operation_name) (trace in the sense of the verb; we can argue it's also a noun). It was simple, it used in-process context propagation via "ScopeManagers" (slightly different in our first implementation) and it returned a Span. We never had complains about that API, I guess mostly because it was easy-to-use
• a change in the start_span method, may be that API (lot to discuss here but just to give a pointer)
• we agreed that having the Scope was necessary for some advanced use-cases (especially for Java). In that case, I'm OK in having an API that uses / creates a Scope.
• start_active_span or start_active_scope? I agree with @pglombardo that start_active_scope is better because it's clear what the method is doing. It starts a new Scope, then you know what a Scope is and you know it creates also a Span. But I agree that it could just create confusion. The basic usage is using in-process context propagation, so it means creating a Scope all the time. We're working on this for a very long time, but a newcomer may really have the question "do I need a Scope or a Span for my use case?"
• I definitely think this is a last hour change. I don't think it's wise to rush and change a public API with a week of conversation prior a 2.0 release for Python. It also introduces a deprecation warning in other libraries that is not great if this is not a stable change. It will happen again and it's costly to ask our users to change the instrumentation when their entire codebase is instrumented via OpenTracing. It will reduce the trust in the project I think

My suggestions are:

• start_active_scope() should be the right name, but not at this stage
• I think we should not merge this change with a discussion happening in a PR. I'd advocate writing an RFC that includes all possibilities to write a stable API with the goal "hide the Scope concept from basic usage"
• keep the implementation as we have now (start_active_span), because it's probably not great but it would close a big pending topic that is here from a year. We don't change anything, we ship the Python 2.0 release, we re-open the debate in an RFC so that we have time to take in consideration all the possibilities

[carlosalberto]

Hey @palazzem thanks for the feedback.

Regarding the RFC, I think it could be part of the ongoing https://github.com/opentracing/specification/blob/master/rfc/scope_manager.md - as we will be moving it to TEST stage, we can start defining specific requirements or strong suggestions for the names.

in general, I think that our end goal must be removing the concept of Scopes from our basic usag

There was some talk around this from @tedsuo on providing a hight level API (on top of the current API we all use) that wouldn't be exposing so much details. This could go in that direction.

[tedsuo]

Thanks for the feedback. There is general agreement that a simpler solution is needed for common use cases. However, it's an open question as to how to implement it.

I think this is a cross-language issue which will take some time to resolve. I'd prefer not to hold up the release of this version of the Python API, as Scopes are useful and this version is ready to go. Given that start_active_span is what we call it in Ruby and Java (SpanBulder#startActive() which is basically the same thing) I am leaning towards sticking to that convention for now.