Improves the stability of the namer.

README.md

@@ -141,24 +141,24 @@ services:
       - { name: kernel.event_listener, event: kernel.exception }
 ```
 
-## Span naming
+## Span namers
 
 By default the span name is being defined by the HTTP verb. This approach is
-a not so bad option seeking for low cardinality in span naming. A more useful
+a not so bad option seeking for low cardinality in span namer. A more useful
 approach is to use the route path: `/user/{user_id}` however including the 
 `@router` in the middleware is an expensive operation thus the best is to 
 precompile a map of `name => path` in cache that can be used to resolve the 
 path in runtime.
 
 ```yaml
-  zipkin.span_naming.route:
-    class: ZipkinBundle\SpanNaming\Route\Naming
-    factory [ZipkinBundle\SpanNaming\Route\Naming, 'create']
+  zipkin.span_namer.route:
+    class: ZipkinBundle\SpanNamers\Route\SpanNamer
+    factory: [ZipkinBundle\SpanNamers\Route\SpanNamer, 'create']
     arguments:
       - "%kernel.cache_dir%"
 
-  zipkin.span_naming.cache_warmer:
-    class: ZipkinBundle\SpanNaming\Route\CacheWarmer
+  zipkin.span_namer.cache_warmer:
+    class: ZipkinBundle\SpanNamers\Route\CacheWarmer
     arguments:
       - "@router"
     tags:
@@ -170,9 +170,9 @@ services:
     arguments:
       - "@zipkin.default_tracing"
       - "@logger"
-      - "@zipkin.span_naming.route"
+      - "@zipkin.span_namer.route"
     tags:
