From bd30dde8e0a963d981e54acef7d33a37b9600cae Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Thu, 11 Jul 2024 22:57:46 +0200 Subject: [PATCH 1/2] feat: Record handled exceptions This change records exceptions even if they are handled by the Kernel. This is handy for apps that have an error handler registered. --- .../Symfony/src/SymfonyInstrumentation.php | 24 +++++++++++++++++++ .../SymfonyInstrumentationTest.php | 23 ++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php b/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php index 898ca0fb..8f54ed74 100644 --- a/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php +++ b/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php @@ -141,5 +141,29 @@ public static function register(): void $span->end(); } ); + + hook( + HttpKernel::class, + 'handleThrowable', + pre: static function ( + HttpKernel $kernel, + array $params, + string $class, + string $function, + ?string $filename, + ?int $lineno, + ) use ($instrumentation): array { + /** @var \Throwable $throwable */ + $throwable = $params[0]; + + Span::getCurrent() + ->recordException($throwable, [ + TraceAttributes::EXCEPTION_ESCAPED => true, + ]) + ->setStatus(StatusCode::STATUS_ERROR, $throwable->getMessage()); + + return $params; + }, + ); } } diff --git a/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php b/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php index 37548aa0..ca8e3b49 100644 --- a/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php +++ b/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php @@ -5,6 +5,7 @@ namespace OpenTelemetry\Tests\Instrumentation\Symfony\tests\Integration; use OpenTelemetry\API\Trace\SpanKind; +use OpenTelemetry\API\Trace\StatusCode; use OpenTelemetry\Contrib\Propagation\ServerTiming\ServerTimingPropagator; use OpenTelemetry\Contrib\Propagation\TraceResponse\TraceResponsePropagator; use OpenTelemetry\SDK\Trace\ImmutableSpan; @@ -18,6 +19,7 @@ use Symfony\Component\HttpFoundation\StreamedResponse; use Symfony\Component\HttpKernel\Controller\ArgumentResolverInterface; use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface; +use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpKernel\HttpKernel; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -40,6 +42,27 @@ public function test_http_kernel_handle_exception(): void ); } + public function test_kernel_marks_root_as_erroneous(): void + { + $this->expectException(HttpException::class); + $kernel = $this->getHttpKernel(new EventDispatcher(), function () { + throw new HttpException(500, 'foo'); + }); + $this->assertCount(0, $this->storage); + + $response = $kernel->handle(new Request(), HttpKernelInterface::MAIN_REQUEST, true); + + $this->assertCount(1, $this->storage); + $this->assertSame(500, $this->storage[0]->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_STATUS_CODE)); + $this->assertSame(StatusCode::STATUS_ERROR, $this->storage[0]->getStatus()); + + $this->assertArrayHasKey( + TraceResponsePropagator::TRACERESPONSE, + $response->headers->all(), + 'traceresponse header is present if TraceResponsePropagator is present' + ); + } + public function test_http_kernel_handle_attributes(): void { $kernel = $this->getHttpKernel(new EventDispatcher()); From 406ff50c805bfb4db89306b6bd43bd39ae10951c Mon Sep 17 00:00:00 2001 From: Cedric Ziel Date: Thu, 11 Jul 2024 23:09:45 +0200 Subject: [PATCH 2/2] fix: style, stan --- src/Instrumentation/Symfony/src/SymfonyInstrumentation.php | 2 +- .../Symfony/tests/Integration/SymfonyInstrumentationTest.php | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php b/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php index 8f54ed74..0c05a9b3 100644 --- a/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php +++ b/src/Instrumentation/Symfony/src/SymfonyInstrumentation.php @@ -152,7 +152,7 @@ public static function register(): void string $function, ?string $filename, ?int $lineno, - ) use ($instrumentation): array { + ): array { /** @var \Throwable $throwable */ $throwable = $params[0]; diff --git a/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php b/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php index ca8e3b49..7bbe0a60 100644 --- a/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php +++ b/src/Instrumentation/Symfony/tests/Integration/SymfonyInstrumentationTest.php @@ -42,7 +42,7 @@ public function test_http_kernel_handle_exception(): void ); } - public function test_kernel_marks_root_as_erroneous(): void + public function test_http_kernel_marks_root_as_erroneous(): void { $this->expectException(HttpException::class); $kernel = $this->getHttpKernel(new EventDispatcher(), function () { @@ -54,7 +54,8 @@ public function test_kernel_marks_root_as_erroneous(): void $this->assertCount(1, $this->storage); $this->assertSame(500, $this->storage[0]->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_STATUS_CODE)); - $this->assertSame(StatusCode::STATUS_ERROR, $this->storage[0]->getStatus()); + + $this->assertSame(StatusCode::STATUS_ERROR, $this->storage[0]->getStatus()->getCode()); $this->assertArrayHasKey( TraceResponsePropagator::TRACERESPONSE,