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

Improve tracing #580

Merged
merged 11 commits into from Oct 12, 2022
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
5 changes: 3 additions & 2 deletions config/sentry.php
Expand Up @@ -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'),

];
114 changes: 17 additions & 97 deletions src/Sentry/Laravel/Integration.php
Expand Up @@ -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;
Expand All @@ -21,11 +22,6 @@ class Integration implements IntegrationInterface
*/
private static $transaction;

/**
* @var null|string
*/
private static $baseControllerNamespace;

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -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.
*
Expand All @@ -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.
*
Expand All @@ -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()
];
}

/**
Expand Down Expand Up @@ -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.
*
Expand Down
13 changes: 4 additions & 9 deletions src/Sentry/Laravel/ServiceProvider.php
Expand Up @@ -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',
];

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
99 changes: 89 additions & 10 deletions src/Sentry/Laravel/Tracing/EventHandler.php
Expand Up @@ -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;
Expand All @@ -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',
];

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

Expand Down