From cfb8a0f05064aecb0ecbb115bbe979dec1632c65 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sun, 8 Sep 2024 20:09:42 +0200 Subject: [PATCH 1/4] Allow us to set the current request in the request fetcher --- src/EventListener/TracingRequestListener.php | 9 ++++++++- src/Integration/RequestFetcher.php | 21 ++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/EventListener/TracingRequestListener.php b/src/EventListener/TracingRequestListener.php index e5797a79..d94f34a7 100644 --- a/src/EventListener/TracingRequestListener.php +++ b/src/EventListener/TracingRequestListener.php @@ -6,6 +6,7 @@ use Sentry\Tracing\TransactionSource; use Symfony\Component\HttpFoundation\Request; +use Sentry\SentryBundle\Integration\RequestFetcher; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Event\TerminateEvent; @@ -75,7 +76,13 @@ public function handleKernelTerminateEvent(TerminateEvent $event): void return; } - $transaction->finish(); + RequestFetcher::withCurrentRequest( + $event->getRequest(), + static function () use ($transaction): void { + $transaction->finish(); + }, + ); + metrics()->flush(); } diff --git a/src/Integration/RequestFetcher.php b/src/Integration/RequestFetcher.php index af371783..cba88383 100644 --- a/src/Integration/RequestFetcher.php +++ b/src/Integration/RequestFetcher.php @@ -6,6 +6,7 @@ use GuzzleHttp\Psr7\HttpFactory; use Psr\Http\Message\ServerRequestInterface; +use Symfony\Component\HttpFoundation\Request; use Sentry\Integration\RequestFetcherInterface; use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory; use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; @@ -28,6 +29,11 @@ final class RequestFetcher implements RequestFetcherInterface */ private $httpMessageFactory; + /** + * @var \Symfony\Component\HttpFoundation\Request|null The current request + */ + private static $currentRequest = null; + /** * Class constructor. * @@ -41,7 +47,7 @@ public function __construct(RequestStack $requestStack, ?HttpMessageFactoryInter new HttpFactory(), new HttpFactory(), new HttpFactory(), - new HttpFactory() + new HttpFactory(), ); } @@ -50,7 +56,7 @@ public function __construct(RequestStack $requestStack, ?HttpMessageFactoryInter */ public function fetchRequest(): ?ServerRequestInterface { - $request = $this->requestStack->getCurrentRequest(); + $request = self::$currentRequest ?? $this->requestStack->getCurrentRequest(); if (null === $request) { return null; @@ -62,4 +68,15 @@ public function fetchRequest(): ?ServerRequestInterface return null; } } + + public static function withCurrentRequest(Request $request, callable $callback): void + { + self::$currentRequest = $request; + + try { + $callback(); + } finally { + self::$currentRequest = null; + } + } } From 018e46a39112e59b60e2130456047a9f6d724ad5 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sun, 8 Sep 2024 20:30:39 +0200 Subject: [PATCH 2/4] Set the traced request directly on the request fetcher --- src/EventListener/TracingRequestListener.php | 30 +++++++++++++++----- src/Integration/RequestFetcher.php | 22 ++++++-------- src/Resources/config/services.xml | 1 + 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/EventListener/TracingRequestListener.php b/src/EventListener/TracingRequestListener.php index d94f34a7..729941c8 100644 --- a/src/EventListener/TracingRequestListener.php +++ b/src/EventListener/TracingRequestListener.php @@ -4,8 +4,10 @@ namespace Sentry\SentryBundle\EventListener; +use Sentry\State\HubInterface; use Sentry\Tracing\TransactionSource; use Symfony\Component\HttpFoundation\Request; +use Sentry\Integration\RequestFetcherInterface; use Sentry\SentryBundle\Integration\RequestFetcher; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Event\TerminateEvent; @@ -21,6 +23,18 @@ */ final class TracingRequestListener extends AbstractTracingRequestListener { + /** + * @var \Sentry\Integration\RequestFetcherInterface + */ + private $requestFetcher; + + public function __construct(HubInterface $hub, RequestFetcherInterface $requestFetcher) + { + parent::__construct($hub); + + $this->requestFetcher = $requestFetcher; + } + /** * This method is called for each subrequest handled by the framework and * starts a new {@see Transaction}. @@ -36,6 +50,10 @@ public function handleKernelRequestEvent(RequestEvent $event): void /** @var Request $request */ $request = $event->getRequest(); + if ($this->requestFetcher instanceof RequestFetcher) { + $this->requestFetcher->setRequest($request); + } + /** @var float $requestStartTime */ $requestStartTime = $request->server->get('REQUEST_TIME_FLOAT', microtime(true)); @@ -76,14 +94,12 @@ public function handleKernelTerminateEvent(TerminateEvent $event): void return; } - RequestFetcher::withCurrentRequest( - $event->getRequest(), - static function () use ($transaction): void { - $transaction->finish(); - }, - ); - + $transaction->finish(); metrics()->flush(); + + if ($this->requestFetcher instanceof RequestFetcher) { + $this->requestFetcher->setRequest(null); + } } /** diff --git a/src/Integration/RequestFetcher.php b/src/Integration/RequestFetcher.php index cba88383..0d8dfac8 100644 --- a/src/Integration/RequestFetcher.php +++ b/src/Integration/RequestFetcher.php @@ -25,14 +25,14 @@ final class RequestFetcher implements RequestFetcherInterface private $requestStack; /** - * @var HttpMessageFactoryInterface The factory to convert Symfony requests to PSR-7 requests + * @var \Symfony\Component\HttpFoundation\Request|null The current request */ - private $httpMessageFactory; + private $currentRequest; /** - * @var \Symfony\Component\HttpFoundation\Request|null The current request + * @var HttpMessageFactoryInterface The factory to convert Symfony requests to PSR-7 requests */ - private static $currentRequest = null; + private $httpMessageFactory; /** * Class constructor. @@ -47,7 +47,7 @@ public function __construct(RequestStack $requestStack, ?HttpMessageFactoryInter new HttpFactory(), new HttpFactory(), new HttpFactory(), - new HttpFactory(), + new HttpFactory() ); } @@ -56,7 +56,7 @@ public function __construct(RequestStack $requestStack, ?HttpMessageFactoryInter */ public function fetchRequest(): ?ServerRequestInterface { - $request = self::$currentRequest ?? $this->requestStack->getCurrentRequest(); + $request = $this->currentRequest ?? $this->requestStack->getCurrentRequest(); if (null === $request) { return null; @@ -69,14 +69,8 @@ public function fetchRequest(): ?ServerRequestInterface } } - public static function withCurrentRequest(Request $request, callable $callback): void + public function setRequest(?Request $request): void { - self::$currentRequest = $request; - - try { - $callback(); - } finally { - self::$currentRequest = null; - } + $this->currentRequest = $request; } } diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index ef3ce7d8..02ad8994 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -48,6 +48,7 @@ + From 668aafb5a4645a2d9468a89e601fe85c67ffb816 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sun, 8 Sep 2024 20:40:59 +0200 Subject: [PATCH 3/4] CS --- .php-cs-fixer.dist.php | 4 ++++ src/EventListener/TracingRequestListener.php | 6 +++--- src/Integration/RequestFetcher.php | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 5906a68d..3f0de29f 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -31,6 +31,10 @@ 'method' => 'multi', 'property' => 'multi', ], + 'trailing_comma_in_multiline' => [ + 'after_heredoc' => false, + 'elements' => ['arrays'], + ], ]) ->setFinder( PhpCsFixer\Finder::create() diff --git a/src/EventListener/TracingRequestListener.php b/src/EventListener/TracingRequestListener.php index 729941c8..2f565013 100644 --- a/src/EventListener/TracingRequestListener.php +++ b/src/EventListener/TracingRequestListener.php @@ -4,11 +4,11 @@ namespace Sentry\SentryBundle\EventListener; +use Sentry\Integration\RequestFetcherInterface; +use Sentry\SentryBundle\Integration\RequestFetcher; use Sentry\State\HubInterface; use Sentry\Tracing\TransactionSource; use Symfony\Component\HttpFoundation\Request; -use Sentry\Integration\RequestFetcherInterface; -use Sentry\SentryBundle\Integration\RequestFetcher; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Event\TerminateEvent; @@ -24,7 +24,7 @@ final class TracingRequestListener extends AbstractTracingRequestListener { /** - * @var \Sentry\Integration\RequestFetcherInterface + * @var RequestFetcherInterface */ private $requestFetcher; diff --git a/src/Integration/RequestFetcher.php b/src/Integration/RequestFetcher.php index 0d8dfac8..2f2d2cd2 100644 --- a/src/Integration/RequestFetcher.php +++ b/src/Integration/RequestFetcher.php @@ -6,10 +6,10 @@ use GuzzleHttp\Psr7\HttpFactory; use Psr\Http\Message\ServerRequestInterface; -use Symfony\Component\HttpFoundation\Request; use Sentry\Integration\RequestFetcherInterface; use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory; use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; /** @@ -25,7 +25,7 @@ final class RequestFetcher implements RequestFetcherInterface private $requestStack; /** - * @var \Symfony\Component\HttpFoundation\Request|null The current request + * @var Request|null The current request */ private $currentRequest; From 1cc3154af4a48b0496693f1819a29ae3c866d784 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sun, 8 Sep 2024 20:42:03 +0200 Subject: [PATCH 4/4] Make new constructor argument optional --- src/EventListener/TracingRequestListener.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EventListener/TracingRequestListener.php b/src/EventListener/TracingRequestListener.php index 2f565013..f94f9dda 100644 --- a/src/EventListener/TracingRequestListener.php +++ b/src/EventListener/TracingRequestListener.php @@ -24,11 +24,11 @@ final class TracingRequestListener extends AbstractTracingRequestListener { /** - * @var RequestFetcherInterface + * @var RequestFetcherInterface|null */ private $requestFetcher; - public function __construct(HubInterface $hub, RequestFetcherInterface $requestFetcher) + public function __construct(HubInterface $hub, ?RequestFetcherInterface $requestFetcher = null) { parent::__construct($hub);