Improve tracing

CHANGELOG.md

@@ -3,6 +3,8 @@
 ## Unreleased
 
 - Drop support for Laravel Lumen (#579)
+- Set the tracing transaction name on the `Illuminate\Routing\Events\RouteMatched` instead of at the end of the request (#580)
+- Remove `Sentry\Integration::extractNameForRoute()`, it's alternative `Sentry\Integration::extractNameAndSourceForRoute()` is marked as `@internal` (#580)
 - Drop support for Laravel 5.x (#581)
 - Fix not setting the correct SDK ID and version when running the `sentry:test` command (#582)
 

config/sentry.php

@@ -45,6 +45,9 @@
 
         // Indicates if the tracing integrations supplied by Sentry should be loaded
         'default_integrations' => true,
+
+        // Indicates that requests without a matching route should be traced
+        'missing_routes' => false,
     ],
 
     // @see: https://docs.sentry.io/platforms/php/configuration/options/#send-default-pii

src/Sentry/Laravel/Integration.php

@@ -6,6 +6,7 @@
 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;
@@ -117,21 +118,6 @@ public static function flushEvents(): void
         }
     }
 
-    /**
-     * Extract the readable name for a route.
-     *
-     * @param \Illuminate\Routing\Route $route
-     *
-     * @return string
-     *
-     * @internal   This helper is used in various places to extra meaninful info from a Laravel Route object.
-     * @deprecated This will be removed in version 3.0, use `extractNameAndSourceForRoute` instead.
-     */
-    public static function extractNameForRoute(Route $route): string
-    {
-        return self::extractNameAndSourceForRoute($route)[0];
-    }
-
     /**
      * Extract the readable name for a route and the transaction source for where that route name came from.
      *
@@ -251,6 +237,18 @@ public static function sentryBaggageMeta(): string
         return $content;
     }
 
+    /**
+     * 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.
      *

src/Sentry/Laravel/Tracing/EventHandler.php

@@ -10,6 +10,7 @@
 use Illuminate\Queue\Events as QueueEvents;
 use Illuminate\Queue\Queue;
 use Illuminate\Queue\QueueManager;
+use Illuminate\Routing\Events as RoutingEvents;
 use RuntimeException;
 use Sentry\Laravel\Integration;
 use Sentry\SentrySdk;
@@ -30,6 +31,7 @@ class EventHandler
      * @var array
      */
     protected static $eventHandlerMap = [
+        RoutingEvents\RouteMatched::class => 'routeMatched',
         DatabaseEvents\QueryExecuted::class => 'queryExecuted',
     ];
 
@@ -122,6 +124,7 @@ public function __construct(Container $container, BacktraceHelper $backtraceHelp
     /**
      * Attach all event handlers.
      *
+     * @uses self::routeMatchedHandler()
      * @uses self::queryExecutedHandler()
      */
     public function subscribe(): void
@@ -202,6 +205,20 @@ public function __call(string $method, array $arguments)
         }
     }
 
+    protected function routeMatchedHandler(RoutingEvents\RouteMatched $match): void
+    {
+        $transaction = Integration::currentTransaction();
+
+        if ($transaction === null) {
+            return;
+        }
+
+        [$transactionName, $transactionSource] = Integration::extractNameAndSourceForRoute($match->route);
+
+        $transaction->setName($transactionName);
+        $transaction->getMetadata()->setSource($transactionSource);
+    }
+
     protected function queryExecutedHandler(DatabaseEvents\QueryExecuted $query): void
     {
         if (!$this->traceSqlQueries) {
@@ -278,24 +295,18 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event):
             $context = new SpanContext;
         }
 
+        $resolvedJobName = $event->job->resolveName();
+
         $job = [
             'job' => $event->job->getName(),
             'queue' => $event->job->getQueue(),
+            'resolved' => $event->job->resolveName(),
             'attempts' => $event->job->attempts(),
             'connection' => $event->connectionName,
         ];
 
-        // Resolve name exists only from Laravel 5.3+
-        $resolvedJobName = method_exists($event->job, 'resolveName')
-            ? $event->job->resolveName()
-            : null;
-
-        if ($resolvedJobName !== null) {
-            $job['resolved'] = $resolvedJobName;
-        }
-
         if ($context instanceof TransactionContext) {
-            $context->setName($resolvedJobName ?? $event->job->getName());
+            $context->setName($resolvedJobName);
             $context->setSource(TransactionSource::task());
         }
 

