diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php index 57fbf3e6..eb7274aa 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -47,13 +47,6 @@ class Middleware */ private $continueAfterResponse; - /** - * Whether the terminating callback has been registered. - * - * @var bool - */ - private $registeredTerminatingCallback = false; - /** * Whether a defined route was matched in the application. * @@ -115,22 +108,11 @@ public function terminate(Request $request, $response): void } if ($this->continueAfterResponse) { - // Ensure we do not register the terminating callback multiple times since there is no point in doing so - if ($this->registeredTerminatingCallback) { - return; - } - - // We need to finish the transaction after the response has been sent to the client - // so we register a terminating callback to do so, this allows us to also capture - // spans that are created during the termination of the application like queue - // dispatched using dispatch(...)->afterResponse(). This middleware is called - // before the terminating callbacks so we are 99.9% sure to be the last one - // to run except if another terminating callback is registered after ours. - app()->terminating(function () { - $this->finishTransaction(); - }); - - $this->registeredTerminatingCallback = true; + // Resolving the transaction finisher class will register the terminating callback + // which is responsible for calling `finishTransaction`. We have registered the + // class as a singleton to keep the state in the container and away from here + // this way we ensure the callback is only registered once even for Octane. + app(TransactionFinisher::class); } else { $this->finishTransaction(); } @@ -220,12 +202,12 @@ private function addAppBootstrapSpan(): ?Span $span = $this->transaction->startChild($spanContextStart); - // Consume the booted timestamp, because we don't want to report the bootstrap span more than once - $this->bootedTimestamp = null; - // Add more information about the bootstrap section if possible $this->addBootDetailTimeSpans($span); + // Consume the booted timestamp, because we don't want to report the boot(strap) spans more than once + $this->bootedTimestamp = null; + return $span; } @@ -250,7 +232,7 @@ private function hydrateResponseData(SymfonyResponse $response): void $this->transaction->setHttpStatus($response->getStatusCode()); } - private function finishTransaction(): void + public function finishTransaction(): void { // We could end up multiple times here since we register a terminating callback so // double check if we have a transaction before trying to finish it since it could diff --git a/src/Sentry/Laravel/Tracing/ServiceProvider.php b/src/Sentry/Laravel/Tracing/ServiceProvider.php index 55e5c94b..f045d5f4 100644 --- a/src/Sentry/Laravel/Tracing/ServiceProvider.php +++ b/src/Sentry/Laravel/Tracing/ServiceProvider.php @@ -59,6 +59,8 @@ public function boot(): void public function register(): void { + $this->app->singleton(TransactionFinisher::class); + $this->app->singleton(Middleware::class, function () { $continueAfterResponse = ($this->getTracingConfig()['continue_after_response'] ?? true) === true; diff --git a/src/Sentry/Laravel/Tracing/TransactionFinisher.php b/src/Sentry/Laravel/Tracing/TransactionFinisher.php new file mode 100644 index 00000000..d75a26ee --- /dev/null +++ b/src/Sentry/Laravel/Tracing/TransactionFinisher.php @@ -0,0 +1,28 @@ +afterResponse() which are terminating + // callbacks themselfs just like we do below. + // + // This class is registered as a singleton in the container to ensure it's only + // instantiated once and the terminating callback is only registered once. + // + // It should be resolved from the container before the terminating callbacks are called. + // Good place is in the `terminate` callback of a middleware for example. + // This way we can be 99.9% sure to be the last ones to run. + app()->terminating(function () { + app(Middleware::class)->finishTransaction(); + }); + } +}