-      - { name: kernel.event_listener, event: kernel.controller }
+      - { name: kernel.event_listener, event: kernel.request, priority: 256 }
       - { name: kernel.event_listener, event: kernel.terminate }
       - { name: kernel.event_listener, event: kernel.exception }
 ```

src/ZipkinBundle/DependencyInjection/Configuration.php

@@ -16,12 +16,12 @@ public function getConfigTreeBuilder()
             ->children()
                 ->booleanNode('noop')->defaultFalse()->end()
                 ->scalarNode('service_name')->defaultNull()->end()
-                ->arrayNode('sampler')
+                ->arrayNode('sampler')->addDefaultsIfNotSet()
                     ->children()
                         ->scalarNode('type')
-                            ->defaultValue('always')
+                            ->defaultValue('never')
                         ->end()
-                        ->arrayNode('route')
+                        ->arrayNode('route')->addDefaultsIfNotSet()
                             ->canBeDisabled()
                             ->children()
                                 ->arrayNode('included_routes')
@@ -32,7 +32,7 @@ public function getConfigTreeBuilder()
                                 ->end()
                             ->end()
                         ->end()
-                        ->arrayNode('path')
+                        ->arrayNode('path')->addDefaultsIfNotSet()
                             ->canBeDisabled()
                             ->children()
                                 ->arrayNode('included_paths')->defaultValue([])
@@ -46,10 +46,10 @@ public function getConfigTreeBuilder()
                         ->scalarNode('percentage')->defaultValue(0.1)->end()
                     ->end()
                 ->end()
-                ->arrayNode('reporter')
+                ->arrayNode('reporter')->addDefaultsIfNotSet()
                     ->children()
-                        ->scalarNode('type')->defaultValue('http')->end()
-                        ->arrayNode('http')
+                        ->scalarNode('type')->defaultValue('log')->end()
+                        ->arrayNode('http')->addDefaultsIfNotSet()
                             ->children()
                                 ->scalarNode('endpoint_url')->defaultValue('http://zipkin:9411/api/v2/spans')->end()
                             ->end()

src/ZipkinBundle/DependencyInjection/ZipkinExtension.php

@@ -52,7 +52,6 @@ public function load(array $configs, ContainerBuilder $container)
             $config['sampler']['path']['included_paths']
         );
 
-
         $container->setParameter(
             'zipkin.sampler.path.excluded_paths',
             $config['sampler']['path']['excluded_paths']

src/ZipkinBundle/Middleware.php

@@ -16,8 +16,8 @@
 use Zipkin\Tags;
 use Zipkin\Tracer;
 use Zipkin\Tracing;
-use ZipkinBundle\SpanNaming\DefaultNaming;
-use ZipkinBundle\SpanNaming\SpanNamingInterface;
+use ZipkinBundle\SpanNamers\DefaultNamer;
+use ZipkinBundle\SpanNamers\SpanNamerInterface;
 
 final class Middleware
 {
@@ -37,18 +37,18 @@ final class Middleware
     private $scopeCloser;
 
     /**
-     * @var SpanNamingInterface
+     * @var SpanNamerInterface
      */
-    private $spanNaming;
+    private $spanNamer;
 
     public function __construct(
         Tracing $tracing,
         LoggerInterface $logger,
-        SpanNamingInterface $spanNamer = null
+        SpanNamerInterface $spanNamer = null
     ) {
         $this->tracing = $tracing;
         $this->logger = $logger;
-        $this->spanNaming = $spanNamer ?: DefaultNaming::create();
+        $this->spanNamer = $spanNamer ?: DefaultNamer::create();
     }
 
     public function onKernelRequest(GetResponseEvent $event)
@@ -69,7 +69,6 @@ public function onKernelRequest(GetResponseEvent $event)
 
         $span = $this->tracing->getTracer()->nextSpan($spanContext);
         $span->start();
-        $span->setName($this->spanNaming->getName($request));
         $span->setKind(Kind\SERVER);
         $span->tag(Tags\HTTP_HOST, $request->getHost());
         $span->tag(Tags\HTTP_METHOD, $request->getMethod());
@@ -90,14 +89,16 @@ public function onKernelException(GetResponseForExceptionEvent $event)
     public function onKernelTerminate(PostResponseEvent $event)
     {
         $span = $this->tracing->getTracer()->getCurrentSpan();
-        $request = $event->getRequest();
-
-        $routeName = $request->attributes->get('_route');
-        if ($routeName) {
-            $span->tag('symfony.route', $routeName);
-        }
 
         if ($span !== null) {
+            $request = $event->getRequest();
+
+            $routeName = $request->attributes->get('_route');
+            if ($routeName) {
+                $span->tag('symfony.route', $routeName);
+            }
+
+            $span->setName($this->spanNamer->getName($request));
             $span->tag(Tags\HTTP_STATUS_CODE, $event->getResponse()->getStatusCode());
             $span->finish();
             call_user_func($this->scopeCloser);

src/ZipkinBundle/SpanNaming/DefaultNaming.php → src/ZipkinBundle/SpanNamers/DefaultNamer.php

@@ -1,10 +1,10 @@
 <?php
 
-namespace ZipkinBundle\SpanNaming;
+namespace ZipkinBundle\SpanNamers;
 
 use Symfony\Component\HttpFoundation\Request;
 
-final class DefaultNaming implements SpanNamingInterface
+final class DefaultNamer implements SpanNamerInterface
 {
     public static function create()
     {

src/ZipkinBundle/SpanNaming/Route/CacheWarmer.php → src/ZipkinBundle/SpanNamers/Route/CacheWarmer.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace ZipkinBundle\SpanNaming\Route;
+namespace ZipkinBundle\SpanNamers\Route;
 
 use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
 use Symfony\Component\Routing\RouterInterface;

src/ZipkinBundle/SpanNaming/Route/Naming.php → src/ZipkinBundle/SpanNamers/Route/SpanNamer.php

@@ -1,12 +1,14 @@
 <?php
 
-namespace ZipkinBundle\SpanNaming\Route;
+namespace ZipkinBundle\SpanNamers\Route;
 
 use Symfony\Component\HttpFoundation\Request;
-use ZipkinBundle\SpanNaming\SpanNamingInterface;
+use ZipkinBundle\SpanNamers\SpanNamerInterface;
 
-final class Naming implements SpanNamingInterface
+final class SpanNamer implements SpanNamerInterface
 {
+    const NOT_FOUND = 'not_found';
+
     /**
      * @var array
      */
@@ -28,12 +30,17 @@ public static function create($cacheDir)
      */
     public function getName(Request $request)
     {
-        $routeName = $request->request->get('_route');
+        $method = $request->getMethod();
+        $routeName = $request->attributes->get('_route');
+
+        if ($routeName === null) {
+            return $method . ' ' . self::NOT_FOUND;
+        }
 
         if (array_key_exists($routeName, $this->routes)) {
-            return $request->getMethod() . ' ' . $this->routes[$routeName];
+            return $method . ' ' . $this->routes[$routeName];
         }
 
-        return $request->getMethod();
+        return $method;
     }
 }

src/ZipkinBundle/SpanNaming/SpanNamingInterface.php → src/ZipkinBundle/SpanNamers/SpanNamerInterface.php

@@ -1,10 +1,10 @@
 <?php
 
-namespace ZipkinBundle\SpanNaming;
+namespace ZipkinBundle\SpanNamers;
 
 use Symfony\Component\HttpFoundation\Request;
 
-interface SpanNamingInterface
+interface SpanNamerInterface
 {
     /**
      * @param Request $request

src/ZipkinBundle/TracingFactory.php

@@ -58,7 +58,7 @@ private static function buildReporter(ContainerInterface $container)
                 return new Log($logger);
                 break;
             case 'http':
-                return new Http(null, $logger, $container->getParameter('zipkin.reporter.http'));
+                return new Http(null, $container->getParameter('zipkin.reporter.http'));
         }
     }
 

tests/Unit/MiddlewareTest.php

@@ -2,10 +2,14 @@
 
 namespace ZipkinBundle\Tests\Unit;
 
+use Exception;
 use PHPUnit_Framework_TestCase;
 use Psr\Log\NullLogger;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
+use Symfony\Component\HttpKernel\Event\PostResponseEvent;
 use Zipkin\Span;
 use Zipkin\TracingBuilder;
 use ZipkinBundle\Middleware;
@@ -27,7 +31,7 @@ public function testSpanIsNotCreatedOnNonMasterRequest()
         $this->assertNull($tracing->getTracer()->getCurrentSpan());
     }
 
-    public function testSpanIsCreated()
+    public function testSpanIsCreatedOnKernelRequest()
     {
         $tracing = TracingBuilder::create()->build();
         $logger = new NullLogger();
@@ -42,4 +46,80 @@ public function testSpanIsCreated()
 
         $this->assertInstanceOf(Span::class, $tracing->getTracer()->getCurrentSpan());
     }
+
+    public function testNoSpanIsTaggedOnKernelExceptionIfItIsNotStarted()
+    {
+        $tracing = TracingBuilder::create()->build();
+        $logger = new NullLogger();
+
+        $middleware = new Middleware($tracing, $logger);
+
+        $event = $this->prophesize(GetResponseEvent::class);
+        $event->isMasterRequest()->willReturn(false);
+        $event->getRequest()->willReturn(new Request());
+
+        $middleware->onKernelRequest($event->reveal());
+
+        $exceptionEvent = $this->prophesize(GetResponseForExceptionEvent::class);
+        $exceptionEvent->getException()->shouldNotBeCalled();
+        $middleware->onKernelException($exceptionEvent->reveal());
+    }
+
+    public function testSpanIsTaggedOnKernelException()
+    {
+        $tracing = TracingBuilder::create()->build();
+        $logger = new NullLogger();
+
+        $middleware = new Middleware($tracing, $logger);
+
+        $event = $this->prophesize(GetResponseEvent::class);
+        $event->isMasterRequest()->willReturn(true);
+        $event->getRequest()->willReturn(new Request());
+
+        $middleware->onKernelRequest($event->reveal());
+
+        $exceptionEvent = $this->prophesize(GetResponseForExceptionEvent::class);
+        $exceptionEvent->getException()->shouldBeCalled()->willReturn(new Exception());
+        $middleware->onKernelException($exceptionEvent->reveal());
+    }
+
+    public function testNoSpanIsTaggedOnKernelTerminateIfItIsNotStarted()
+    {
+        $tracing = TracingBuilder::create()->build();
+        $logger = new NullLogger();
+
+        $middleware = new Middleware($tracing, $logger);
+
+        $event = $this->prophesize(GetResponseEvent::class);
+        $event->isMasterRequest()->willReturn(false);
+        $event->getRequest()->willReturn(new Request());
+
+        $middleware->onKernelRequest($event->reveal());
+
+        $responseEvent = $this->prophesize(PostResponseEvent::class);
+        $responseEvent->getRequest()->shouldNotBeCalled();
+        $middleware->onKernelTerminate($responseEvent->reveal());
+    }
+
+    public function testSpanIsTaggedOnKernelTerminate()
+    {
+        $tracing = TracingBuilder::create()->build();
+        $logger = new NullLogger();
+
+        $middleware = new Middleware($tracing, $logger);
+
+        $request = new Request();
+        $event = $this->prophesize(GetResponseEvent::class);
+        $event->isMasterRequest()->willReturn(true);
+        $event->getRequest()->willReturn($request);
+
+        $middleware->onKernelRequest($event->reveal());
+
+        $exceptionEvent = $this->prophesize(PostResponseEvent::class);
+        $exceptionEvent->getRequest()->shouldBeCalled()->willReturn($request);
+        $exceptionEvent->getResponse()->shouldBeCalled()->willReturn(new Response);
+        $middleware->onKernelTerminate($exceptionEvent->reveal());
+
+        $this->assertNull($tracing->getTracer()->getCurrentSpan());
+    }
 }

tests/Unit/SpanNaming/Route/CacheWarmerTest.php → tests/Unit/SpanNamers/Route/CacheWarmerTest.php

@@ -1,13 +1,13 @@
 <?php
 
-namespace ZipkinBundle\Tests\Unit\SpanNaming\Route;
+namespace ZipkinBundle\Tests\Unit\SpanNamers\Route;
 
 use PHPUnit_Framework_TestCase;
 use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;
 use Symfony\Component\Routing\Router;
-use ZipkinBundle\SpanNaming\Route\CacheWarmer;
+use ZipkinBundle\SpanNamers\Route\CacheWarmer;
 
 final class CacheWarmerTest extends PHPUnit_Framework_TestCase
 {