Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move request info tracking to the middleware #408

Merged
merged 3 commits into from Nov 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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());
}
}