Skip to content

Commit

Permalink
Move request info tracking to the middleware (#408)
Browse files Browse the repository at this point in the history
* Hydrate request data in the middleware

* Trim away the leading \ in route action names

* Add changelog entry
  • Loading branch information
stayallive committed Nov 3, 2020
1 parent 23c14c3 commit be92d66
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 42 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/Laravel/Integration.php
Expand Up @@ -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 ?? '';

Expand Down
37 changes: 0 additions & 37 deletions src/Sentry/Laravel/Tracing/EventHandler.php
Expand Up @@ -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
{
Expand All @@ -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
];
Expand Down Expand Up @@ -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
*
Expand Down
33 changes: 30 additions & 3 deletions src/Sentry/Laravel/Tracing/Middleware.php
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}
}

0 comments on commit be92d66

Please sign in to comment.