src/Sentry/Laravel/Tracing/Middleware.php

@@ -4,15 +4,13 @@
 
 use Closure;
 use Illuminate\Http\Request;
-use Illuminate\Routing\Route;
-use Sentry\Laravel\Integration;
 use Sentry\SentrySdk;
 use Sentry\State\HubInterface;
 use Sentry\Tracing\Span;
 use Sentry\Tracing\SpanContext;
 use Sentry\Tracing\TransactionContext;
 use Sentry\Tracing\TransactionSource;
-use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
 
 class Middleware
 {
@@ -45,7 +43,7 @@ class Middleware
      *
      * @return mixed
      */
-    public function handle($request, Closure $next)
+    public function handle(Request $request, Closure $next)
     {
         if (app()->bound(HubInterface::class)) {
             $this->startTransaction($request, app(HubInterface::class));
@@ -57,14 +55,19 @@ public function handle($request, Closure $next)
     /**
      * Handle the application termination.
      *
-     * @param \Illuminate\Http\Request                   $request
-     * @param \Symfony\Component\HttpFoundation\Response $response
+     * @param \Illuminate\Http\Request $request
+     * @param mixed                    $response
      *
      * @return void
      */
-    public function terminate($request, $response): void
+    public function terminate(Request $request, $response): void
     {
         if ($this->transaction !== null && app()->bound(HubInterface::class)) {
+            // We stop here if a route has not been matched unless we are configured to trace missing routes
+            if (config('sentry.tracing.missing_routes', false) === false && $request->route() === null) {
+                return;
+            }
+
             if ($this->appSpan !== null) {
                 $this->appSpan->finish();
             }
@@ -73,11 +76,7 @@ public function terminate($request, $response): void
             // If the transaction is not on the scope during finish, the trace.context is wrong
             SentrySdk::getCurrentHub()->setSpan($this->transaction);
 
-            if ($request instanceof Request) {
-                $this->hydrateRequestData($request);
-            }
-
-            if ($response instanceof Response) {
+            if ($response instanceof SymfonyResponse) {
                 $this->hydrateResponseData($response);
             }
 
@@ -108,12 +107,17 @@ private function startTransaction(Request $request, HubInterface $sentry): void
             $request->header('baggage', '')
         );
 
+        $requestPath = '/' . ltrim($request->path(), '/');
+
         $context->setOp('http.server');
+        $context->setName($requestPath);
+        $context->setSource(TransactionSource::url());
+        $context->setStartTimestamp($requestStartTime);
+
         $context->setData([
-            'url' => '/' . ltrim($request->path(), '/'),
+            'url' => $requestPath,
             'method' => strtoupper($request->method()),
         ]);
-        $context->setStartTimestamp($requestStartTime);
 
         $this->transaction = $sentry->startTransaction($context);
 
@@ -122,7 +126,7 @@ private function startTransaction(Request $request, HubInterface $sentry): void
 
         $bootstrapSpan = $this->addAppBootstrapSpan($request);
 
-        $appContextStart = new SpanContext();
+        $appContextStart = new SpanContext;
         $appContextStart->setOp('middleware.handle');
         $appContextStart->setStartTimestamp($bootstrapSpan ? $bootstrapSpan->getEndTimestamp() : microtime(true));
 
@@ -143,7 +147,7 @@ private function addAppBootstrapSpan(Request $request): ?Span
             return null;
         }
 
-        $spanContextStart = new SpanContext();
+        $spanContextStart = new SpanContext;
         $spanContextStart->setOp('app.bootstrap');
         $spanContextStart->setStartTimestamp($laravelStartTime);
         $spanContextStart->setEndTimestamp($this->bootedTimestamp);
@@ -175,45 +179,8 @@ private function addBootDetailTimeSpans(Span $bootstrap): void
         $bootstrap->startChild($autoload);
     }
 
-    private function hydrateRequestData(Request $request): void
-    {
-        $route = $request->route();
-
-        if ($route instanceof Route) {
-            [$transactionName, $transactionSource] = Integration::extractNameAndSourceForRoute($route);
-
-            $this->updateTransactionNameIfDefault($transactionName, $transactionSource);
-
-            $this->transaction->setData([
-                'name' => $route->getName(),
-                'action' => $route->getActionName(),
-                'method' => $request->getMethod(),
-            ]);
-        }
-
-        $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/'), TransactionSource::url());
-    }
-
-    private function hydrateResponseData(Response $response): void
+    private function hydrateResponseData(SymfonyResponse $response): void
     {
         $this->transaction->setHttpStatus($response->getStatusCode());
     }
-
-    private function updateTransactionNameIfDefault(?string $name, ?TransactionSource $source): void
-    {
-        // Ignore empty names (and `null`) for caller convenience
-        if (empty($name)) {
-            return;
-        }
-
-        // If the transaction already has a name other than the default
-        // ignore the new name, this will most occur if the user has set a
-        // transaction name themself before the application reaches this point
-        if ($this->transaction->getName() !== TransactionContext::DEFAULT_NAME) {
-            return;
-        }
-
-        $this->transaction->setName($name);
-        $this->transaction->getMetadata()->setSource($source ?? TransactionSource::custom());
-    }
 }

test/Sentry/IntegrationTest.php

@@ -9,6 +9,7 @@
 use Sentry\Event;
 use Sentry\Laravel\Integration;
 use Sentry\State\Scope;
+use Sentry\Tracing\TransactionSource;
 use function Sentry\withScope;
 
 class IntegrationTest extends SentryLaravelTestCase
@@ -92,7 +93,7 @@ public function testExtractingNameForRouteWithName(): void
     {
         $route = (new Route('GET', '/foo', []))->name($routeName = 'foo-bar');
 
-        $this->assertSame($routeName, Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, $routeName, TransactionSource::component());
     }
 
     public function testExtractingNameForRouteWithAction(): void
@@ -101,14 +102,14 @@ public function testExtractingNameForRouteWithAction(): void
             'controller' => $controller = 'SomeController@someAction',
         ]));
 
