Merge pull request #60 from jcchavezs/removes_scope

fix: reset scope in the request once it is closed.

README.md

@@ -2,10 +2,10 @@
 
 [![Build Status](https://travis-ci.org/jcchavezs/zipkin-instrumentation-symfony.svg?branch=master)](https://travis-ci.org/jcchavezs/zipkin-instrumentation-symfony)
 [![CircleCI](https://circleci.com/gh/jcchavezs/zipkin-instrumentation-symfony/tree/master.svg?style=svg)](https://circleci.com/gh/jcchavezs/zipkin-instrumentation-symfony/tree/master)
-[![Latest Stable Version](https://poser.pugx.org/jcchavezs/zipkin-symfony/v/stable)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony)
+[![Latest Stable Version](https://poser.pugx.org/jcchavezs/zipkin-instrumentation-symfony/v/stable)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony)
 [![Minimum PHP Version](https://img.shields.io/badge/php-%3E%3D%205.6-8892BF.svg)](https://php.net/)
 [![Total Downloads](https://poser.pugx.org/jcchavezs/zipkin-instrumentation-symfony/downloads)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony)
-[![License](https://poser.pugx.org/jcchavezs/zipkin-symfony/license)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony)
+[![License](https://poser.pugx.org/jcchavezs/zipkin-instrumentation-symfony/license)](https://packagist.org/packages/jcchavezs/zipkin-instrumentation-symfony)
 
 A Zipkin integration for Symfony applications
 
@@ -212,7 +212,7 @@ services:
 
 ## Contributing
 
-All contribution and feedback are welcome.
+All contributions and feedback are welcome.
 
 ### Unit testing
 
@@ -228,7 +228,7 @@ On every build we run a end to end (E2E) test against a symfony application.
 This test run in our CI tests but it can be also reproduced in local by:
 
 1. Go to `tests/Integration`
-2. Run `make SYMFONY_VERSION={{SYMFONY_VERSION}} build` to build the test application (by default newest version) 
+2. Run `SYMFONY_VERSION={{SYMFONY_VERSION}} make build` to build the test application (by default newest version)
 3. Run `make run-zipkin` to start zipkin sever
 4. Run `make run-app` to start the test application
 5. Hit the application `curl -i http://localhost:8002/_health`

src/ZipkinBundle/Middleware.php

@@ -15,11 +15,10 @@
 use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\KernelEvent;
-use Symfony\Component\HttpKernel\Event\ResponseEvent;
 
 final class Middleware
 {
-    const SPAN_CLOSER_KEY = 'zipkin_bundle_span_closer';
+    const SCOPE_CLOSER_KEY = 'zipkin_bundle_scope_closer';
 
     /**
      * @var Tracer
@@ -95,7 +94,7 @@ public function onKernelRequest(KernelEvent $event)
             $span->tag($key, $value);
         }
 
-        $request->attributes->set(self::SPAN_CLOSER_KEY, $this->tracer->openScope($span));
+        $request->attributes->set(self::SCOPE_CLOSER_KEY, $this->tracer->openScope($span));
     }
 
     /**
@@ -167,14 +166,14 @@ public function onKernelResponse(KernelEvent $event)
     public function onKernelTerminate(KernelEvent $event)
     {
         // Previously, the onKernelResponse listener did not exist in this class
-        // and hence we finished the span and closed the scope on terminate.
-        // However, terminate happens after the response have been sent and it could
-        // happen that span is being finished after some other processing attached by
-        // the user. onKernelResponse is the right place to finish the span but in order
-        // to not to break existing user relaying on the onKernelTerminate to finish
-        // the span we add this temporary fix.
-
-        $scopeCloser = $event->getRequest()->attributes->get(self::SPAN_CLOSER_KEY);
+        // and hence we finished the span and closed the scope on the onKernelTerminate.
+        // However, terminate occurs after the response has been sent and it could
+        // happen that span is being finished after some other processing (potentially
+        // expensive) attached by the user. onKernelResponse is the right place to finish
+        // the span but in order to not to break existing user relaying on the
+        // onKernelTerminate to finish the span we add this temporary fix.
+
+        $scopeCloser = $event->getRequest()->attributes->get(self::SCOPE_CLOSER_KEY);
         if ($scopeCloser !== null) {
             if ($this->usesDeprecatedEvents) {
                 /**
@@ -217,9 +216,11 @@ private function finishSpan(Request $request, $response)
         }
 
         $span->finish();
-        $scopeCloser = $request->attributes->get(self::SPAN_CLOSER_KEY);
+        $scopeCloser = $request->attributes->get(self::SCOPE_CLOSER_KEY);
         if ($scopeCloser !== null) {
             $scopeCloser();
+            // We reset the scope closer as it did its job
+            $request->attributes->remove(self::SCOPE_CLOSER_KEY);
         }
     }