Adds $closeSpanOnFinish as second parameter for activate

[jcchavezs]

Background

Tracer::startActiveSpan starts and span that is captured in a scope and one of the SpanOptions allows you to determine whether the Scope will be closed when finishing the span by passing ['close_span_on_finish' => true].

Problem

For usability matters, $closeSpanOnFinish should allow you to be passed when doing ScopeManager::activate. Those cases occur when starting an Span not as an active one and in such a case it is impossible to pass a value for close_span_on_finish.

Proposal

Changes ScopeManager::activate(OpenTracingSpan $span) into ScopeManager::activate(OpenTracingSpan $span, $closeSpanOnFinish) so you can also decide to pass the $closeSpanOnFinish when activating a span in a Scope.

Java already does this: https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/ScopeManager.java#L36

Ping @yurishkuro @tedsuo @beberlei @ellisv

[jcchavezs]

Ping @eligijusvitkauskas-home24 @avsej @felixfbecker

[ellisv]

Because of 3010347 commit we have inverted so that Span finish is able to close Scope (thought I am personally not a fan of as we leak Scope concept to Spans). But current option name is misleading. Spans we finish, Scopes we close. So currently an option should be named something like close_scope_on_finish.

Now that a Span has to be aware of a Scope in order to close it this might be problematic as inside of ScopeManager::activate() we accept Span interface which does not have any methods so that we can pass the scope to it.

Ideally for me I would rather go back to concept of startActiveSpan() returning a Scope to not leak Scopes to Spans as it is done in other OT libs.

But in order to keep everything as it is now you would also need to introduce a method in Span interface named something like setScopeToClose()

[yurishkuro]

If span is aware of the scope, then I think there was a mistake in the design. Java has finishOnClose ie "finish underlying span on scope close". I think this API needs that as well.

[jcchavezs]

@ellisv @yurishkuro I opened a PR to address this issue: https://github.com/opentracing/opentracing-php/pull/61/files

[jcchavezs]

Closed by #61