-        $this->assertSame($controller, Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, $controller, TransactionSource::component());
     }
 
     public function testExtractingNameForRouteWithoutName(): void
     {
         $route = new Route('GET', $url = '/foo', []);
 
-        $this->assertSame($url, Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, $url, TransactionSource::route());
     }
 
     public function testExtractingNameForRouteWithActionAndName(): void
@@ -117,22 +118,22 @@ public function testExtractingNameForRouteWithActionAndName(): void
             'controller' => 'SomeController@someAction',
         ]))->name($routeName = 'foo-bar');
 
-        $this->assertSame($routeName, Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, $routeName, TransactionSource::component());
     }
 
     public function testExtractingNameForRouteWithAutoGeneratedName(): void
     {
         // We fake a generated name here, Laravel generates them each starting with `generated::`
         $route = (new Route('GET', $url = '/foo', []))->name('generated::KoAePbpBofo01ey4');
 
-        $this->assertSame($url, Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, $url, TransactionSource::route());
     }
 
     public function testExtractingNameForRouteWithIncompleteGroupName(): void
     {
         $route = (new Route('GET', $url = '/foo', []))->name('group-name.');
 
-        $this->assertSame($url, Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, $url, TransactionSource::route());
     }
 
     public function testExtractingNameForRouteWithStrippedBaseNamespaceFromAction(): void
@@ -143,8 +144,16 @@ public function testExtractingNameForRouteWithStrippedBaseNamespaceFromAction():
             'controller' => 'BaseNamespace\\SomeController@someAction',
         ]));
 
-        $this->assertSame('SomeController@someAction', Integration::extractNameForRoute($route));
+        $this->assetRouteNameAndSource($route, 'SomeController@someAction', TransactionSource::component());
 
         Integration::setControllersBaseNamespace(null);
     }
+
+    private function assetRouteNameAndSource(Route $route, string $expectedName, TransactionSource $expectedSource): void
+    {
+        [$actualName, $actualSource] = Integration::extractNameAndSourceForRoute($route);
+
+        $this->assertSame($expectedName, $actualName);
+        $this->assertSame($expectedSource, $actualSource);
+    }
 }