From 06529d7e85831f7fe996ffc9156ebc4e2babecaf Mon Sep 17 00:00:00 2001
From: Cedric Ziel <cedric@cedric-ziel.com>
Date: Sat, 8 Apr 2023 00:17:55 +0200
Subject: [PATCH] Add conditional traceresponse propagation to Symfony
 auto-instrumentation

Add composer suggest

dont mention x-ray

CS Fix: Fix import order
---
 src/Instrumentation/Symfony/composer.json     | 11 ++++-
 ...gator.php => RequestPropagationGetter.php} |  2 +-
 .../Symfony/src/ResponsePropagationSetter.php | 37 ++++++++++++++
 .../Symfony/src/SymfonyInstrumentation.php    | 36 +++++++++-----
 .../SymfonyInstrumentationTest.php            | 49 ++++++++++++++++---
 5 files changed, 114 insertions(+), 21 deletions(-)
 rename src/Instrumentation/Symfony/src/{HeadersPropagator.php => RequestPropagationGetter.php} (91%)
 create mode 100644 src/Instrumentation/Symfony/src/ResponsePropagationSetter.php

diff --git a/src/Instrumentation/Symfony/composer.json b/src/Instrumentation/Symfony/composer.json
index f6be21a5..fd5163f2 100644
--- a/src/Instrumentation/Symfony/composer.json
+++ b/src/Instrumentation/Symfony/composer.json
@@ -14,6 +14,9 @@
     "symfony/http-kernel": "*",
     "symfony/http-client-contracts": "*"
   },
+  "suggest": {
+    "open-telemetry/opentelemetry-propagation-traceresponse": "Automatically propagate the context to the client."
+  },
   "require-dev": {
     "friendsofphp/php-cs-fixer": "^3",
     "phan/phan": "^5.0",
@@ -24,7 +27,8 @@
     "open-telemetry/sdk": "^1.0",
     "phpunit/phpunit": "^9.5",
     "vimeo/psalm": "^4.0",
-    "symfony/http-client": "^5.4||^6.0"
+    "symfony/http-client": "^5.4||^6.0",
+    "open-telemetry/opentelemetry-propagation-traceresponse": "*"
   },
   "autoload": {
     "psr-4": {
@@ -38,5 +42,10 @@
     "psr-4": {
       "OpenTelemetry\\Tests\\Instrumentation\\Symfony\\": "tests/"
     }
+  },
+  "config": {
+    "allow-plugins": {
+      "php-http/discovery": false
+    }
   }
 }
diff --git a/src/Instrumentation/Symfony/src/HeadersPropagator.php b/src/Instrumentation/Symfony/src/RequestPropagationGetter.php
similarity index 91%
rename from src/Instrumentation/Symfony/src/HeadersPropagator.php
rename to src/Instrumentation/Symfony/src/RequestPropagationGetter.php
index b4fde483..2b4e48ad 100644
--- a/src/Instrumentation/Symfony/src/HeadersPropagator.php
+++ b/src/Instrumentation/Symfony/src/RequestPropagationGetter.php
@@ -11,7 +11,7 @@
 /**
  * @internal
  */
-final class HeadersPropagator implements PropagationGetterInterface
+final class RequestPropagationGetter implements PropagationGetterInterface
 {
     public static function instance(): self
     {
diff --git a/src/Instrumentation/Symfony/src/ResponsePropagationSetter.php b/src/Instrumentation/Symfony/src/ResponsePropagationSetter.php
new file mode 100644
index 00000000..f233c1b1
--- /dev/null
+++ b/src/Instrumentation/Symfony/src/ResponsePropagationSetter.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Contrib\Instrumentation\Symfony;
+
+use function assert;
+use OpenTelemetry\Context\Propagation\PropagationSetterInterface;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * @internal
+ */
+final class ResponsePropagationSetter implements PropagationSetterInterface
+{
+    public static function instance(): self
+    {
+        static $instance;
+
+        return $instance ??= new self();
+    }
+
+    /** @psalm-suppress InvalidReturnType */
+    public function keys($carrier): array
+    {
+        assert($carrier instanceof Response);
+        /** @psalm-suppress InvalidReturnStatement */
+        return $carrier->headers->keys();
+    }
+
+    public function set(&$carrier, string $key, string $value): void
+    {
+        assert($carrier instanceof Response);
+
+        $carrier->headers->set($key, $value);
+    }
+}
diff --git a/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php b/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php
index 23403582..1a15cb27 100644
--- a/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php
+++ b/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php
@@ -49,7 +49,7 @@ public static function register(): void
 
                 $parent = Context::getCurrent();
                 if ($request) {
-                    $parent = Globals::propagator()->extract($request, HeadersPropagator::instance());
+                    $parent = Globals::propagator()->extract($request, RequestPropagationGetter::instance());
                     $span = $builder
                         ->setParent($parent)
                         ->setAttribute(TraceAttributes::HTTP_URL, $request->getUri())
@@ -94,19 +94,29 @@ public static function register(): void
                     $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage());
                 }
 
-                if ($response) {
-                    if ($response->getStatusCode() >= Response::HTTP_BAD_REQUEST) {
-                        $span->setStatus(StatusCode::STATUS_ERROR);
-                    }
-                    $span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode());
-                    $span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion());
-                    $contentLength = $response->headers->get('Content-Length');
-                    /** @psalm-suppress PossiblyFalseArgument */
-                    if (null === $contentLength && is_string($response->getContent())) {
-                        $contentLength = \strlen($response->getContent());
-                    }
+                if (null === $response) {
+                    $span->end();
+
+                    return;
+                }
+
+                if ($response->getStatusCode() >= Response::HTTP_BAD_REQUEST) {
+                    $span->setStatus(StatusCode::STATUS_ERROR);
+                }
+                $span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode());
+                $span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion());
+                $contentLength = $response->headers->get('Content-Length');
+                /** @psalm-suppress PossiblyFalseArgument */
+                if (null === $contentLength && is_string($response->getContent())) {
+                    $contentLength = \strlen($response->getContent());
+                }
+
+                $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $contentLength);
 
