Cleanup event handlers (#592)

Author: stayallive

CHANGELOG.md

@@ -12,13 +12,17 @@
     - Drop support for Laravel 5.x (#581)
 - Remove `Sentry\Integration::extractNameForRoute()`, it's alternative `Sentry\Integration::extractNameAndSourceForRoute()` is marked as `@internal` (#580)
 - Remove extracting route name or controller for transaction names (#583). This unifies the transaction names to a more concise format.
+- Remove internal `Sentry\Integration::currentTracingSpan()`, use `SentrySdk::getCurrentHub()->getSpan()` if you were using this internal method. (#592)
 
 **Other changes**
 
 - Add support for Dynamic Sampling (#572)
 - Set the tracing transaction name on the `Illuminate\Routing\Events\RouteMatched` instead of at the end of the request (#580)
 - Add tracing span for Laravel HTTP client requests (#585)
 - Simplify Sentry meta tag retrieval, by adding `Sentry\Laravel\Integration::sentryMeta()` (#586)
+- Fix tracing with nested queue jobs (mostly when running jobs in the `sync` driver) (#592)
+- Add a `http.route` span, this span indicates the time that is spent inside a controller method or route closure (#593)
+- Add a `db.transaction` span, this span indicates the time that is spent inside a database transaction (#594)
 
 ## 2.14.2
 

src/Sentry/Laravel/EventHandler.php

@@ -140,9 +140,9 @@ class EventHandler
     /**
      * Indicates if we pushed a scope for the queue.
      *
-     * @var bool
+     * @var int
      */
-    private $pushedQueueScope = false;
+    private $pushedQueueScopeCount = 0;
 
     /**
      * Indicates if we pushed a scope for Octane.
@@ -173,76 +173,40 @@ public function __construct(Container $container, array $config)
     /**
      * Attach all event handlers.
      */
-    public function subscribe(): void
+    public function subscribe(Dispatcher $dispatcher): void
     {
-        /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */
-        try {
-            $dispatcher = $this->container->make(Dispatcher::class);
-
-            foreach (static::$eventHandlerMap as $eventName => $handler) {
-                $dispatcher->listen($eventName, [$this, $handler]);
-            }
-        } catch (BindingResolutionException $e) {
-            // If we cannot resolve the event dispatcher we also cannot listen to events
+        foreach (static::$eventHandlerMap as $eventName => $handler) {
+            $dispatcher->listen($eventName, [$this, $handler]);
         }
     }
 
     /**
      * Attach all authentication event handlers.
      */
-    public function subscribeAuthEvents(): void
+    public function subscribeAuthEvents(Dispatcher $dispatcher): void
     {
-        /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */
-        try {
-            $dispatcher = $this->container->make(Dispatcher::class);
-
-            foreach (static::$authEventHandlerMap as $eventName => $handler) {
-                $dispatcher->listen($eventName, [$this, $handler]);
-            }
-        } catch (BindingResolutionException $e) {
-            // If we cannot resolve the event dispatcher we also cannot listen to events
+        foreach (static::$authEventHandlerMap as $eventName => $handler) {
+            $dispatcher->listen($eventName, [$this, $handler]);
         }
     }
 
     /**
      * Attach all queue event handlers.
      */
-    public function subscribeOctaneEvents(): void
+    public function subscribeOctaneEvents(Dispatcher $dispatcher): void
     {
-        /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */
-        try {
-            $dispatcher = $this->container->make(Dispatcher::class);
-
-            foreach (static::$octaneEventHandlerMap as $eventName => $handler) {
-                $dispatcher->listen($eventName, [$this, $handler]);
-            }
-        } catch (BindingResolutionException $e) {
-            // If we cannot resolve the event dispatcher we also cannot listen to events
+        foreach (static::$octaneEventHandlerMap as $eventName => $handler) {
+            $dispatcher->listen($eventName, [$this, $handler]);
         }
     }
 
     /**
      * Attach all queue event handlers.
-     *
-     * @param \Illuminate\Queue\QueueManager $queue
      */
-    public function subscribeQueueEvents(QueueManager $queue): void
+    public function subscribeQueueEvents(Dispatcher $dispatcher): void
     {
-        $queue->looping(function () {
-            $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScope);
-
-            $this->pushedQueueScope = false;
-        });
-
-        /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */
-        try {
-            $dispatcher = $this->container->make(Dispatcher::class);
-
-            foreach (static::$queueEventHandlerMap as $eventName => $handler) {
-                $dispatcher->listen($eventName, [$this, $handler]);
-            }
-        } catch (BindingResolutionException $e) {
-            // If we cannot resolve the event dispatcher we also cannot listen to events
+        foreach (static::$queueEventHandlerMap as $eventName => $handler) {
+            $dispatcher->listen($eventName, [$this, $handler]);
         }
     }
 
@@ -382,11 +346,9 @@ private function configureUserScopeFromModel($authUser): void
 
     protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void
     {
-        $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScope);
-
         $this->prepareScopeForTaskWithinLongRunningProcess();
 
-        $this->pushedQueueScope = true;
+        ++$this->pushedQueueScopeCount;
 
         if (!$this->recordQueueInfo) {
             return;
@@ -415,11 +377,15 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event):
 
     protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event): void
     {
+        $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScopeCount > 0);
+
         $this->afterTaskWithinLongRunningProcess();
     }
 
     protected function queueJobProcessedHandler(QueueEvents\JobProcessed $event): void
     {
+        $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScopeCount > 0);
+
         $this->afterTaskWithinLongRunningProcess();
     }
 

src/Sentry/Laravel/Integration.php

@@ -3,10 +3,7 @@
 namespace Sentry\Laravel;
 
 use Illuminate\Routing\Route;
-use Illuminate\Support\Str;
 use Sentry\SentrySdk;
-use Sentry\Tracing\Span;
-use Sentry\Tracing\Transaction;
 use Sentry\Tracing\TransactionSource;
 use function Sentry\addBreadcrumb;
 use function Sentry\configureScope;
@@ -118,7 +115,7 @@ public static function extractNameAndSourceForRoute(Route $route): array
     {
         return [
             '/' . ltrim($route->uri(), '/'),
-            TransactionSource::route()
+            TransactionSource::route(),
         ];
     }
 
@@ -140,7 +137,7 @@ public static function sentryMeta(): string
      */
     public static function sentryTracingMeta(): string
     {
-        $span = self::currentTracingSpan();
+        $span = SentrySdk::getCurrentHub()->getSpan();
 
         if ($span === null) {
             return '';
@@ -157,36 +154,12 @@ public static function sentryTracingMeta(): string
      */
     public static function sentryBaggageMeta(): string
     {
-        $span = self::currentTracingSpan();
+        $span = SentrySdk::getCurrentHub()->getSpan();
 
         if ($span === null) {
             return '';
         }
 
         return sprintf('<meta name="baggage" content="%s"/>', $span->toBaggage());
     }
-
-    /**
-     * Get the current active tracing span from the scope.
-     *
-     * @return \Sentry\Tracing\Transaction|null
-     *
-     * @internal This is used internally as an easy way to retrieve the current active transaction.
-     */
-    public static function currentTransaction(): ?Transaction
-    {
-        return SentrySdk::getCurrentHub()->getTransaction();
-    }
-
-    /**
-     * Get the current active tracing span from the scope.
-     *
-     * @return \Sentry\Tracing\Span|null
-     *
-     * @internal This is used internally as an easy way to retrieve the current active tracing span.
-     */
-    public static function currentTracingSpan(): ?Span
-    {
-        return SentrySdk::getCurrentHub()->getSpan();
-    }
 }

src/Sentry/Laravel/ServiceProvider.php

@@ -2,6 +2,8 @@
 
 namespace Sentry\Laravel;
 
+use Illuminate\Contracts\Container\BindingResolutionException;
+use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Contracts\Http\Kernel as HttpKernelInterface;
 use Illuminate\Foundation\Application as Laravel;
 use Illuminate\Foundation\Http\Kernel as HttpKernel;
@@ -95,18 +97,25 @@ protected function bindEvents(): void
 
         $handler = new EventHandler($this->app, $userConfig);
 
-        $handler->subscribe();
+        try {
+            /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */
+            $dispatcher = $this->app->make(Dispatcher::class);
 
-        if ($this->app->bound('octane')) {
-            $handler->subscribeOctaneEvents();
-        }
+            $handler->subscribe($dispatcher);
 
-        if ($this->app->bound('queue')) {
-            $handler->subscribeQueueEvents($this->app->make('queue'));
-        }
+            if ($this->app->bound('octane')) {
+                $handler->subscribeOctaneEvents($dispatcher);
+            }
 
-        if (isset($userConfig['send_default_pii']) && $userConfig['send_default_pii'] !== false) {
-            $handler->subscribeAuthEvents();
+            if ($this->app->bound('queue')) {
+                $handler->subscribeQueueEvents($dispatcher, $this->app->make('queue'));
+            }
+
+            if (isset($userConfig['send_default_pii']) && $userConfig['send_default_pii'] !== false) {
+                $handler->subscribeAuthEvents($dispatcher);
+            }
+        } catch (BindingResolutionException $e) {
+            // If we cannot resolve the event dispatcher we also cannot listen to events
         }
     }