From be92d6693d737ddb8a4f9ee1520d358d96d0d0a6 Mon Sep 17 00:00:00 2001
From: Alex Bouma <me@alexbouma.me>
Date: Tue, 3 Nov 2020 16:09:26 +0100
Subject: [PATCH] Move request info tracking to the middleware (#408)

* Hydrate request data in the middleware

* Trim away the leading \ in route action names

* Add changelog entry
---
 CHANGELOG.md                                |  1 +
 src/Sentry/Laravel/Integration.php          |  4 +--
 src/Sentry/Laravel/Tracing/EventHandler.php | 37 ---------------------
 src/Sentry/Laravel/Tracing/Middleware.php   | 33 ++++++++++++++++--
 4 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c37ae1bf..b463de1c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 - Fix incorrectly stripped base controller action from transaction name (#406)
+- Move tracing request/response data hydration to the tracing middleware (#408)
 
 ## 2.1.1
 
diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php
index 0f6f1661..59b04eb7 100644
--- a/src/Sentry/Laravel/Integration.php
+++ b/src/Sentry/Laravel/Integration.php
@@ -148,8 +148,8 @@ public static function extractNameForRoute(Route $route): ?string
         }
 
         if (empty($routeName) && $route->getActionName()) {
-            // SomeController@someAction (controller action)
-            $routeName = $route->getActionName();
+            // Some\Controller@someAction (controller action)
+            $routeName = ltrim($route->getActionName(), '\\');
 
             $baseNamespace = self::$baseControllerNamespace ?? '';
 
diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php
index e6cfab21..e77712fe 100644
--- a/src/Sentry/Laravel/Tracing/EventHandler.php
+++ b/src/Sentry/Laravel/Tracing/EventHandler.php
@@ -5,13 +5,9 @@
 use Exception;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Database\Events\QueryExecuted;
-use Illuminate\Routing\Events\RouteMatched;
-use Illuminate\Routing\Route;
 use RuntimeException;
 use Sentry\Laravel\Integration;
-use Sentry\SentrySdk;
 use Sentry\Tracing\SpanContext;
-use Sentry\Tracing\Transaction;
 
 class EventHandler
 {
@@ -21,9 +17,6 @@ class EventHandler
      * @var array
      */
     protected static $eventHandlerMap = [
-        'router.matched'    => 'routerMatched',  // Until Laravel 5.1
-        RouteMatched::class => 'routeMatched',   // Since Laravel 5.2
-
         'illuminate.query'   => 'query',         // Until Laravel 5.1
         QueryExecuted::class => 'queryExecuted', // Since Laravel 5.2
     ];
@@ -83,36 +76,6 @@ public function __call($method, $arguments)
         }
     }
 
-    /**
-     * Until Laravel 5.1
-     *
-     * @param \Illuminate\Routing\Route $route
-     */
-    protected function routerMatchedHandler(Route $route): void
-    {
-        $transaction = SentrySdk::getCurrentHub()->getTransaction();
-
-        if ($transaction instanceof Transaction) {
-            $routeName = Integration::extractNameForRoute($route) ?? '<unlabeled transaction>';
-
-            $transaction->setName($routeName);
-            $transaction->setData([
-                'action' => $route->getActionName(),
-                'name'   => $route->getName(),
-            ]);
-        }
-    }
-
-    /**
-     * Since Laravel 5.2
-     *
-     * @param \Illuminate\Routing\Events\RouteMatched $match
-     */
-    protected function routeMatchedHandler(RouteMatched $match): void
-    {
-        $this->routerMatchedHandler($match->route);
-    }
-
     /**
      * Until Laravel 5.1
      *
diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php
index 37031f1a..56782019 100644
--- a/src/Sentry/Laravel/Tracing/Middleware.php
+++ b/src/Sentry/Laravel/Tracing/Middleware.php
@@ -5,8 +5,10 @@
 use Closure;
 use Illuminate\Http\Request;
 use Illuminate\Http\Response;
+use Illuminate\Routing\Route;
+use Sentry\Laravel\Integration;
 use Sentry\SentrySdk;
-use Sentry\State\Hub;
+use Sentry\State\HubInterface;
 use Sentry\Tracing\SpanContext;
 use Sentry\Tracing\TransactionContext;
 
@@ -62,15 +64,19 @@ 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) {
-                $this->transaction->setHttpStatus($response->status());
+                $this->hydrateResponseData($response);
             }
 
             $this->transaction->finish();
         }
     }
 
-    private function startTransaction(Request $request, Hub $sentry): void
+    private function startTransaction(Request $request, HubInterface $sentry): void
     {
         $path = '/' . ltrim($request->path(), '/');
         $fallbackTime = microtime(true);
@@ -140,4 +146,25 @@ private function addBootTimeSpans(): bool
 
         return true;
     }
+
+    private function hydrateRequestData(Request $request): void
+    {
+        $route = $request->route();
+
+        if ($route instanceof Route) {
+            $routeName = Integration::extractNameForRoute($route) ?? '<unlabeled transaction>';
+
+            $this->transaction->setName($routeName);
+            $this->transaction->setData([
+                'name' => $route->getName(),
+                'action' => $route->getActionName(),
+                'method' => $request->getMethod(),
+            ]);
+        }
+    }
+
+    private function hydrateResponseData(Response $response): void
+    {
+        $this->transaction->setHttpStatus($response->status());
+    }
 }