-                    $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $contentLength);
+                // Propagate traceresponse header to response, if TraceResponsePropagator is present
+                if (class_exists('OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator')) {
+                    $prop = new \OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator();
+                    $prop->inject($response, ResponsePropagationSetter::instance(), $scope->context());
                 }
 
                 $span->end();
diff --git a/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php b/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php
index ac0503b2..a6d4eeda 100644
--- a/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php
+++ b/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php
@@ -4,6 +4,7 @@
 
 namespace OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration;
 
+use OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator;
 use OpenTelemetry\SemConv\TraceAttributes;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -27,7 +28,13 @@ public function test_http_kernel_handle_exception(): void
         });
         $this->assertCount(0, $this->storage);
 
-        $kernel->handle(new Request());
+        $response = $kernel->handle(new Request());
+
+        $this->assertArrayHasKey(
+            TraceResponsePropagator::TRACERESPONSE,
+            $response->headers->all(),
+            'traceresponse header is present if TraceResponsePropagator is present'
+        );
     }
 
     public function test_http_kernel_handle_attributes(): void
@@ -37,7 +44,7 @@ public function test_http_kernel_handle_attributes(): void
         $request = new Request();
         $request->attributes->set('_route', 'test_route');
 
-        $kernel->handle($request);
+        $response = $kernel->handle($request);
 
         $attributes = $this->storage[0]->getAttributes();
         $this->assertCount(1, $this->storage);
@@ -49,6 +56,12 @@ public function test_http_kernel_handle_attributes(): void
         $this->assertEquals(200, $attributes->get(TraceAttributes::HTTP_STATUS_CODE));
         $this->assertEquals('1.0', $attributes->get(TraceAttributes::HTTP_FLAVOR));
         $this->assertEquals(5, $attributes->get(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH));
+
+        $this->assertArrayHasKey(
+            TraceResponsePropagator::TRACERESPONSE,
+            $response->headers->all(),
+            'traceresponse header is present if TraceResponsePropagator is present'
+        );
     }
 
     public function test_http_kernel_handle_stream_response(): void
@@ -59,9 +72,15 @@ public function test_http_kernel_handle_stream_response(): void
         }));
         $this->assertCount(0, $this->storage);
 
-        $kernel->handle(new Request());
+        $response = $kernel->handle(new Request());
         $this->assertCount(1, $this->storage);
         $this->assertNull($this->storage[0]->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH));
+
+        $this->assertArrayHasKey(
+            TraceResponsePropagator::TRACERESPONSE,
+            $response->headers->all(),
+            'traceresponse header is present if TraceResponsePropagator is present'
+        );
     }
 
     public function test_http_kernel_handle_binary_file_response(): void
@@ -69,9 +88,15 @@ public function test_http_kernel_handle_binary_file_response(): void
         $kernel = $this->getHttpKernel(new EventDispatcher(), fn () => new BinaryFileResponse(__FILE__));
         $this->assertCount(0, $this->storage);
 
-        $kernel->handle(new Request());
+        $response = $kernel->handle(new Request());
         $this->assertCount(1, $this->storage);
         $this->assertNull($this->storage[0]->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH));
+
+        $this->assertArrayHasKey(
+            TraceResponsePropagator::TRACERESPONSE,
+            $response->headers->all(),
+            'traceresponse header is present if TraceResponsePropagator is present'
+        );
     }
 
     public function test_http_kernel_handle_with_empty_route(): void
@@ -81,9 +106,15 @@ public function test_http_kernel_handle_with_empty_route(): void
         $request = new Request();
         $request->attributes->set('_route', '');
 
-        $kernel->handle($request, HttpKernelInterface::MAIN_REQUEST, true);
+        $response = $kernel->handle($request, HttpKernelInterface::MAIN_REQUEST, true);
         $this->assertCount(1, $this->storage);
         $this->assertFalse($this->storage[0]->getAttributes()->has(TraceAttributes::HTTP_ROUTE));
+
+        $this->assertArrayHasKey(
+            TraceResponsePropagator::TRACERESPONSE,
+            $response->headers->all(),
+            'traceresponse header is present if TraceResponsePropagator is present'
+        );
     }
 
     public function test_http_kernel_handle_without_route(): void
@@ -91,9 +122,15 @@ public function test_http_kernel_handle_without_route(): void
         $kernel = $this->getHttpKernel(new EventDispatcher());
         $this->assertCount(0, $this->storage);
 
-        $kernel->handle(new Request(), HttpKernelInterface::MAIN_REQUEST, true);
+        $response = $kernel->handle(new Request(), HttpKernelInterface::MAIN_REQUEST, true);
         $this->assertCount(1, $this->storage);
         $this->assertFalse($this->storage[0]->getAttributes()->has(TraceAttributes::HTTP_ROUTE));
+
+        $this->assertArrayHasKey(
+            TraceResponsePropagator::TRACERESPONSE,
+            $response->headers->all(),
+            'traceresponse header is present if TraceResponsePropagator is present'
+        );
     }
 
     private function getHttpKernel(EventDispatcherInterface $eventDispatcher, $controller = null, RequestStack $requestStack = null, array $arguments = []): HttpKernel