[#60] Changes the API to return a scope when starting an active span …

…and makes the span not aware of the scope.

README.md

@@ -27,7 +27,7 @@ composer require opentracing/opentracing
 
 When consuming this library one really only need to worry about a couple of key
 abstractions: the `Tracer::startActiveSpan` and `Tracer::startSpan` method,
-the `Span` interface, and binding a `Tracer` at bootstrap time. Here are code snippets
+the `Span` interface, the `Scope` interface and binding a `Tracer` at bootstrap time. Here are code snippets
 demonstrating some important use cases:
 
 ### Singleton initialization
@@ -58,7 +58,7 @@ $spanContext = GlobalTracer::get()->extract(
 function doSomething() {
     ...
 
-    $span = GlobalTracer::get()->startActiveSpan('my_span', ['child_of' => $spanContext]);
+    $span = GlobalTracer::get()->startSpan('my_span', ['child_of' => $spanContext]);
 
     ...
 
@@ -77,7 +77,7 @@ function doSomething() {
 It's always possible to create a "root" `Span` with no parent or other causal reference.
 
 ```php
-$span = $tracer->startActiveSpan('my_first_span');
+$span = $tracer->startSpan('my_first_span');
 ...
 $span->finish();
 ```
@@ -95,11 +95,11 @@ Unless you are using asynchronous code that tracks multiple spans at the same
 time, such as when using cURL Multi Exec or MySQLi Polling you are better
 of just using `Tracer::startActiveSpan` everywhere in your application.
 
-The currently active span gets automatically closed and deactivated from the scope
-when you call `$span->finish()` as you can see in the previous example.
+The currently active span gets automatically finished when you call `$scope->close()`
+as you can see in the previous example.
 
 If you don't want a span to automatically close when `Span::finish()` is called
-then you must pass the option `'close_span_on_finish'=> false,` to the `$options`
+then you must pass the option `'finish_span_on_close'=> false,` to the `$options`
 argument of `startActiveSpan`.
 
 An example of a linear, two level deep span tree using active spans looks like
@@ -112,10 +112,11 @@ $root = $tracer->startActiveSpan('php');
 
         $http = $tracer->startActiveSpan('http');
         file_get_contents('http://php.net');
-        $http->finish();
+        $http->close();
 
-    $controller->finish();
-$root->finish();
+    $controller->close();
+
+$root->close();
 ```
 
 #### Creating a child span assigning parent manually
@@ -153,11 +154,11 @@ $child = GlobalTracer::get()->startActiveSpan('my_second_span');
 
 ...
 
-$child->finish();
+$child->close();
 
 ...
 
-$parent->finish();
+$parent->close();
 ```
 
 ### Serializing to the wire
@@ -237,7 +238,7 @@ register_shutdown_function(function() {
 This is optional, tracers can decide to immediately send finished spans to a
 backend. The flush call can be implemented as a NO-OP for these tracers.
 
-### Using Span Options
+### Using `StartSpanOptions`
 
 Passing options to the pass can be done using either an array or the
 SpanOptions wrapper object. The following keys are valid:
@@ -246,6 +247,8 @@ SpanOptions wrapper object. The following keys are valid:
 - `child_of` is an object of type `OpenTracing\SpanContext` or `OpenTracing\Span`.
 - `references` is an array of `OpenTracing\Reference`.
 - `tags` is an array with string keys and scalar values that represent OpenTracing tags.
+- `finish_span_on_close` is a boolean that determines whether a span should be finished when the
+scope is closed or not.
 
 ```php
 $span = $tracer->startActiveSpan('my_span', [

src/OpenTracing/Exceptions/InvalidSpanOption.php

@@ -103,10 +103,10 @@ public static function forInvalidReferenceSet($value)
      * @param mixed $value
      * @return InvalidSpanOption
      */
-    public static function forCloseSpanOnFinish($value)
+    public static function forFinishSpanOnClose($value)
     {
         return new self(sprintf(
-            'Invalid type for close_span_on_finish. Expected bool, got %s',
+            'Invalid type for finish_span_on_close. Expected bool, got %s',
             is_object($value) ? get_class($value) : gettype($value)
         ));
     }

src/OpenTracing/Mock/MockScope.php

@@ -17,17 +17,35 @@ final class MockScope implements Scope
      */
     private $scopeManager;
 
-    public function __construct(MockScopeManager $scopeManager, Span $span)
-    {
+    /**
+     * @var bool
+     */
+    private $finishSpanOnClose;
+
+    /**
+     * @param MockScopeManager $scopeManager
+     * @param Span $span
+     * @param bool $finishSpanOnClose
+     */
+    public function __construct(
+        MockScopeManager $scopeManager,
+        Span $span,
+        $finishSpanOnClose
+    ) {
         $this->scopeManager = $scopeManager;
         $this->span = $span;
+        $this->finishSpanOnClose = $finishSpanOnClose;
     }
 
     /**
      * {@inheritdoc}
      */
     public function close()
     {
+        if ($this->finishSpanOnClose) {
+            $this->span->finish();
+        }
+
         $this->scopeManager->deactivate($this);
     }
 

src/OpenTracing/Mock/MockScopeManager.php

@@ -16,9 +16,11 @@ final class MockScopeManager implements ScopeManager
     /**
      * {@inheritdoc}
      */
-    public function activate(Span $span)
+    public function activate(Span $span, $finishSpanOnClose = true)
     {
-        $this->scopes[] = new MockScope($this, $span);
+        $scope = new MockScope($this, $span, $finishSpanOnClose);
+        $this->scopes[] = $scope;
+        return $scope;
     }
 
     /**
@@ -33,23 +35,6 @@ public function getActive()
         return $this->scopes[count($this->scopes) - 1];
     }
 
-    /**
-     * {@inheritdoc}
-     * @param Span|MockSpan $span
-     */
-    public function getScope(Span $span)
-    {
-        $scopeLength = count($this->scopes);
-
-        for ($i = 0; $i < $scopeLength; $i++) {
-            if ($span === $this->scopes[$i]->getSpan()) {
-                return $this->scopes[$i];
-            }
-        }
-
-        return null;
-    }
-
     public function deactivate(MockScope $scope)
     {
         $scopeLength = count($this->scopes);

src/OpenTracing/Mock/MockSpan.php

@@ -38,28 +38,14 @@ final class MockSpan implements Span
      */
     private $duration;
 
-    /**
-     * @var bool
-     */
-    private $closeOnFinish;
-
-    /**
-     * @var ScopeManager
-     */
-    private $scopeManager;
-
     public function __construct(
-        ScopeManager $scopeManager,
         $operationName,
         MockSpanContext $context,
-        $startTime = null,
-        $closeOnFinish = false
+        $startTime = null
     ) {
-        $this->scopeManager = $scopeManager;
         $this->operationName = $operationName;
         $this->context = $context;
         $this->startTime = $startTime ?: time();
-        $this->closeOnFinish = $closeOnFinish;
     }
 
     /**
@@ -91,14 +77,6 @@ public function finish($finishTime = null)
     {
         $finishTime = ($finishTime ?: time());
         $this->duration = $finishTime - $this->startTime;
-
-        if (!$this->closeOnFinish) {
-            return;
-        }
-
-        if (($scope = $this->scopeManager->getScope($this)) !== null) {
-            $scope->close();
-        }
     }
 
     public function isFinished()

src/OpenTracing/Mock/MockTracer.php

@@ -4,7 +4,7 @@
 
 use OpenTracing\Exceptions\UnsupportedFormat;
 use OpenTracing\ScopeManager;
-use OpenTracing\SpanOptions;
+use OpenTracing\StartSpanOptions;
 use OpenTracing\Tracer;
 use OpenTracing\SpanContext;
 
@@ -42,8 +42,8 @@ public function __construct(array $injectors = [], array $extractors = [])
      */
     public function startActiveSpan($operationName, $options = [])
     {
-        if (!($options instanceof SpanOptions)) {
-            $options = SpanOptions::create($options);
+        if (!($options instanceof StartSpanOptions)) {
+            $options = StartSpanOptions::create($options);
         }
 
         if (($activeSpan = $this->getActiveSpan()) !== null) {
@@ -52,18 +52,16 @@ public function startActiveSpan($operationName, $options = [])
 
         $span = $this->startSpan($operationName, $options);
 
-        $this->scopeManager->activate($span);
-
-        return $span;
+        return $this->scopeManager->activate($span, $options->shouldFinishSpanOnClose());
     }
 
     /**
      * {@inheritdoc}
      */
     public function startSpan($operationName, $options = [])
     {
-        if (!($options instanceof SpanOptions)) {
-            $options = SpanOptions::create($options);
+        if (!($options instanceof StartSpanOptions)) {
+            $options = StartSpanOptions::create($options);
         }
 
         if (empty($options->getReferences())) {
@@ -73,7 +71,6 @@ public function startSpan($operationName, $options = [])
         }
 
         $span = new MockSpan(
-            $this->scopeManager,
             $operationName,
             $spanContext,
             $options->getStartTime()

src/OpenTracing/NoopScopeManager.php

@@ -12,7 +12,7 @@ public static function create()
     /**
      * {@inheritdoc}
      */
-    public function activate(Span $span)
+    public function activate(Span $span, $finishSpanOnClose)
     {
     }
 
@@ -23,12 +23,4 @@ public function getActive()
     {
         return NoopScope::create();
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    public function getScope(Span $span)
-    {
-        return NoopScope::create();
-    }
 }

src/OpenTracing/NoopTracer.php

@@ -38,7 +38,7 @@ public function startSpan($operationName, $options = [])
     public function startActiveSpan($operationName, $finishSpanOnClose = true, $options = [])
     {
 
-        return NoopSpan::create();
+        return NoopScope::create();
     }
 
     /**

src/OpenTracing/ScopeManager.php

@@ -13,14 +13,12 @@ interface ScopeManager
      * that previous spans can be resumed after a deactivation.
      *
      * @param Span $span the {@link Span} that should become the {@link #active()}
+     * @param bool $finishSpanOnClose whether span should automatically be finished when {@link Scope#close()} is called
      *
-     * Weather the span automatically closes when finish is called depends on
-     * the span options set during the creation of the span. See
-     * {@link SpanOptions#closeSpanOnFinish}
-     *
-     * @return Scope
+     * @return Scope instance to control the end of the active period for the {@link Span}. It is a
+     * programming error to neglect to call {@link Scope#close()} on the returned instance.
      */
-    public function activate(Span $span);
+    public function activate(Span $span, $finishSpanOnClose);
 
     /**
      * Return the currently active {@link Scope} which can be used to access the
@@ -31,16 +29,7 @@ public function activate(Span $span);
      * newly-created {@link Span} at {@link Tracer.SpanBuilder#startActive(boolean)} or {@link SpanBuilder#start()}
      * time rather than at {@link Tracer#buildSpan(String)} time.
      *
-     * @return \OpenTracing\Scope|null
+     * @return Scope|null
      */
     public function getActive();
-
-    /**
-     * Access the scope of a given Span if available.
-     *
-     * @param Span $span
-     *
-     * @return \OpenTracing\Scope|null
-     */
-    public function getScope(Span $span);
 }