Improve tracing (#580)

Author: stayallive

CHANGELOG.md

@@ -3,11 +3,14 @@
 ## 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)
 
 ## 2.14.1
 
 - Fix not setting the correct SDK ID and version when running the `sentry:test` command (#582)
+- Transaction names now only show the parameterized URL (`/some/{route}`) instead of the route name or controller class (#583)
 
 ## 2.14.0
 

config/sentry.php

@@ -45,13 +45,14 @@
 
         // 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
     'send_default_pii' => env('SENTRY_SEND_DEFAULT_PII', false),
 
     'traces_sample_rate' => (float)(env('SENTRY_TRACES_SAMPLE_RATE', 0.0)),
 
-    'controllers_base_namespace' => env('SENTRY_CONTROLLERS_BASE_NAMESPACE', 'App\\Http\\Controllers'),
-
 ];

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;
@@ -21,11 +22,6 @@ class Integration implements IntegrationInterface
      */
     private static $transaction;
 
-    /**
-     * @var null|string
-     */
-    private static $baseControllerNamespace;
-
     /**
      * {@inheritdoc}
      */
@@ -94,14 +90,6 @@ public static function setTransaction(?string $transaction): void
         self::$transaction = $transaction;
     }
 
-    /**
-     * @param null|string $namespace
-     */
-    public static function setControllersBaseNamespace(?string $namespace): void
-    {
-        self::$baseControllerNamespace = $namespace !== null ? trim($namespace, '\\') : null;
-    }
-
     /**
      * Block until all async events are processed for the HTTP transport.
      *
@@ -117,21 +105,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.
      *
@@ -143,75 +116,10 @@ public static function extractNameForRoute(Route $route): string
      */
     public static function extractNameAndSourceForRoute(Route $route): array
     {
-        $source = null;
-        $routeName = null;
-
-        // some.action (route name/alias)
-        if ($route->getName()) {
-            $source = TransactionSource::component();
-            $routeName = self::extractNameForNamedRoute($route->getName());
-        }
-
-        // Some\Controller@someAction (controller action)
-        if (empty($routeName) && $route->getActionName()) {
-            $source = TransactionSource::component();
-            $routeName = self::extractNameForActionRoute($route->getActionName());
-        }
-
-        // /some/{action} // Fallback to the route uri (with parameter placeholders)
-        if (empty($routeName) || $routeName === 'Closure') {
-            $source = TransactionSource::route();
-            $routeName = '/' . ltrim($route->uri(), '/');
-        }
-
-        return [$routeName, $source];
-    }
-
-    /**
-     * Take a route name and return it only if it's a usable route name.
-     *
-     * @param string $name
-     *
-     * @return string|null
-     */
-    private static function extractNameForNamedRoute(string $name): ?string
-    {
-        // Laravel 7 route caching generates a route names if the user didn't specify one
-        // theirselfs to optimize route matching. These route names are useless to the
-        // developer so if we encounter a generated route name we discard the value
-        if (Str::contains($name, 'generated::')) {
-            return null;
-        }
-
-        // If the route name ends with a `.` we assume an incomplete group name prefix
-        // we discard this value since it will most likely not mean anything to the
-        // developer and will be duplicated by other unnamed routes in the group
-        if (Str::endsWith($name, '.')) {
-            return null;
-        }
-
-        return $name;
-    }
-
-    /**
-     * Take a controller action and strip away the base namespace if needed.
-     *
-     * @param string $action
-     *
-     * @return string
-     */
-    private static function extractNameForActionRoute(string $action): string
-    {
-        $routeName = ltrim($action, '\\');
-
-        $baseNamespace = self::$baseControllerNamespace ?? '';
-
-        if (empty($baseNamespace)) {
-            return $routeName;
-        }
-
-        // Strip away the base namespace from the action name
-        return ltrim(Str::after($routeName, $baseNamespace), '\\');
+        return [
+            '/' . ltrim($route->uri(), '/'),
+            TransactionSource::route()
+        ];
     }
 
     /**
@@ -258,6 +166,18 @@ public static function sentryBaggageMeta(): string
         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.
      *

src/Sentry/Laravel/ServiceProvider.php

@@ -33,7 +33,8 @@ class ServiceProvider extends BaseServiceProvider
         'integrations',
         // This is kept for backwards compatibility and can be dropped in a future breaking release
         'breadcrumbs.sql_bindings',
-        // The base namespace for controllers to strip of the beginning of controller class names
+
+        // This config option is no longer in use but to prevent errors when upgrading we leave it here to be discarded
         'controllers_base_namespace',
     ];
 
@@ -125,12 +126,6 @@ protected function registerArtisanCommands(): void
      */
     protected function configureAndRegisterClient(): void
     {
-        $userConfig = $this->getUserConfig();
-
-        if (isset($userConfig['controllers_base_namespace'])) {
-            Integration::setControllersBaseNamespace($userConfig['controllers_base_namespace']);
-        }
-
         $this->app->bind(ClientBuilderInterface::class, function () {
             $basePath   = base_path();
             $userConfig = $this->getUserConfig();
@@ -161,15 +156,15 @@ protected function configureAndRegisterClient(): void
             return $clientBuilder;
         });
 
-        $this->app->singleton(HubInterface::class, function ($app) {
+        $this->app->singleton(HubInterface::class, function () {
             /** @var \Sentry\ClientBuilderInterface $clientBuilder */
             $clientBuilder = $this->app->make(ClientBuilderInterface::class);
 
             $options = $clientBuilder->getOptions();
 
             $userIntegrations = $this->resolveIntegrationsFromUserConfig();
 
-            $options->setIntegrations(function (array $integrations) use ($options, $userIntegrations, $app) {
+            $options->setIntegrations(function (array $integrations) use ($options, $userIntegrations) {
                 if ($options->hasDefaultIntegrations()) {
                     // Remove the default error and fatal exception listeners to let Laravel handle those
                     // itself. These event are still bubbling up through the documented changes in the users

src/Sentry/Laravel/Tracing/EventHandler.php

@@ -7,9 +7,11 @@
 use Illuminate\Contracts\Container\Container;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Database\Events as DatabaseEvents;
+use Illuminate\Http\Client\Events as HttpClientEvents;
 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,7 +32,11 @@ class EventHandler
      * @var array
      */
     protected static $eventHandlerMap = [
+        RoutingEvents\RouteMatched::class => 'routeMatched',
         DatabaseEvents\QueryExecuted::class => 'queryExecuted',
+        HttpClientEvents\RequestSending::class => 'httpClientRequestSending',
+        HttpClientEvents\ResponseReceived::class => 'httpClientResponseReceived',
+        HttpClientEvents\ConnectionFailed::class => 'httpClientConnectionFailed',
     ];
 
     /**
@@ -93,6 +99,20 @@ class EventHandler
      */
     private $currentQueueJobSpan;
 
+    /**
+     * Holds a reference to the parent http client request span.
+     *
+     * @var \Sentry\Tracing\Span|null
+     */
+    private $parentHttpClientRequestSpan;
+
+    /**
+     * Holds a reference to the current http client request span.
+     *
+     * @var \Sentry\Tracing\Span|null
+     */
+    private $currentHttpClientRequestSpan;
+
     /**
      * The backtrace helper.
      *
@@ -122,6 +142,7 @@ public function __construct(Container $container, BacktraceHelper $backtraceHelp
     /**
      * Attach all event handlers.
      *
+     * @uses self::routeMatchedHandler()
      * @uses self::queryExecutedHandler()
      */
     public function subscribe(): void
@@ -202,6 +223,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) {
@@ -250,6 +285,56 @@ private function resolveQueryOriginFromBacktrace(): ?string
         return "{$filePath}:{$firstAppFrame->getLine()}";
     }
 
+    protected function httpClientRequestSendingHandler(HttpClientEvents\RequestSending $event): void
+    {
+        $parentSpan = Integration::currentTracingSpan();
+
+        if ($parentSpan === null) {
+            return;
+        }
+
+        $context = new SpanContext;
+
+        $context->setOp('http.client');
+        $context->setDescription($event->request->method() . ' ' . $event->request->url());
+        $context->setStartTimestamp(microtime(true));
+
+        $this->currentHttpClientRequestSpan = $parentSpan->startChild($context);
+
+        $this->parentHttpClientRequestSpan = $parentSpan;
+
+        SentrySdk::getCurrentHub()->setSpan($this->currentHttpClientRequestSpan);
+    }
+
+    protected function httpClientResponseReceivedHandler(HttpClientEvents\ResponseReceived $event): void
+    {
+        if ($this->currentHttpClientRequestSpan !== null) {
+            $this->currentHttpClientRequestSpan->setHttpStatus($event->response->status());
+            $this->afterHttpClientRequest();
+        }
+    }
+
+    protected function httpClientConnectionFailedHandler(HttpClientEvents\ConnectionFailed $event): void
+    {
+        if ($this->currentHttpClientRequestSpan !== null) {
+            $this->currentHttpClientRequestSpan->setStatus(SpanStatus::internalError());
+            $this->afterHttpClientRequest();
+        }
+    }
+
+    private function afterHttpClientRequest(): void
+    {
+        if ($this->currentHttpClientRequestSpan === null) {
+            return;
+        }
+
+        $this->currentHttpClientRequestSpan->finish();
+        $this->currentHttpClientRequestSpan = null;
+
+        SentrySdk::getCurrentHub()->setSpan($this->parentHttpClientRequestSpan);
+        $this->parentHttpClientRequestSpan = null;
+    }
+
     protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void
     {
         $parentSpan = Integration::currentTracingSpan();
@@ -278,24 +363,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());
         }