feat: add trace id header (#39)

contributors: @mmross

EventListener/FinishControllerSpanSubscriber.php

@@ -4,6 +4,7 @@
 
 namespace Auxmoney\OpentracingBundle\EventListener;
 
+use Auxmoney\OpentracingBundle\Internal\TracingId;
 use Auxmoney\OpentracingBundle\Service\Tracing;
 use Psr\Log\LoggerInterface;
 use ReflectionClass;
@@ -17,11 +18,23 @@ final class FinishControllerSpanSubscriber implements EventSubscriberInterface
 {
     private $tracing;
     private $logger;
+    private $tracingId;
+    private $returnTraceId;
 
-    public function __construct(Tracing $tracing, LoggerInterface $logger)
-    {
+    public function __construct(
+        Tracing $tracing,
+        TracingId $tracingId,
+        LoggerInterface $logger,
+        string $returnTraceId
+    ) {
         $this->tracing = $tracing;
         $this->logger = $logger;
+        $this->tracingId = $tracingId;
+        $this->returnTraceId = filter_var(
+            $returnTraceId,
+            FILTER_VALIDATE_BOOLEAN,
+            FILTER_NULL_ON_FAILURE
+        ) ?? true;
     }
 
     /**
@@ -45,11 +58,11 @@ public function onResponse($event): void
         // This check ensures there was a span started on a corresponding kernel.controller event for this request
         if ($attributes->has('_auxmoney_controller')) {
             $response = $this->getResponse($event);
-            $responseStatusCode = $response ? $response->getStatusCode() : null;
-            $this->tracing->setTagOfActiveSpan(HTTP_STATUS_CODE, $responseStatusCode ?? 'not determined');
-            if ($responseStatusCode && $responseStatusCode >= 400) {
-                $this->tracing->setTagOfActiveSpan(ERROR, true);
-            }
+
+            $this->addTagsFromStatusCode($response);
+
+            $this->addTraceIdHeader($response);
+
             $this->tracing->finishActiveSpan();
         }
     }
@@ -74,4 +87,20 @@ private function getResponse($event): ?Response
 
         return $response;
     }
+
+    private function addTagsFromStatusCode(?Response $response): void
+    {
+        $responseStatusCode = $response ? $response->getStatusCode() : 'not determined';
+        $this->tracing->setTagOfActiveSpan(HTTP_STATUS_CODE, $responseStatusCode);
+        if ($responseStatusCode && $responseStatusCode >= 400) {
+            $this->tracing->setTagOfActiveSpan(ERROR, true);
+        }
+    }
+
+    private function addTraceIdHeader(?Response $response): void
+    {
+        if ($response && $this->returnTraceId) {
+            $response->headers->set('X-Auxmoney-Opentracing-Trace-Id', $this->tracingId->getAsString());
+        }
+    }
 }

README.md

@@ -87,7 +87,8 @@ If you cannot change environment variables in your project, you can alternativel
 | AUXMONEY_OPENTRACING_PROJECT_NAME | auxmoney_opentracing.project.name | `string` | `basename(kernel.project_dir)` |  passed to the tracer as tracer name / service name |
 | AUXMONEY_OPENTRACING_SAMPLER_CLASS | auxmoney_opentracing.sampler.class | `string` | (depends on the chosen tracer) | class of the using sampler |
 | AUXMONEY_OPENTRACING_SAMPLER_VALUE | auxmoney_opentracing.sampler.value | `string` | (depends on the chosen tracer and sampler) | must be a JSON decodable string, for the configuration of the chosen sampler |
-
+| AUXMONEY_OPENTRACING_RESPONSE_HEADER | auxmoney_opentracing.response.header | `string` | `true` | if HTTP responses should ship the trace id as header; set to `false` (as string) to deactivate |
+
 ## Usage
 
 ### Propagation of tracing headers

Resources/config/services.yaml

@@ -5,13 +5,15 @@ parameters:
   env(AUXMONEY_OPENTRACING_PROJECT_NAME): 'default will be provided by extension (basename(kernel.project_dir))'
   # env(AUXMONEY_OPENTRACING_SAMPLER_CLASS) will be provided by packages which provide auxmoney/opentracing-bundle-tracer-implementation
   # env(AUXMONEY_OPENTRACING_SAMPLER_VALUE) will be provided by packages which provide auxmoney/opentracing-bundle-tracer-implementation
+  env(AUXMONEY_OPENTRACING_RESPONSE_HEADER): 'true'
 
   auxmoney_opentracing.hostname: '%env(HOSTNAME)%'
   auxmoney_opentracing.agent.host: '%env(AUXMONEY_OPENTRACING_AGENT_HOST)%'
   auxmoney_opentracing.agent.port: '%env(AUXMONEY_OPENTRACING_AGENT_PORT)%'
   auxmoney_opentracing.project.name: '%env(AUXMONEY_OPENTRACING_PROJECT_NAME)%'
   auxmoney_opentracing.sampler.class: '%env(AUXMONEY_OPENTRACING_SAMPLER_CLASS)%'
   auxmoney_opentracing.sampler.value: '%env(AUXMONEY_OPENTRACING_SAMPLER_VALUE)%'
+  auxmoney_opentracing.response.header: '%env(AUXMONEY_OPENTRACING_RESPONSE_HEADER)%'
 
 
 services:
@@ -65,7 +67,9 @@ services:
   Auxmoney\OpentracingBundle\EventListener\StartRootSpanSubscriber: ~
   Auxmoney\OpentracingBundle\EventListener\FinishRootSpanSubscriber: ~
   Auxmoney\OpentracingBundle\EventListener\StartControllerSpanSubscriber: ~
-  Auxmoney\OpentracingBundle\EventListener\FinishControllerSpanSubscriber: ~
+  Auxmoney\OpentracingBundle\EventListener\FinishControllerSpanSubscriber:
+    arguments:
+      $returnTraceId: '%auxmoney_opentracing.response.header%'
   Auxmoney\OpentracingBundle\EventListener\StartCommandSpanSubscriber: ~
   Auxmoney\OpentracingBundle\EventListener\FinishCommandSpanSubscriber: ~
   Auxmoney\OpentracingBundle\EventListener\ExceptionLogSubscriber: ~

Tests/EventListener/FinishControllerSpanSubscriberTest.php

@@ -5,17 +5,19 @@
 namespace Auxmoney\OpentracingBundle\Tests\EventListener;
 
 use Auxmoney\OpentracingBundle\EventListener\FinishControllerSpanSubscriber;
+use Auxmoney\OpentracingBundle\Internal\TracingId;
+use Auxmoney\OpentracingBundle\Service\Tracing;
 use Auxmoney\OpentracingBundle\Tests\Mock\EventWithNoResponse;
 use Auxmoney\OpentracingBundle\Tests\Mock\EventWithResponse;
 use Auxmoney\OpentracingBundle\Tests\Mock\EventWithResponseAndReflectionError;
-use Auxmoney\OpentracingBundle\Service\Tracing;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\Request;
 
 class FinishControllerSpanSubscriberTest extends TestCase
 {
+    private $tracingId;
     private $logger;
     private $tracing;
     private $subject;
@@ -25,8 +27,14 @@ public function setUp()
         parent::setUp();
         $this->tracing = $this->prophesize(Tracing::class);
         $this->logger = $this->prophesize(LoggerInterface::class);
+        $this->tracingId = $this->prophesize(TracingId::class);
 
-        $this->subject = new FinishControllerSpanSubscriber($this->tracing->reveal(), $this->logger->reveal());
+        $this->subject = new FinishControllerSpanSubscriber(
+            $this->tracing->reveal(),
+            $this->tracingId->reveal(),
+            $this->logger->reveal(),
+            'true'
+        );
     }
 
     public function testGetSubscribedEvents(): void
@@ -47,6 +55,7 @@ public function testOnTerminateNoResponse(): void
         );
 
         $this->logger->error(Argument::any())->shouldNotBeCalled();
+        $this->tracingId->getAsString()->shouldNotBeCalled();
         $this->tracing->setTagOfActiveSpan('http.status_code', 'not determined')->shouldBeCalledOnce();
         $this->tracing->finishActiveSpan()->shouldBeCalledOnce();
 
@@ -58,10 +67,14 @@ public function testOnTerminateNoController(): void
         $request = new Request();
 
         $this->logger->error(Argument::any())->shouldNotBeCalled();
+        $this->tracingId->getAsString()->shouldNotBeCalled();
         $this->tracing->setTagOfActiveSpan(Argument::any(), Argument::any())->shouldNotBeCalled();
         $this->tracing->finishActiveSpan()->shouldNotBeCalled();
 
-        $this->subject->onResponse(new EventWithResponse($request));
+        $event = new EventWithResponse($request);
+        $this->subject->onResponse($event);
+
+        self::assertFalse($event->getResponse()->headers->has('X-Auxmoney-Opentracing-Trace-Id'));
     }
 
     public function testOnTerminateReflectionFailed(): void
@@ -77,12 +90,43 @@ public function testOnTerminateReflectionFailed(): void
         );
 
         $this->logger->error(Argument::any())->shouldBeCalled();
+        $this->tracingId->getAsString()->shouldNotBeCalled();
         $this->tracing->setTagOfActiveSpan('http.status_code', 'not determined')->shouldBeCalledOnce();
         $this->tracing->finishActiveSpan()->shouldBeCalledOnce();
 
         $this->subject->onResponse(new EventWithResponseAndReflectionError($request));
     }
 
+    public function testOnTerminateSuccessWithoutTraceId(): void
+    {
+        $this->subject = new FinishControllerSpanSubscriber(
+            $this->tracing->reveal(),
+            $this->tracingId->reveal(),
+            $this->logger->reveal(),
+            'false'
+        );
+
+        $request = new Request();
+        $request->attributes->add(
+            [
+                '_controller' => 'controller name',
+                '_route' => 'controller route',
+                '_route_params' => ['a route' => 'param', 'and' => 5],
+                '_auxmoney_controller' => true
+            ]
+        );
+
+        $this->logger->error(Argument::any())->shouldNotBeCalled();
+        $this->tracingId->getAsString()->shouldNotBeCalled();
+        $this->tracing->setTagOfActiveSpan('http.status_code', 200)->shouldBeCalledOnce();
+        $this->tracing->finishActiveSpan()->shouldBeCalledOnce();
+
+        $event = new EventWithResponse($request);
+        $this->subject->onResponse($event);
+
+        self::assertFalse($event->getResponse()->headers->has('X-Auxmoney-Opentracing-Trace-Id'));
+    }
+
     public function testOnTerminateSuccess(): void
     {
         $request = new Request();
@@ -96,9 +140,14 @@ public function testOnTerminateSuccess(): void
         );
 
         $this->logger->error(Argument::any())->shouldNotBeCalled();
+        $this->tracingId->getAsString()->shouldBeCalledOnce()->willReturn('tracing id');
         $this->tracing->setTagOfActiveSpan('http.status_code', 200)->shouldBeCalledOnce();
         $this->tracing->finishActiveSpan()->shouldBeCalledOnce();
 
-        $this->subject->onResponse(new EventWithResponse($request));
+        $event = new EventWithResponse($request);
+        $this->subject->onResponse($event);
+
+        self::assertTrue($event->getResponse()->headers->has('X-Auxmoney-Opentracing-Trace-Id'));
+        self::assertSame('tracing id', $event->getResponse()->headers->get('X-Auxmoney-Opentracing-Trace-Id'));
     }
 }

Tests/Functional/Scripts/requireAdditionalVendors.sh

@@ -3,6 +3,5 @@
 cd build/testproject/
 composer config extra.symfony.allow-contrib true
 composer config repositories.origin vcs https://github.com/${PR_ORIGIN}
-composer update auxmoney/core
 composer require php-http/curl-client nyholm/psr7 webmozart/assert
 cd ../../

Tests/Functional/TestProjectFiles/default/config/packages/psr18.yaml

@@ -9,3 +9,6 @@ services:
 
   Psr\Http\Client\ClientInterface:
     class: Http\Client\Curl\Client
+
+  logger:
+    class: Psr\Log\NullLogger

Tests/Mock/EventWithResponse.php

@@ -10,10 +10,12 @@
 final class EventWithResponse
 {
     private $request;
+    private $response;
 
     public function __construct(Request $request)
     {
         $this->request = $request;
+        $this->response = new Response();
     }
 
     public function getRequest(): Request
@@ -23,6 +25,6 @@ public function getRequest(): Request
 
     public function getResponse(): Response
     {
-        return new Response();
+        return $this->response;
     }
 }