From ccab8983b5227542280660b2546d5807c3a9c2e6 Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Sat, 8 Apr 2023 00:03:49 +0200 Subject: [PATCH 1/5] Add traceresponse propagation to Slim auto-instrumentation --- src/Instrumentation/Slim/composer.json | 13 ++++++++++++- .../Slim/src/SlimInstrumentation.php | 14 +++++++++++++- .../tests/Integration/SlimInstrumentationTest.php | 14 +++++++++++--- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/Instrumentation/Slim/composer.json b/src/Instrumentation/Slim/composer.json index 984bc39c..fc0577ad 100644 --- a/src/Instrumentation/Slim/composer.json +++ b/src/Instrumentation/Slim/composer.json @@ -15,6 +15,9 @@ "open-telemetry/api": "^1.0", "slim/slim": "^4" }, + "suggest": { + "open-telemetry/opentelemetry-propagation-traceresponse": "Automatically propagate the context to the client." + }, "require-dev": { "friendsofphp/php-cs-fixer": "^3", "mockery/mockery": "^1.5", @@ -27,7 +30,10 @@ "psalm/plugin-phpunit": "^0.16", "open-telemetry/sdk": "^1.0", "phpunit/phpunit": "^9.5", - "vimeo/psalm": "^4.0" + "vimeo/psalm": "^4.0", + "open-telemetry/opentelemetry-propagation-traceresponse": "*", + "symfony/http-client": "^6.0", + "guzzlehttp/promises": "^1.5" }, "autoload": { "psr-4": { @@ -41,5 +47,10 @@ "psr-4": { "OpenTelemetry\\Tests\\Instrumentation\\Slim\\": "tests/" } + }, + "config": { + "allow-plugins": { + "php-http/discovery": true + } } } diff --git a/src/Instrumentation/Slim/src/SlimInstrumentation.php b/src/Instrumentation/Slim/src/SlimInstrumentation.php index d87ede73..5a036995 100644 --- a/src/Instrumentation/Slim/src/SlimInstrumentation.php +++ b/src/Instrumentation/Slim/src/SlimInstrumentation.php @@ -11,6 +11,7 @@ use OpenTelemetry\API\Trace\SpanKind; use OpenTelemetry\API\Trace\StatusCode; use OpenTelemetry\Context\Context; +use OpenTelemetry\Context\Propagation\ArrayAccessGetterSetter; use function OpenTelemetry\Instrumentation\hook; use OpenTelemetry\SemConv\TraceAttributes; use Psr\Http\Message\ResponseInterface; @@ -61,7 +62,7 @@ public static function register(): void return [$request]; }, - post: static function (App $app, array $params, ?ResponseInterface $response, ?Throwable $exception) { + post: static function (App $app, array $params, ?ResponseInterface &$response, ?Throwable $exception) { $scope = Context::storage()->scope(); if (!$scope) { return; @@ -79,6 +80,17 @@ public static function register(): void $span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode()); $span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion()); $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $response->getHeaderLine('Content-Length')); + + // Propagate traceresponse header to response, if TraceResponsePropagator is present + if (class_exists('OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator')) { + $carrier = []; + $prop = new \OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator(); + $prop->inject($carrier, ArrayAccessGetterSetter::getInstance(), $scope->context()); + + foreach ($carrier as $name => $value) { + $response = $response->withHeader($name, $value); + } + } } $span->end(); diff --git a/src/Instrumentation/Slim/tests/Integration/SlimInstrumentationTest.php b/src/Instrumentation/Slim/tests/Integration/SlimInstrumentationTest.php index b3cc2b25..658badcf 100644 --- a/src/Instrumentation/Slim/tests/Integration/SlimInstrumentationTest.php +++ b/src/Instrumentation/Slim/tests/Integration/SlimInstrumentationTest.php @@ -10,7 +10,7 @@ use Nyholm\Psr7\ServerRequest; use OpenTelemetry\API\Common\Instrumentation\Configurator; use OpenTelemetry\Context\ScopeInterface; -use OpenTelemetry\SDK\Trace\ImmutableSpan; +use OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator; use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter; use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor; use OpenTelemetry\SDK\Trace\TracerProvider; @@ -67,10 +67,13 @@ public function performRouting(ServerRequestInterface $request): ServerRequestIn $this->createMock(ResponseInterface::class), $routingMiddleware ); - $app->handle($request->withAttribute(RouteContext::ROUTE, $route)); + $response = $app->handle($request->withAttribute(RouteContext::ROUTE, $route)); + $this->assertCount(1, $this->storage); $span = $this->storage->offsetGet(0); // @var ImmutableSpan $span $this->assertSame($expected, $span->getName()); + + $this->assertTrue($response->hasHeader(TraceResponsePropagator::TRACERESPONSE), 'traceresponse header is added'); } /** @@ -125,14 +128,19 @@ public function performRouting(ServerRequestInterface $request): ServerRequestIn $routingMiddleware ); + $response = null; + try { - $app->handle($request); + $response = $app->handle($request); } catch (\Exception $e) { $this->assertSame('routing failed', $e->getMessage()); } $this->assertCount(1, $this->storage); $span = $this->storage->offsetGet(0); // @var ImmutableSpan $span $this->assertSame('HTTP GET', $span->getName(), 'span name was not updated because routing failed'); + + /** @psalm-suppress PossiblyNullReference */ + $this->assertTrue($response->hasHeader(TraceResponsePropagator::TRACERESPONSE), 'traceresponse header is added'); } public function createMockStrategy(): InvocationStrategyInterface From 3c62dbfd588be3ad1f9eb8ccc4f9e13d8ff4bcf3 Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Wed, 26 Apr 2023 14:57:48 +0200 Subject: [PATCH 2/5] In case of traceresponse injection, dont modify reference, but return --- src/Instrumentation/Slim/src/SlimInstrumentation.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Instrumentation/Slim/src/SlimInstrumentation.php b/src/Instrumentation/Slim/src/SlimInstrumentation.php index 5a036995..ad98a238 100644 --- a/src/Instrumentation/Slim/src/SlimInstrumentation.php +++ b/src/Instrumentation/Slim/src/SlimInstrumentation.php @@ -62,7 +62,7 @@ public static function register(): void return [$request]; }, - post: static function (App $app, array $params, ?ResponseInterface &$response, ?Throwable $exception) { + post: static function (App $app, array $params, ?ResponseInterface $response, ?Throwable $exception) { $scope = Context::storage()->scope(); if (!$scope) { return; @@ -90,6 +90,10 @@ public static function register(): void foreach ($carrier as $name => $value) { $response = $response->withHeader($name, $value); } + + $span->end(); + + return $response; } } From 58e6d12a6c5a007543f375488efcae17d5d09ddf Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Wed, 26 Apr 2023 15:36:46 +0200 Subject: [PATCH 3/5] Try to suppress PhanPluginInconsistentReturnFunction --- src/Instrumentation/Slim/src/SlimInstrumentation.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Instrumentation/Slim/src/SlimInstrumentation.php b/src/Instrumentation/Slim/src/SlimInstrumentation.php index ad98a238..d7369935 100644 --- a/src/Instrumentation/Slim/src/SlimInstrumentation.php +++ b/src/Instrumentation/Slim/src/SlimInstrumentation.php @@ -27,6 +27,9 @@ class SlimInstrumentation { public const NAME = 'slim'; + /** + * @suppress PhanPluginInconsistentReturnFunction + */ public static function register(): void { $instrumentation = new CachedInstrumentation('io.opentelemetry.contrib.php.slim'); From 47403610c7d64a2df0c20c9434962d6f3c309ed0 Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Wed, 26 Apr 2023 15:39:40 +0200 Subject: [PATCH 4/5] Try to suppress PhanPluginInconsistentReturnFunction --- src/Instrumentation/Slim/src/SlimInstrumentation.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Instrumentation/Slim/src/SlimInstrumentation.php b/src/Instrumentation/Slim/src/SlimInstrumentation.php index d7369935..3c60dd2a 100644 --- a/src/Instrumentation/Slim/src/SlimInstrumentation.php +++ b/src/Instrumentation/Slim/src/SlimInstrumentation.php @@ -27,9 +27,6 @@ class SlimInstrumentation { public const NAME = 'slim'; - /** - * @suppress PhanPluginInconsistentReturnFunction - */ public static function register(): void { $instrumentation = new CachedInstrumentation('io.opentelemetry.contrib.php.slim'); @@ -65,6 +62,7 @@ public static function register(): void return [$request]; }, + /** @suppress PhanPluginInconsistentReturnFunction */ post: static function (App $app, array $params, ?ResponseInterface $response, ?Throwable $exception) { $scope = Context::storage()->scope(); if (!$scope) { From 9a3b5279d11bf11a43c689d6019b36dd803fbd6c Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Wed, 26 Apr 2023 15:45:28 +0200 Subject: [PATCH 5/5] Make returns consistent --- src/Instrumentation/Slim/src/SlimInstrumentation.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Instrumentation/Slim/src/SlimInstrumentation.php b/src/Instrumentation/Slim/src/SlimInstrumentation.php index 3c60dd2a..354b6d31 100644 --- a/src/Instrumentation/Slim/src/SlimInstrumentation.php +++ b/src/Instrumentation/Slim/src/SlimInstrumentation.php @@ -66,7 +66,7 @@ public static function register(): void post: static function (App $app, array $params, ?ResponseInterface $response, ?Throwable $exception) { $scope = Context::storage()->scope(); if (!$scope) { - return; + return $response; } $scope->detach(); $span = Span::fromContext($scope->context()); @@ -99,6 +99,8 @@ public static function register(): void } $span->end(); + + return $response; } );