Add the traceresponse header in tracecontext mode (#23)

123inkt · Jan 9, 2024 · daa863b · daa863b
1 parent 94f2c83
commit daa863b
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 15 deletions.
diff --git a/src/Generator/TraceContext/TraceContextIdGenerator.php b/src/Generator/TraceContext/TraceContextIdGenerator.php
@@ -13,13 +13,20 @@
  */
 class TraceContextIdGenerator implements TraceIdGeneratorInterface
 {
+    /**
+     * This is the ID of this request as known by the caller
+     * (in some tracing systems, this is known as the span-id, where a span is the execution of a client request).
+     */
     public function generateTransactionId(): string
     {
-        return substr(bin2hex(random_bytes(8)), 8);
+        return substr(bin2hex(random_bytes(16)), 16);
     }
 
+    /**
+     * This is the ID of the whole trace forest and is used to uniquely identify a distributed trace through a system.
+     */
     public function generateTraceId(): string
     {
-        return substr(bin2hex(random_bytes(16)), 16);
+        return substr(bin2hex(random_bytes(32)), 32);
     }
 }
diff --git a/src/Service/TraceContext/TraceContextService.php b/src/Service/TraceContext/TraceContextService.php
@@ -15,8 +15,9 @@
  */
 class TraceContextService implements TraceServiceInterface
 {
-    public const HEADER_TRACEPARENT = 'traceparent';
-    public const HEADER_TRACESTATE  = 'tracestate';
+    public const HEADER_TRACEPARENT   = 'traceparent';
+    public const HEADER_TRACESTATE    = 'tracestate';
+    public const HEADER_TRACERESPONSE = 'traceresponse';
 
     public function __construct(private readonly TraceContextIdGenerator $generator)
     {
@@ -58,7 +59,9 @@ public function getRequestTrace(Request $request): TraceContext
      */
     public function handleResponse(Response $response, TraceContext $context): void
     {
-        // Do nothing
+        if ($context->getTraceId() !== null) {
+            $response->headers->set(self::HEADER_TRACERESPONSE, $this->renderTraceParent($context));
+        }
     }
 
     public function handleClientRequest(TraceContext $trace, string $method, string $url, array $options = []): array

diff --git a/tests/Functional/App/config/tracecontext.yml b/tests/Functional/App/config/tracecontext.yml
@@ -1,8 +1,5 @@
 symfony_trace:
   traceMode: 'tracecontext'
-  traceid:
-    request_header: Trace-Id
-    response_header: Trace-Id
   trust_request_header: true
   storage_service: request.id.storage
   enable_messenger: true

diff --git a/tests/Functional/RequestHandleTest.php b/tests/Functional/RequestHandleTest.php
@@ -67,7 +67,16 @@ public function testRequestThatAlreadyHasATraceContextDoesNotReplaceIt(): void
         $crawler = $client->request('GET', '/', [], [], ['HTTP_TRACEPARENT' => '00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-00']);
         static::assertResponseIsSuccessful();
 
-        static::assertSame('0af7651916cd43dd8448eb211c80319c', self::getService(TraceStorageInterface::class)->getTraceId());
+        $trace = self::getService(TraceStorageInterface::class)->getTrace();
+        static::assertSame('0af7651916cd43dd8448eb211c80319c', $trace->getTraceId());
+        static::assertSame('b7ad6b7169203331', $trace->getParentTransactionId());
+        static::assertNotSame('b7ad6b7169203331', $trace->getTransactionId());
+
+        $response = $client->getResponse();
+        static::assertSame(
+            '00-' . $trace->getTraceId() . '-' . $trace->getTransactionId() . '-00',
+            $response->headers->get('traceresponse')
+        );
         self::assertLogsHaveTraceId('0af7651916cd43dd8448eb211c80319c');
         static::assertGreaterThan(
             0,
@@ -86,12 +95,19 @@ public function testRequestWithOutTraceContextCreatesOneAndPassesThroughTheRespo
         $crawler = $client->request('GET', '/');
         static::assertResponseIsSuccessful();
 
-        $id = self::getService(TraceStorageInterface::class)->getTraceId();
-        static::assertNotEmpty($id);
-        self::assertLogsHaveTraceId($id);
+        $traceStorage = self::getService(TraceStorageInterface::class);
+        static::assertNotEmpty($traceStorage->getTraceId());
+        static::assertNotEmpty($traceStorage->getTransactionId());
+
+        $response = $client->getResponse();
+        static::assertSame(
+            '00-' . $traceStorage->getTraceId() . '-' . $traceStorage->getTransactionId() . '-00',
+            $response->headers->get('traceresponse')
+        );
+        self::assertLogsHaveTraceId($traceStorage->getTraceId());
         static::assertGreaterThan(
             0,
-            $crawler->filter(sprintf('h1:contains("%s")', $id))->count(),
+            $crawler->filter(sprintf('h1:contains("%s")', $traceStorage->getTraceId()))->count(),
             'should have the request ID in the response HTML'
         );
     }

diff --git a/tests/Unit/Generator/TraceContext/TraceContextIdGeneratorTest.php b/tests/Unit/Generator/TraceContext/TraceContextIdGeneratorTest.php
@@ -14,12 +14,12 @@ class TraceContextIdGeneratorTest extends TestCase
     public function testGenerateTransactionId(): void
     {
         $generator = new TraceContextIdGenerator();
-        static::assertSame(8, strlen($generator->generateTransactionId()));
+        static::assertSame(16, strlen($generator->generateTransactionId()));
     }
 
     public function testGenerateTraceId(): void
     {
         $generator = new TraceContextIdGenerator();
-        static::assertSame(16, strlen($generator->generateTraceId()));
+        static::assertSame(32, strlen($generator->generateTraceId()));
     }
 }
diff --git a/tests/Unit/Service/TraceContext/TraceContextServiceTest.php b/tests/Unit/Service/TraceContext/TraceContextServiceTest.php
@@ -13,6 +13,7 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
 
 #[CoversClass(TraceContextService::class)]
 class TraceContextServiceTest extends TestCase
@@ -69,6 +70,20 @@ public function testGetRequestTrace(): void
         static::assertSame(['foo' => 'bar', 'bar' => 'baz'], $trace->getTraceState());
     }
 
+    public function testHandleResponse(): void
+    {
+        $trace = new TraceContext();
+        $trace->setTraceId('0af7651916cd43dd8448eb211c80319c');
+        $trace->setTransactionId('b7ad6b7169203331');
+
+        $response = new Response();
+        $this->service->handleResponse($response, $trace);
+        static::assertSame(
+            '00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-00',
+            $response->headers->get(TraceContextService::HEADER_TRACERESPONSE)
+        );
+    }
+
     public function testHandleClientRequest(): void
     {
         $trace = new TraceContext();