From e4dfa9f02d0dbb196bce255f2e1691967fe30336 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 7 Oct 2022 14:20:44 +0200 Subject: [PATCH 01/12] Drop Laravel Lumen support (#579) --- CHANGELOG.md | 2 + README.md | 1 + composer.json | 3 + src/Sentry/Laravel/Integration.php | 62 --------------- src/Sentry/Laravel/ServiceProvider.php | 10 +-- src/Sentry/Laravel/Tracing/Middleware.php | 17 +--- .../Laravel/Tracing/ServiceProvider.php | 14 +--- test/Sentry/IntegrationTest.php | 79 ------------------- 8 files changed, 13 insertions(+), 175 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c09af0ed..074b1fe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Drop support for Laravel Lumen (#579) + ## 2.14.0 - Fix not listening to queue events because `QueueManager` is registered as `queue` in the container and not by it's class name (#568) diff --git a/README.md b/README.md index 30be5cbd..f1bd372d 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,7 @@ try { ## Laravel Version Compatibility +- Laravel Lumen is supported until `2.14.x` - Laravel `<= 4.2.x` is supported until `0.8.x` - Laravel `<= 5.7.x` on PHP `<= 7.0` is supported until `0.11.x` - Laravel `>= 5.x.x` on PHP `>= 7.1` is supported in all versions diff --git a/composer.json b/composer.json index e95d2a4e..cf18b33c 100644 --- a/composer.json +++ b/composer.json @@ -28,6 +28,9 @@ "symfony/psr-http-message-bridge": "^1.0 | ^2.0", "nyholm/psr7": "^1.0" }, + "conflict": { + "laravel/lumen-framework": "*" + }, "autoload": { "psr-0": { "Sentry\\Laravel\\": "src/" diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 905d3622..aabe8f15 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -167,68 +167,6 @@ public static function extractNameAndSourceForRoute(Route $route): array return [$routeName, $source]; } - /** - * Extract the readable name for a Lumen route. - * - * @param array $routeData The array of route data - * @param string $path The path of the request - * - * @return string - * - * @internal This helper is used in various places to extra meaninful info from a Lumen route data. - * @deprecated This will be removed in version 3.0, use `extractNameAndSourceForLumenRoute` instead. - */ - public static function extractNameForLumenRoute(array $routeData, string $path): string - { - return self::extractNameAndSourceForLumenRoute($routeData, $path)[0]; - } - - /** - * Extract the readable name for a Lumen route and the transaction source for where that route name came from. - * - * @param array $routeData The array of route data - * @param string $path The path of the request - * - * @return array{0: string, 1: \Sentry\Tracing\TransactionSource} - * - * @internal This helper is used in various places to extra meaninful info from a Lumen route data. - */ - public static function extractNameAndSourceForLumenRoute(array $routeData, string $path): array - { - $source = null; - $routeName = null; - - $route = $routeData[1] ?? []; - - // some.action (route name/alias) - if (!empty($route['as'])) { - $source = TransactionSource::component(); - $routeName = self::extractNameForNamedRoute($route['as']); - } - - // Some\Controller@someAction (controller action) - if (empty($routeName) && !empty($route['uses'])) { - $source = TransactionSource::component(); - $routeName = self::extractNameForActionRoute($route['uses']); - } - - // /some/{action} // Fallback to the route uri (with parameter placeholders) - if (empty($routeName) || $routeName === 'Closure') { - $routeUri = array_reduce( - array_keys($routeData[2]), - static function ($carry, $key) use ($routeData) { - return str_replace($routeData[2][$key], "{{$key}}", $carry); - }, - $path - ); - - $source = TransactionSource::url(); - $routeName = '/' . ltrim($routeUri, '/'); - } - - return [$routeName, $source]; - } - /** * Take a route name and return it only if it's a usable route name. * diff --git a/src/Sentry/Laravel/ServiceProvider.php b/src/Sentry/Laravel/ServiceProvider.php index 596c4cac..9edb5015 100644 --- a/src/Sentry/Laravel/ServiceProvider.php +++ b/src/Sentry/Laravel/ServiceProvider.php @@ -6,7 +6,6 @@ use Illuminate\Foundation\Application as Laravel; use Illuminate\Foundation\Http\Kernel as HttpKernel; use Illuminate\Log\LogManager; -use Laravel\Lumen\Application as Lumen; use RuntimeException; use Sentry\ClientBuilder; use Sentry\ClientBuilderInterface; @@ -48,10 +47,7 @@ public function boot(): void if ($this->hasDsnSet()) { $this->bindEvents(); - if ($this->app instanceof Lumen) { - $this->app->middleware(SetRequestMiddleware::class); - $this->app->middleware(SetRequestIpMiddleware::class); - } elseif ($this->app->bound(HttpKernelInterface::class)) { + if ($this->app->bound(HttpKernelInterface::class)) { /** @var \Illuminate\Foundation\Http\Kernel $httpKernel */ $httpKernel = $this->app->make(HttpKernelInterface::class); @@ -78,10 +74,6 @@ public function boot(): void */ public function register(): void { - if ($this->app instanceof Lumen) { - $this->app->configure(static::$abstract); - } - $this->mergeConfigFrom(__DIR__ . '/../../../config/sentry.php', static::$abstract); $this->configureAndRegisterClient(); diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php index 589d1353..460ec941 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -91,9 +91,8 @@ public function terminate($request, $response): void * @param float|null $timestamp The unix timestamp of the booted event, default to `microtime(true)` if not `null`. * * @return void - * @internal This method should only be invoked right after the application has finished "booting": - * For Laravel this is from the application `booted` callback. - * For Lumen this is right before returning from the `bootstrap/app.php` file. + * + * @internal This method should only be invoked right after the application has finished "booting". */ public function setBootedTimestamp(?float $timestamp = null): void { @@ -190,18 +189,6 @@ private function hydrateRequestData(Request $request): void 'action' => $route->getActionName(), 'method' => $request->getMethod(), ]); - } elseif (is_array($route) && count($route) === 3) { - [$transactionName, $transactionSource] = Integration::extractNameAndSourceForLumenRoute($route, $request->path()); - - $this->updateTransactionNameIfDefault($transactionName, $transactionSource); - - $action = $route[1] ?? []; - - $this->transaction->setData([ - 'name' => $action['as'] ?? null, - 'action' => $action['uses'] ?? 'Closure', - 'method' => $request->getMethod(), - ]); } $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/'), TransactionSource::url()); diff --git a/src/Sentry/Laravel/Tracing/ServiceProvider.php b/src/Sentry/Laravel/Tracing/ServiceProvider.php index 3235b3c4..336e6025 100644 --- a/src/Sentry/Laravel/Tracing/ServiceProvider.php +++ b/src/Sentry/Laravel/Tracing/ServiceProvider.php @@ -6,11 +6,9 @@ use Illuminate\Contracts\View\Engine; use Illuminate\Contracts\View\View; use Illuminate\Foundation\Http\Kernel as HttpKernel; -use Illuminate\Queue\QueueManager; use Illuminate\View\Engines\EngineResolver; use Illuminate\View\Factory as ViewFactory; use InvalidArgumentException; -use Laravel\Lumen\Application as Lumen; use Sentry\Laravel\BaseServiceProvider; use Sentry\Serializer\RepresentationSerializer; @@ -29,9 +27,7 @@ public function boot(): void $this->bindViewEngine($tracingConfig); - if ($this->app instanceof Lumen) { - $this->app->middleware(Middleware::class); - } elseif ($this->app->bound(HttpKernelInterface::class)) { + if ($this->app->bound(HttpKernelInterface::class)) { /** @var \Illuminate\Foundation\Http\Kernel $httpKernel */ $httpKernel = $this->app->make(HttpKernelInterface::class); @@ -55,11 +51,9 @@ public function register(): void return new BacktraceHelper($options, new RepresentationSerializer($options)); }); - if (!$this->app instanceof Lumen) { - $this->app->booted(function () { - $this->app->make(Middleware::class)->setBootedTimestamp(); - }); - } + $this->app->booted(function () { + $this->app->make(Middleware::class)->setBootedTimestamp(); + }); } private function bindEvents(array $tracingConfig): void diff --git a/test/Sentry/IntegrationTest.php b/test/Sentry/IntegrationTest.php index c116bfc2..34c98d27 100644 --- a/test/Sentry/IntegrationTest.php +++ b/test/Sentry/IntegrationTest.php @@ -162,83 +162,4 @@ public function testExtractingNameForRouteWithStrippedBaseNamespaceFromAction(): Integration::setControllersBaseNamespace(null); } - - public function testExtractingNameForLumenRouteWithName(): void - { - $route = [0, ['as' => $routeName = 'foo-bar'], []]; - - $this->assertSame($routeName, Integration::extractNameForLumenRoute($route, '/some-route')); - } - - public function testExtractingNameForLumenRouteWithAction(): void - { - $route = [0, ['uses' => $controller = 'SomeController@someAction'], []]; - - $this->assertSame($controller, Integration::extractNameForLumenRoute($route, '/some-route')); - } - - public function testExtractingNameForLumenRouteWithoutName(): void - { - $url = '/some-route'; - - $this->assertSame($url, Integration::extractNameForLumenRoute([0, [], []], $url)); - } - - public function testExtractingNameForLumenRouteWithParamInUrl(): void - { - $route = [1, [], ['param1' => 'foo']]; - - $url = '/foo/bar/baz'; - - $this->assertSame('/{param1}/bar/baz', Integration::extractNameForLumenRoute($route, $url)); - } - - public function testExtractingNameForLumenRouteWithParamsInUrl(): void - { - $route = [1, [], ['param1' => 'foo', 'param2' => 'bar']]; - - $url = '/foo/bar/baz'; - - $this->assertSame('/{param1}/{param2}/baz', Integration::extractNameForLumenRoute($route, $url)); - } - - public function testExtractingNameForLumenRouteWithActionAndName(): void - { - $route = [0, [ - 'as' => $routeName = 'foo-bar', - 'uses' => 'SomeController@someAction', - ], []]; - - $this->assertSame($routeName, Integration::extractNameForLumenRoute($route, '/some-route')); - } - - public function testExtractingNameForLumenRouteWithAutoGeneratedName(): void - { - // We fake a generated name here, Laravel generates them each starting with `generated::` - $route = [0, ['as' => 'generated::KoAePbpBofo01ey4'], []]; - - $url = '/some-route'; - - $this->assertSame($url, Integration::extractNameForLumenRoute($route, $url)); - } - - public function testExtractingNameForLumenRouteWithIncompleteGroupName(): void - { - $route = [0, ['as' => 'group-name.'], []]; - - $url = '/some-route'; - - $this->assertSame($url, Integration::extractNameForLumenRoute($route, $url)); - } - - public function testExtractingNameForLumenRouteWithStrippedBaseNamespaceFromAction(): void - { - Integration::setControllersBaseNamespace('BaseNamespace'); - - $route = [0, ['uses' => 'BaseNamespace\\SomeController@someAction'], []]; - - $this->assertSame('SomeController@someAction', Integration::extractNameForLumenRoute($route, '/some-route')); - - Integration::setControllersBaseNamespace(null); - } } From bfc40480f012a01ac0e9aaabad2e7c18dea7ba2e Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 7 Oct 2022 14:35:38 +0200 Subject: [PATCH 02/12] Drop Laravel 5.x support (#581) --- .github/workflows/ci.yaml | 40 ---- CHANGELOG.md | 2 + README.md | 15 +- composer.json | 8 +- src/Sentry/Laravel/Console/PublishCommand.php | 7 - src/Sentry/Laravel/Console/TestCommand.php | 13 +- src/Sentry/Laravel/EventHandler.php | 226 ++++-------------- .../Laravel/Http/SetRequestMiddleware.php | 29 +-- src/Sentry/Laravel/Integration.php | 3 +- src/Sentry/Laravel/Tracing/EventHandler.php | 94 ++------ test/Sentry/CommandInfoInBreadcrumbsTest.php | 13 - test/Sentry/IntegrationTest.php | 15 -- test/Sentry/Laravel/LogChannelTest.php | 14 -- test/Sentry/LaravelLogsInBreadcrumbsTest.php | 13 +- test/Sentry/SentryLaravelTestCase.php | 9 +- test/Sentry/SqlBindingsInBreadcrumbsTest.php | 26 +- ...readcrumbsWithOldConfigKeyDisabledTest.php | 14 +- ...BreadcrumbsWithOldConfigKeyEnabledTest.php | 16 +- test/Sentry/SqlQueriesInBreadcrumbsTest.php | 13 +- 19 files changed, 164 insertions(+), 406 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index cea3cf83..da978ff3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -21,14 +21,6 @@ jobs: php: [ "8.1", "8.0", "7.4", "7.3", "7.2" ] packages: # All versions below should be test on PHP ^7.1 (Sentry SDK requirement) - - { laravel: 5.1.*, testbench: 3.1.*, phpunit: 5.7.* } - - { laravel: 5.2.*, testbench: 3.2.*, phpunit: 5.7.* } - - { laravel: 5.3.*, testbench: 3.3.*, phpunit: 5.7.* } - - { laravel: 5.4.*, testbench: 3.4.*, phpunit: 5.7.* } - - { laravel: 5.5.*, testbench: 3.5.*, phpunit: 6.5.* } - - { laravel: 5.6.*, testbench: 3.6.*, phpunit: 7.5.* } - - { laravel: 5.7.*, testbench: 3.7.*, phpunit: 7.5.* } - - { laravel: 5.8.*, testbench: 3.8.*, phpunit: 7.5.* } - { laravel: ^6.0, testbench: 4.7.*, phpunit: 8.4.* } - { laravel: ^7.0, testbench: 5.1.*, phpunit: 8.4.* } @@ -48,43 +40,11 @@ jobs: - php: "7.2" packages: { laravel: ^8.0, testbench: ^6.0, phpunit: 9.3.* } - - php: "8.0" - packages: { laravel: 5.1.*, testbench: 3.1.*, phpunit: 5.7.* } - - php: "8.0" - packages: { laravel: 5.2.*, testbench: 3.2.*, phpunit: 5.7.* } - - php: "8.0" - packages: { laravel: 5.3.*, testbench: 3.3.*, phpunit: 5.7.* } - - php: "8.0" - packages: { laravel: 5.4.*, testbench: 3.4.*, phpunit: 5.7.* } - - php: "8.0" - packages: { laravel: 5.5.*, testbench: 3.5.*, phpunit: 6.5.* } - - php: "8.0" - packages: { laravel: 5.6.*, testbench: 3.6.*, phpunit: 7.5.* } - - php: "8.0" - packages: { laravel: 5.7.*, testbench: 3.7.*, phpunit: 7.5.* } - - php: "8.0" - packages: { laravel: 5.8.*, testbench: 3.8.*, phpunit: 7.5.* } - php: "8.0" packages: { laravel: ^6.0, testbench: 4.7.*, phpunit: 8.4.* } - php: "8.0" packages: { laravel: ^7.0, testbench: 5.1.*, phpunit: 8.4.* } - - php: "8.1" - packages: { laravel: 5.1.*, testbench: 3.1.*, phpunit: 5.7.* } - - php: "8.1" - packages: { laravel: 5.2.*, testbench: 3.2.*, phpunit: 5.7.* } - - php: "8.1" - packages: { laravel: 5.3.*, testbench: 3.3.*, phpunit: 5.7.* } - - php: "8.1" - packages: { laravel: 5.4.*, testbench: 3.4.*, phpunit: 5.7.* } - - php: "8.1" - packages: { laravel: 5.5.*, testbench: 3.5.*, phpunit: 6.5.* } - - php: "8.1" - packages: { laravel: 5.6.*, testbench: 3.6.*, phpunit: 7.5.* } - - php: "8.1" - packages: { laravel: 5.7.*, testbench: 3.7.*, phpunit: 7.5.* } - - php: "8.1" - packages: { laravel: 5.8.*, testbench: 3.8.*, phpunit: 7.5.* } - php: "8.1" packages: { laravel: ^6.0, testbench: 4.7.*, phpunit: 8.4.* } - php: "8.1" diff --git a/CHANGELOG.md b/CHANGELOG.md index 074b1fe0..d0bf7d37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased - Drop support for Laravel Lumen (#579) +- Drop support for Laravel 5.x (#581) +- Fix not setting the correct SDK ID and version when running the `sentry:test` command (#582) ## 2.14.0 diff --git a/README.md b/README.md index f1bd372d..29089b1c 100644 --- a/README.md +++ b/README.md @@ -78,16 +78,21 @@ try { ## Laravel Version Compatibility -- Laravel Lumen is supported until `2.14.x` -- Laravel `<= 4.2.x` is supported until `0.8.x` -- Laravel `<= 5.7.x` on PHP `<= 7.0` is supported until `0.11.x` -- Laravel `>= 5.x.x` on PHP `>= 7.1` is supported in all versions +The Laravel versions listed below are all currently supported: + - Laravel `>= 6.x.x` on PHP `>= 7.2` is supported starting from `1.2.0` - Laravel `>= 7.x.x` on PHP `>= 7.2` is supported starting from `1.7.0` - Laravel `>= 8.x.x` on PHP `>= 7.3` is supported starting from `1.9.0` - Laravel `>= 9.x.x` on PHP `>= 8.0` is supported starting from `2.11.0` -Please note that of version `>= 2.0.0` we require PHP Version `>= 7.2` because we are using our new [PHP SDK](https://github.com/getsentry/sentry-php) underneath. +Please note that starting with version `>= 2.0.0` we require PHP Version `>= 7.2` because we are using our new [PHP SDK](https://github.com/getsentry/sentry-php) underneath. + +The Laravel and Lumen version listed below were supported in previous versions: + +- Laravel `<= 4.2.x` is supported until `0.8.x` +- Laravel `<= 5.7.x` on PHP `<= 7.0` is supported until `0.11.x` +- Laravel `>= 5.x.x` on PHP `>= 7.1` is supported until `2.14.x` +- Laravel Lumen is supported until `2.14.x` ## Contributing to the SDK diff --git a/composer.json b/composer.json index cf18b33c..460310bd 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ ], "require": { "php": "^7.2 | ^8.0", - "illuminate/support": "5.0 - 5.8 | ^6.0 | ^7.0 | ^8.0 | ^9.0", + "illuminate/support": "^6.0 | ^7.0 | ^8.0 | ^9.0", "sentry/sentry": "^3.9", "sentry/sdk": "^3.1", "symfony/psr-http-message-bridge": "^1.0 | ^2.0", @@ -37,9 +37,9 @@ } }, "require-dev": { - "phpunit/phpunit": "^5.7 | ^6.5 | ^7.5 | ^8.4 | ^9.3", - "laravel/framework": "5.0 - 5.8 | ^6.0 | ^7.0 | ^8.0 | ^9.0", - "orchestra/testbench": "3.1 - 3.8 | ^4.7 | ^5.1 | ^6.0 | ^7.0", + "phpunit/phpunit": "^8.4 | ^9.3", + "laravel/framework": "^6.0 | ^7.0 | ^8.0 | ^9.0", + "orchestra/testbench": "^4.7 | ^5.1 | ^6.0 | ^7.0", "friendsofphp/php-cs-fixer": "^3.11", "mockery/mockery": "^1.3" }, diff --git a/src/Sentry/Laravel/Console/PublishCommand.php b/src/Sentry/Laravel/Console/PublishCommand.php index 04cd0d54..19f253ef 100644 --- a/src/Sentry/Laravel/Console/PublishCommand.php +++ b/src/Sentry/Laravel/Console/PublishCommand.php @@ -10,13 +10,6 @@ class PublishCommand extends Command { - /** - * Laravel 5.0.x: The name and signature of the console command. - * - * @var string - */ - protected $name = 'sentry:publish'; - /** * The name and signature of the console command. * diff --git a/src/Sentry/Laravel/Console/TestCommand.php b/src/Sentry/Laravel/Console/TestCommand.php index 7cf52127..67c60847 100644 --- a/src/Sentry/Laravel/Console/TestCommand.php +++ b/src/Sentry/Laravel/Console/TestCommand.php @@ -7,21 +7,16 @@ use Illuminate\Support\Str; use Psr\Log\AbstractLogger; use Sentry\ClientBuilder; +use Sentry\Laravel\Version; use Sentry\State\Hub; use Sentry\State\HubInterface; use Sentry\Tracing\SpanContext; use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; use Throwable; class TestCommand extends Command { - /** - * Laravel 5.0.x: The name and signature of the console command. - * - * @var string - */ - protected $name = 'sentry:test'; - /** * The name and signature of the console command. * @@ -99,6 +94,10 @@ public function handle(): int return 1; } + // Set the Laravel SDK identifier and version + $clientBuilder->setSdkIdentifier(Version::SDK_IDENTIFIER); + $clientBuilder->setSdkVersion(Version::SDK_VERSION); + // We set a logger so we can surface errors thrown internally by the SDK $clientBuilder->setLogger(new class($this) extends AbstractLogger { private $command; diff --git a/src/Sentry/Laravel/EventHandler.php b/src/Sentry/Laravel/EventHandler.php index 8db2bb1d..61fed658 100644 --- a/src/Sentry/Laravel/EventHandler.php +++ b/src/Sentry/Laravel/EventHandler.php @@ -3,24 +3,19 @@ namespace Sentry\Laravel; use Exception; -use Illuminate\Auth\Events\Authenticated; -use Illuminate\Console\Events\CommandFinished; -use Illuminate\Console\Events\CommandStarting; +use Illuminate\Auth\Events as AuthEvents; +use Illuminate\Console\Events as ConsoleEvents; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Database\Eloquent\Model; -use Illuminate\Database\Events\QueryExecuted; +use Illuminate\Database\Events as DatabaseEvents; use Illuminate\Http\Request; -use Illuminate\Log\Events\MessageLogged; -use Illuminate\Queue\Events\JobExceptionOccurred; -use Illuminate\Queue\Events\JobProcessed; -use Illuminate\Queue\Events\JobProcessing; -use Illuminate\Queue\Events\WorkerStopping; +use Illuminate\Log\Events as LogEvents; +use Illuminate\Queue\Events as QueueEvents; use Illuminate\Queue\QueueManager; -use Illuminate\Routing\Events\RouteMatched; -use Illuminate\Routing\Route; +use Illuminate\Routing\Events as RoutingEvents; use Laravel\Octane\Events as Octane; use Laravel\Sanctum\Events as Sanctum; use RuntimeException; @@ -36,17 +31,11 @@ class EventHandler * @var array */ protected static $eventHandlerMap = [ - 'router.matched' => 'routerMatched', // Until Laravel 5.1 - 'Illuminate\Routing\Events\RouteMatched' => 'routeMatched', // Since Laravel 5.2 - - 'illuminate.query' => 'query', // Until Laravel 5.1 - 'Illuminate\Database\Events\QueryExecuted' => 'queryExecuted', // Since Laravel 5.2 - - 'illuminate.log' => 'log', // Until Laravel 5.3 - 'Illuminate\Log\Events\MessageLogged' => 'messageLogged', // Since Laravel 5.4 - - 'Illuminate\Console\Events\CommandStarting' => 'commandStarting', // Since Laravel 5.5 - 'Illuminate\Console\Events\CommandFinished' => 'commandFinished', // Since Laravel 5.5 + LogEvents\MessageLogged::class => 'messageLogged', + RoutingEvents\RouteMatched::class => 'routeMatched', + DatabaseEvents\QueryExecuted::class => 'queryExecuted', + ConsoleEvents\CommandStarting::class => 'commandStarting', + ConsoleEvents\CommandFinished::class => 'commandFinished', ]; /** @@ -55,8 +44,8 @@ class EventHandler * @var array */ protected static $authEventHandlerMap = [ - 'Illuminate\Auth\Events\Authenticated' => 'authenticated', // Since Laravel 5.3 - 'Laravel\Sanctum\Events\TokenAuthenticated' => 'sanctumTokenAuthenticated', // Since Sanctum 2.13 + AuthEvents\Authenticated::class => 'authenticated', + Sanctum\TokenAuthenticated::class => 'sanctumTokenAuthenticated', // Since Sanctum 2.13 ]; /** @@ -65,10 +54,10 @@ class EventHandler * @var array */ protected static $queueEventHandlerMap = [ - 'Illuminate\Queue\Events\JobProcessed' => 'queueJobProcessed', // Since Laravel 5.2 - 'Illuminate\Queue\Events\JobProcessing' => 'queueJobProcessing', // Since Laravel 5.2 - 'Illuminate\Queue\Events\WorkerStopping' => 'queueWorkerStopping', // Since Laravel 5.2 - 'Illuminate\Queue\Events\JobExceptionOccurred' => 'queueJobExceptionOccurred', // Since Laravel 5.2 + QueueEvents\JobProcessed::class => 'queueJobProcessed', + QueueEvents\JobProcessing::class => 'queueJobProcessing', + QueueEvents\WorkerStopping::class => 'queueWorkerStopping', + QueueEvents\JobExceptionOccurred::class => 'queueJobExceptionOccurred', ]; /** @@ -77,17 +66,17 @@ class EventHandler * @var array */ protected static $octaneEventHandlerMap = [ - 'Laravel\Octane\Events\RequestReceived' => 'octaneRequestReceived', - 'Laravel\Octane\Events\RequestTerminated' => 'octaneRequestTerminated', + Octane\RequestReceived::class => 'octaneRequestReceived', + Octane\RequestTerminated::class => 'octaneRequestTerminated', - 'Laravel\Octane\Events\TaskReceived' => 'octaneTaskReceived', - 'Laravel\Octane\Events\TaskTerminated' => 'octaneTaskTerminated', + Octane\TaskReceived::class => 'octaneTaskReceived', + Octane\TaskTerminated::class => 'octaneTaskTerminated', - 'Laravel\Octane\Events\TickReceived' => 'octaneTickReceived', - 'Laravel\Octane\Events\TickTerminated' => 'octaneTickTerminated', + Octane\TickReceived::class => 'octaneTickReceived', + Octane\TickTerminated::class => 'octaneTickTerminated', - 'Laravel\Octane\Events\WorkerErrorOccurred' => 'octaneWorkerErrorOccurred', - 'Laravel\Octane\Events\WorkerStopping' => 'octaneWorkerStopping', + Octane\WorkerErrorOccurred::class => 'octaneWorkerErrorOccurred', + Octane\WorkerStopping::class => 'octaneWorkerStopping', ]; /** @@ -261,7 +250,7 @@ public function subscribeQueueEvents(QueueManager $queue): void * @param string $method * @param array $arguments */ - public function __call($method, $arguments) + public function __call(string $method, array $arguments) { $handlerMethod = "{$method}Handler"; @@ -276,14 +265,9 @@ public function __call($method, $arguments) } } - /** - * Until Laravel 5.1 - * - * @param Route $route - */ - protected function routerMatchedHandler(Route $route) + protected function routeMatchedHandler(RoutingEvents\RouteMatched $match): void { - [$routeName] = Integration::extractNameAndSourceForRoute($route); + [$routeName] = Integration::extractNameAndSourceForRoute($match->route); Integration::addBreadcrumb(new Breadcrumb( Breadcrumb::LEVEL_INFO, @@ -295,106 +279,32 @@ protected function routerMatchedHandler(Route $route) Integration::setTransaction($routeName); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Routing\Events\RouteMatched $match - */ - protected function routeMatchedHandler(RouteMatched $match) - { - $this->routerMatchedHandler($match->route); - } - - /** - * Until Laravel 5.1 - * - * @param string $query - * @param array $bindings - * @param int $time - * @param string $connectionName - */ - protected function queryHandler($query, $bindings, $time, $connectionName) + protected function queryExecutedHandler(DatabaseEvents\QueryExecuted $query): void { if (!$this->recordSqlQueries) { return; } - $this->addQueryBreadcrumb($query, $bindings, $time, $connectionName); - } + $data = ['connectionName' => $query->connectionName]; - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Database\Events\QueryExecuted $query - */ - protected function queryExecutedHandler(QueryExecuted $query) - { - if (!$this->recordSqlQueries) { - return; - } - - $this->addQueryBreadcrumb($query->sql, $query->bindings, $query->time, $query->connectionName); - } - - /** - * Helper to add an query breadcrumb. - * - * @param string $query - * @param array $bindings - * @param float|null $time - * @param string $connectionName - */ - private function addQueryBreadcrumb($query, $bindings, $time, $connectionName) - { - $data = ['connectionName' => $connectionName]; - - if ($time !== null) { - $data['executionTimeMs'] = $time; + if ($query->time !== null) { + $data['executionTimeMs'] = $query->time; } if ($this->recordSqlBindings) { - $data['bindings'] = $bindings; + $data['bindings'] = $query->bindings; } Integration::addBreadcrumb(new Breadcrumb( Breadcrumb::LEVEL_INFO, Breadcrumb::TYPE_DEFAULT, 'sql.query', - $query, + $query->sql, $data )); } - /** - * Until Laravel 5.3 - * - * @param string $level - * @param string $message - * @param array|null $context - */ - protected function logHandler($level, $message, $context) - { - $this->addLogBreadcrumb($level, $message, is_array($context) ? $context : []); - } - - /** - * Since Laravel 5.4 - * - * @param \Illuminate\Log\Events\MessageLogged $logEntry - */ - protected function messageLoggedHandler(MessageLogged $logEntry) - { - $this->addLogBreadcrumb($logEntry->level, $logEntry->message, $logEntry->context); - } - - /** - * Helper to add an log breadcrumb. - * - * @param string $level Log level. May be any standard. - * @param string|null $message Log message. - * @param array $context Log context. - */ - private function addLogBreadcrumb(string $level, ?string $message, array $context = []): void + protected function messageLoggedHandler(LogEvents\MessageLogged $logEntry): void { if (!$this->recordLaravelLogs) { return; @@ -403,35 +313,25 @@ private function addLogBreadcrumb(string $level, ?string $message, array $contex // A log message with `null` as value will not be recorded by Laravel // however empty strings are logged so we mimick that behaviour to // check for `null` to stay consistent with how Laravel logs it - if ($message === null) { + if ($logEntry->message === null) { return; } Integration::addBreadcrumb(new Breadcrumb( - $this->logLevelToBreadcrumbLevel($level), + $this->logLevelToBreadcrumbLevel($logEntry->level), Breadcrumb::TYPE_DEFAULT, - 'log.' . $level, - $message, - $context + 'log.' . $logEntry->level, + $logEntry->message, + $logEntry->context )); } - /** - * Since Laravel 5.3 - * - * @param \Illuminate\Auth\Events\Authenticated $event - */ - protected function authenticatedHandler(Authenticated $event) + protected function authenticatedHandler(AuthEvents\Authenticated $event): void { $this->configureUserScopeFromModel($event->user); } - /** - * Since Sanctum 2.13 - * - * @param \Laravel\Sanctum\Events\TokenAuthenticated $event - */ - protected function sanctumTokenAuthenticatedHandler(Sanctum\TokenAuthenticated $event) + protected function sanctumTokenAuthenticatedHandler(Sanctum\TokenAuthenticated $event): void { $this->configureUserScopeFromModel($event->token->tokenable); } @@ -478,12 +378,7 @@ private function configureUserScopeFromModel($authUser): void }); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\JobProcessing $event - */ - protected function queueJobProcessingHandler(JobProcessing $event) + protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void { $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScope); @@ -516,43 +411,23 @@ protected function queueJobProcessingHandler(JobProcessing $event) )); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\JobExceptionOccurred $event - */ - protected function queueJobExceptionOccurredHandler(JobExceptionOccurred $event) + protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event): void { $this->afterTaskWithinLongRunningProcess(); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\JobProcessed $event - */ - protected function queueJobProcessedHandler(JobProcessed $event) + protected function queueJobProcessedHandler(QueueEvents\JobProcessed $event): void { $this->afterTaskWithinLongRunningProcess(); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\WorkerStopping $event - */ - protected function queueWorkerStoppingHandler(WorkerStopping $event) + protected function queueWorkerStoppingHandler(QueueEvents\WorkerStopping $event): void { // Flush any and all events that were possibly generated by queue jobs Integration::flushEvents(); } - /** - * Since Laravel 5.5 - * - * @param \Illuminate\Console\Events\CommandStarting $event - */ - protected function commandStartingHandler(CommandStarting $event) + protected function commandStartingHandler(ConsoleEvents\CommandStarting $event): void { if ($event->command) { Integration::configureScope(static function (Scope $scope) use ($event): void { @@ -575,12 +450,7 @@ protected function commandStartingHandler(CommandStarting $event) } } - /** - * Since Laravel 5.5 - * - * @param \Illuminate\Console\Events\CommandFinished $event - */ - protected function commandFinishedHandler(CommandFinished $event) + protected function commandFinishedHandler(ConsoleEvents\CommandFinished $event): void { if ($this->recordCommandInfo) { Integration::addBreadcrumb(new Breadcrumb( diff --git a/src/Sentry/Laravel/Http/SetRequestMiddleware.php b/src/Sentry/Laravel/Http/SetRequestMiddleware.php index bbdfca4e..ffa85e6a 100644 --- a/src/Sentry/Laravel/Http/SetRequestMiddleware.php +++ b/src/Sentry/Laravel/Http/SetRequestMiddleware.php @@ -4,11 +4,10 @@ use Closure; use Illuminate\Container\Container; +use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Http\Request; -use Nyholm\Psr7\Factory\Psr17Factory; use Psr\Http\Message\ServerRequestInterface; use Sentry\State\HubInterface; -use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory; /** * This middleware caches a PSR-7 version of the request as early as possible. @@ -21,7 +20,7 @@ public function handle(Request $request, Closure $next) $container = Container::getInstance(); if ($container->bound(HubInterface::class)) { - $psrRequest = $this->resolvePsrRequest($request); + $psrRequest = $this->resolvePsrRequest($container); if ($psrRequest !== null) { $container->instance(LaravelRequestFetcher::CONTAINER_PSR7_INSTANCE_KEY, $psrRequest); @@ -31,26 +30,12 @@ public function handle(Request $request, Closure $next) return $next($request); } - /** - * This code was copied from the Laravel codebase which was introduced in Laravel 6. - * - * The reason we have it copied here is because older (<6.0) versions of Laravel use a different - * method to construct the PSR-7 request object which requires other packages to create that object - * but most importantly it does not function when those packages are not available resulting in errors - * - * So long story short, this is here to backport functionality to Laravel <6.0 - * if we drop support for those versions in the future we can reconsider this and - * move back to using the container binding provided by Laravel for the PSR-7 object - * - * @see https://github.com/laravel/framework/blob/cb550b5bdc2b2c4cf077082adabde0144a72d190/src/Illuminate/Routing/RoutingServiceProvider.php#L127-L146 - */ - private function resolvePsrRequest(Request $request): ?ServerRequestInterface + private function resolvePsrRequest(Container $container): ?ServerRequestInterface { - if (class_exists(Psr17Factory::class) && class_exists(PsrHttpFactory::class)) { - $psr17Factory = new Psr17Factory; - - return (new PsrHttpFactory($psr17Factory, $psr17Factory, $psr17Factory, $psr17Factory)) - ->createRequest($request); + try { + return $container->make(ServerRequestInterface::class); + } catch (BindingResolutionException $e) { + // This happens if Laravel doesn't have the correct classes available to construct the PSR-7 object } return null; diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index aabe8f15..49cd62e8 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -211,8 +211,7 @@ private static function extractNameForActionRoute(string $action): string } // Strip away the base namespace from the action name - // @see: Str::after, but this is not available before Laravel 5.4 so we use a inlined version - return array_reverse(explode($baseNamespace . '\\', $routeName, 2))[0]; + return ltrim(Str::after($routeName, $baseNamespace), '\\'); } /** diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index 1fafb3bd..bcba08b7 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -30,8 +30,7 @@ class EventHandler * @var array */ protected static $eventHandlerMap = [ - 'illuminate.query' => 'query', // Until Laravel 5.1 - DatabaseEvents\QueryExecuted::class => 'queryExecuted', // Since Laravel 5.2 + DatabaseEvents\QueryExecuted::class => 'queryExecuted', ]; /** @@ -40,9 +39,9 @@ class EventHandler * @var array */ protected static $queueEventHandlerMap = [ - QueueEvents\JobProcessing::class => 'queueJobProcessing', // Since Laravel 5.2 - QueueEvents\JobProcessed::class => 'queueJobProcessed', // Since Laravel 5.2 - QueueEvents\JobExceptionOccurred::class => 'queueJobExceptionOccurred', // Since Laravel 5.2 + QueueEvents\JobProcessing::class => 'queueJobProcessing', + QueueEvents\JobProcessed::class => 'queueJobProcessed', + QueueEvents\JobExceptionOccurred::class => 'queueJobExceptionOccurred', ]; /** @@ -122,6 +121,8 @@ public function __construct(Container $container, BacktraceHelper $backtraceHelp /** * Attach all event handlers. + * + * @uses self::queryExecutedHandler() */ public function subscribe(): void { @@ -141,6 +142,10 @@ public function subscribe(): void * Attach all queue event handlers. * * @param \Illuminate\Queue\QueueManager $queue + * + * @uses self::queueJobProcessingHandler() + * @uses self::queueJobProcessedHandler() + * @uses self::queueJobExceptionOccurredHandler() */ public function subscribeQueueEvents(QueueManager $queue): void { @@ -149,19 +154,16 @@ public function subscribeQueueEvents(QueueManager $queue): void return; } - // The payload create callback was introduced in Laravel 5.7 so we need to guard against older versions - if (method_exists(Queue::class, 'createPayloadUsing')) { - Queue::createPayloadUsing(static function (?string $connection, ?string $queue, ?array $payload): ?array { - $currentSpan = Integration::currentTracingSpan(); + Queue::createPayloadUsing(static function (?string $connection, ?string $queue, ?array $payload): ?array { + $currentSpan = Integration::currentTracingSpan(); - if ($currentSpan !== null && $payload !== null) { - $payload[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] = $currentSpan->toTraceparent(); - $payload[self::QUEUE_PAYLOAD_BAGGAGE_DATA] = $currentSpan->toBaggage(); - } + if ($currentSpan !== null && $payload !== null) { + $payload[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] = $currentSpan->toTraceparent(); + $payload[self::QUEUE_PAYLOAD_BAGGAGE_DATA] = $currentSpan->toBaggage(); + } - return $payload; - }); - } + return $payload; + }); $queue->looping(function () { $this->afterQueuedJob(); @@ -185,7 +187,7 @@ public function subscribeQueueEvents(QueueManager $queue): void * @param string $method * @param array $arguments */ - public function __call($method, $arguments) + public function __call(string $method, array $arguments) { $handlerMethod = "{$method}Handler"; @@ -200,36 +202,7 @@ public function __call($method, $arguments) } } - /** - * Until Laravel 5.1 - * - * @param string $query - * @param array $bindings - * @param int $time - * @param string $connectionName - */ - protected function queryHandler($query, $bindings, $time, $connectionName): void - { - $this->recordQuerySpan($query, $time); - } - - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Database\Events\QueryExecuted $query - */ protected function queryExecutedHandler(DatabaseEvents\QueryExecuted $query): void - { - $this->recordQuerySpan($query->sql, $query->time); - } - - /** - * Helper to add an query breadcrumb. - * - * @param string $query - * @param float|null $time - */ - private function recordQuerySpan($query, $time): void { if (!$this->traceSqlQueries) { return; @@ -244,12 +217,12 @@ private function recordQuerySpan($query, $time): void $context = new SpanContext(); $context->setOp('db.sql.query'); - $context->setDescription($query); - $context->setStartTimestamp(microtime(true) - $time / 1000); - $context->setEndTimestamp($context->getStartTimestamp() + $time / 1000); + $context->setDescription($query->sql); + $context->setStartTimestamp(microtime(true) - $query->time / 1000); + $context->setEndTimestamp($context->getStartTimestamp() + $query->time / 1000); if ($this->traceSqlQueryOrigins) { - $queryOrigin = $this->resolveQueryOriginFromBacktrace($context); + $queryOrigin = $this->resolveQueryOriginFromBacktrace(); if ($queryOrigin !== null) { $context->setData(['sql.origin' => $queryOrigin]); @@ -277,12 +250,7 @@ private function resolveQueryOriginFromBacktrace(): ?string return "{$filePath}:{$firstAppFrame->getLine()}"; } - /* - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\JobProcessing $event - */ - protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event) + protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void { $parentSpan = Integration::currentTracingSpan(); @@ -347,22 +315,12 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event) SentrySdk::getCurrentHub()->setSpan($this->currentQueueJobSpan); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\JobExceptionOccurred $event - */ - protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event) + protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event): void { $this->afterQueuedJob(SpanStatus::internalError()); } - /** - * Since Laravel 5.2 - * - * @param \Illuminate\Queue\Events\JobProcessed $event - */ - protected function queueJobProcessedHandler(QueueEvents\JobProcessed $event) + protected function queueJobProcessedHandler(QueueEvents\JobProcessed $event): void { $this->afterQueuedJob(SpanStatus::ok()); } diff --git a/test/Sentry/CommandInfoInBreadcrumbsTest.php b/test/Sentry/CommandInfoInBreadcrumbsTest.php index f0b0a61c..0a7929f2 100644 --- a/test/Sentry/CommandInfoInBreadcrumbsTest.php +++ b/test/Sentry/CommandInfoInBreadcrumbsTest.php @@ -10,10 +10,6 @@ class CommandInfoInBreadcrumbsTest extends SentryLaravelTestCase { public function testCommandInfoAreRecordedWhenEnabled() { - if ($this->shouldSkip()) { - $this->markTestSkipped('Laravel version <5.5 does not contain the events tested.'); - } - $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.command_info' => true, ]); @@ -30,10 +26,6 @@ public function testCommandInfoAreRecordedWhenEnabled() public function testCommandInfoAreRecordedWhenDisabled() { - if ($this->shouldSkip()) { - $this->markTestSkipped('Laravel version <5.5 does not contain the events tested.'); - } - $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.command_info' => false, ]); @@ -60,9 +52,4 @@ private function dispatchCommandStartEvent() ) ); } - - private function shouldSkip() - { - return !class_exists(CommandStarting::class); - } } diff --git a/test/Sentry/IntegrationTest.php b/test/Sentry/IntegrationTest.php index 34c98d27..2365931b 100644 --- a/test/Sentry/IntegrationTest.php +++ b/test/Sentry/IntegrationTest.php @@ -22,10 +22,6 @@ public function testIntegrationIsRegistered(): void public function testTransactionIsSetWhenRouteMatchedEventIsFired(): void { - if (!class_exists(RouteMatched::class)) { - $this->markTestSkipped('RouteMatched event class does not exist on this version of Laravel.'); - } - Integration::setTransaction(null); $event = new RouteMatched( @@ -38,17 +34,6 @@ public function testTransactionIsSetWhenRouteMatchedEventIsFired(): void $this->assertSame($routeUrl, Integration::getTransaction()); } - public function testTransactionIsSetWhenRouterMatchedEventIsFired(): void - { - Integration::setTransaction(null); - - $this->dispatchLaravelEvent('router.matched', [ - new Route('GET', $routeUrl = '/sentry-router-matched-event', []), - ]); - - $this->assertSame($routeUrl, Integration::getTransaction()); - } - public function testTransactionIsAppliedToEventWithoutTransaction(): void { Integration::setTransaction($transaction = 'some-transaction-name'); diff --git a/test/Sentry/Laravel/LogChannelTest.php b/test/Sentry/Laravel/LogChannelTest.php index 26727660..e0ecc850 100644 --- a/test/Sentry/Laravel/LogChannelTest.php +++ b/test/Sentry/Laravel/LogChannelTest.php @@ -2,7 +2,6 @@ namespace Sentry\Laravel\Tests\Sentry\Laravel; -use Illuminate\Log\LogManager; use Monolog\Handler\FingersCrossedHandler; use Sentry\Laravel\LogChannel; use Sentry\Laravel\SentryHandler; @@ -12,8 +11,6 @@ class LogChannelTest extends SentryLaravelTestCase { public function test_creating_handler_without_action_level_config() { - $this->skipIfLogManagerNotAvailable(); - $logChannel = new LogChannel($this->app); $logger = $logChannel([]); @@ -22,8 +19,6 @@ public function test_creating_handler_without_action_level_config() public function test_creating_handler_with_action_level_config() { - $this->skipIfLogManagerNotAvailable(); - $logChannel = new LogChannel($this->app); $logger = $logChannel(['action_level' => 'critical']); @@ -36,13 +31,4 @@ public function test_creating_handler_with_action_level_config() $this->assertContainsOnlyInstancesOf(SentryHandler::class, $loggerWithoutActionLevel->getHandlers()); } - - private function skipIfLogManagerNotAvailable() - { - if (class_exists(LogManager::class)) { - return; - } - - $this->markTestSkipped('Laravel version <=5.5 does not contain the LogManager required for this functionality.'); - } } diff --git a/test/Sentry/LaravelLogsInBreadcrumbsTest.php b/test/Sentry/LaravelLogsInBreadcrumbsTest.php index 6fa09ebd..724bec96 100644 --- a/test/Sentry/LaravelLogsInBreadcrumbsTest.php +++ b/test/Sentry/LaravelLogsInBreadcrumbsTest.php @@ -2,9 +2,12 @@ namespace Sentry\Laravel\Tests; +use Illuminate\Log\Events\MessageLogged; +use Mockery; + class LaravelLogsInBreadcrumbsTest extends SentryLaravelTestCase { - public function testLaravelLogsAreRecordedWhenEnabled() + public function testLaravelLogsAreRecordedWhenEnabled(): void { $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.logs' => true, @@ -12,11 +15,11 @@ public function testLaravelLogsAreRecordedWhenEnabled() $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.logs')); - $this->dispatchLaravelEvent('illuminate.log', [ + $this->dispatchLaravelEvent(new MessageLogged( $level = 'debug', $message = 'test message', - $context = ['1'], - ]); + $context = ['1'] + )); $lastBreadcrumb = $this->getLastBreadcrumb(); @@ -25,7 +28,7 @@ public function testLaravelLogsAreRecordedWhenEnabled() $this->assertEquals($context, $lastBreadcrumb->getMetadata()); } - public function testLaravelLogsAreRecordedWhenDisabled() + public function testLaravelLogsAreRecordedWhenDisabled(): void { $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.logs' => false, diff --git a/test/Sentry/SentryLaravelTestCase.php b/test/Sentry/SentryLaravelTestCase.php index eab9b670..fdb72835 100644 --- a/test/Sentry/SentryLaravelTestCase.php +++ b/test/Sentry/SentryLaravelTestCase.php @@ -42,14 +42,9 @@ protected function resetApplicationWithConfig(array $config) $this->refreshApplication(); } - protected function dispatchLaravelEvent($event, array $payload = []) + protected function dispatchLaravelEvent($event, array $payload = []): void { - $dispatcher = $this->app['events']; - - // Laravel 5.4+ uses the dispatch method to dispatch/fire events - return method_exists($dispatcher, 'dispatch') - ? $dispatcher->dispatch($event, $payload) - : $dispatcher->fire($event, $payload); + $this->app['events']->dispatch($event, $payload); } protected function getHubFromContainer(): HubInterface diff --git a/test/Sentry/SqlBindingsInBreadcrumbsTest.php b/test/Sentry/SqlBindingsInBreadcrumbsTest.php index 65affa33..e8e09300 100644 --- a/test/Sentry/SqlBindingsInBreadcrumbsTest.php +++ b/test/Sentry/SqlBindingsInBreadcrumbsTest.php @@ -2,9 +2,13 @@ namespace Sentry\Laravel\Tests; +use Illuminate\Database\Connection; +use Illuminate\Database\Events\QueryExecuted; +use Mockery; + class SqlBindingsInBreadcrumbsTest extends SentryLaravelTestCase { - public function testSqlBindingsAreRecordedWhenEnabled() + public function testSqlBindingsAreRecordedWhenEnabled(): void { $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.sql_bindings' => true, @@ -12,12 +16,15 @@ public function testSqlBindingsAreRecordedWhenEnabled() $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.sql_bindings')); - $this->dispatchLaravelEvent('illuminate.query', [ + $connection = Mockery::mock(Connection::class) + ->shouldReceive('getName')->andReturn('test'); + + $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', $bindings = ['1'], 10, - 'test', - ]); + $connection + )); $lastBreadcrumb = $this->getLastBreadcrumb(); @@ -25,7 +32,7 @@ public function testSqlBindingsAreRecordedWhenEnabled() $this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']); } - public function testSqlBindingsAreRecordedWhenDisabled() + public function testSqlBindingsAreRecordedWhenDisabled(): void { $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.sql_bindings' => false, @@ -33,12 +40,15 @@ public function testSqlBindingsAreRecordedWhenDisabled() $this->assertFalse($this->app['config']->get('sentry.breadcrumbs.sql_bindings')); - $this->dispatchLaravelEvent('illuminate.query', [ + $connection = Mockery::mock(Connection::class) + ->shouldReceive('getName')->andReturn('test'); + + $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings <> ?;', ['1'], 10, - 'test', - ]); + $connection + )); $lastBreadcrumb = $this->getLastBreadcrumb(); diff --git a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php index c60295bf..3b97bfc1 100644 --- a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php +++ b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php @@ -3,6 +3,9 @@ namespace Sentry\Laravel\Tests; use Illuminate\Config\Repository; +use Illuminate\Database\Connection; +use Illuminate\Database\Events\QueryExecuted; +use Mockery; class SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest extends SentryLaravelTestCase { @@ -17,16 +20,19 @@ protected function getEnvironmentSetUp($app) $app['config'] = new Repository($config); } - public function testSqlBindingsAreRecordedWhenDisabledByOldConfigKey() + public function testSqlBindingsAreRecordedWhenDisabledByOldConfigKey(): void { $this->assertFalse($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']); - $this->dispatchLaravelEvent('illuminate.query', [ + $connection = Mockery::mock(Connection::class) + ->shouldReceive('getName')->andReturn('test'); + + $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', ['1'], 10, - 'test', - ]); + $connection + )); $lastBreadcrumb = $this->getLastBreadcrumb(); diff --git a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php index 3850ac32..f2a15f68 100644 --- a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php +++ b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php @@ -3,6 +3,9 @@ namespace Sentry\Laravel\Tests; use Illuminate\Config\Repository; +use Illuminate\Database\Connection; +use Illuminate\Database\Events\QueryExecuted; +use Mockery; class SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest extends SentryLaravelTestCase { @@ -17,16 +20,21 @@ protected function getEnvironmentSetUp($app) $app['config'] = new Repository($config); } - public function testSqlBindingsAreRecordedWhenEnabledByOldConfigKey() + public function testSqlBindingsAreRecordedWhenEnabledByOldConfigKey(): void { $this->assertTrue($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']); - $this->dispatchLaravelEvent('illuminate.query', [ + $connection = Mockery::mock(Connection::class) + ->shouldReceive('getName')->andReturn('test'); + + + + $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', $bindings = ['1'], 10, - 'test', - ]); + $connection + )); $lastBreadcrumb = $this->getLastBreadcrumb(); diff --git a/test/Sentry/SqlQueriesInBreadcrumbsTest.php b/test/Sentry/SqlQueriesInBreadcrumbsTest.php index 2d718e95..6a26b506 100644 --- a/test/Sentry/SqlQueriesInBreadcrumbsTest.php +++ b/test/Sentry/SqlQueriesInBreadcrumbsTest.php @@ -2,6 +2,10 @@ namespace Sentry\Laravel\Tests; +use Illuminate\Database\Connection; +use Illuminate\Database\Events\QueryExecuted; +use Mockery; + class SqlQueriesInBreadcrumbsTest extends SentryLaravelTestCase { public function testSqlQueriesAreRecordedWhenEnabled() @@ -12,12 +16,15 @@ public function testSqlQueriesAreRecordedWhenEnabled() $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.sql_queries')); - $this->dispatchLaravelEvent('illuminate.query', [ + $connection = Mockery::mock(Connection::class) + ->shouldReceive('getName')->andReturn('test'); + + $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', ['1'], 10, - 'test', - ]); + $connection + )); $lastBreadcrumb = $this->getLastBreadcrumb(); From 226c7a64711e6d4e31164be008b5011d56bf0172 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 7 Oct 2022 14:49:12 +0200 Subject: [PATCH 03/12] Update branch aliasses --- composer.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 460310bd..65d7186c 100644 --- a/composer.json +++ b/composer.json @@ -64,7 +64,9 @@ }, "extra": { "branch-alias": { - "dev-master": "2.x-dev", + "dev-3.x": "3.x-dev", + "dev-2.x": "2.x-dev", + "dev-1.x": "1.x-dev", "dev-0.x": "0.x-dev" }, "laravel": { From 86f793f817ee8ad359fd42af4b481590ce600512 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 6 Oct 2022 23:26:14 +0200 Subject: [PATCH 04/12] Fix not setting correct SDK ID/version when running test command (#582) # Conflicts: # CHANGELOG.md From ca28be0bfe580c8447ff54418d1322d42690260c Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 11 Oct 2022 14:00:40 +0200 Subject: [PATCH 05/12] Revert "Merge branch 'master' into 3.x" This reverts commit 9fdf6290a4b5fdfa37a94282642ca44c10be25d2, reversing changes made to 3e6e5bebe3e09ed649ce539cba90336df85e9551. --- src/Sentry/Laravel/Console/TestCommand.php | 1 + src/Sentry/Laravel/Integration.php | 48 +++++++++++++++++-- src/Sentry/Laravel/Tracing/EventHandler.php | 9 ++-- .../Integrations/LighthouseIntegration.php | 4 +- src/Sentry/Laravel/Tracing/Middleware.php | 20 ++++---- 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/Sentry/Laravel/Console/TestCommand.php b/src/Sentry/Laravel/Console/TestCommand.php index cb6709cb..67c60847 100644 --- a/src/Sentry/Laravel/Console/TestCommand.php +++ b/src/Sentry/Laravel/Console/TestCommand.php @@ -139,6 +139,7 @@ public function log($level, $message, array $context = []): void $transactionContext = new TransactionContext(); $transactionContext->setSampled(true); $transactionContext->setName('Sentry Test Transaction'); + $transactionContext->setSource(TransactionSource::custom()); $transactionContext->setOp('sentry.test'); $transaction = $hub->startTransaction($transactionContext); diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 6dcecc7d..49cd62e8 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -6,6 +6,7 @@ use Illuminate\Support\Str; use Sentry\SentrySdk; use Sentry\Tracing\Span; +use Sentry\Tracing\TransactionSource; use function Sentry\addBreadcrumb; use function Sentry\configureScope; use Sentry\Breadcrumb; @@ -122,27 +123,48 @@ public static function flushEvents(): void * @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. + * + * @param \Illuminate\Routing\Route $route + * + * @return array{0: string, 1: \Sentry\Tracing\TransactionSource} + * + * @internal This helper is used in various places to extra meaninful info from a Laravel Route object. + */ + public static function extractNameAndSourceForRoute(Route $route): array + { + $source = null; $routeName = null; - // someaction (route name/alias) + // 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()); } - // /someaction // Fallback to the url + // /some/{action} // Fallback to the route uri (with parameter placeholders) if (empty($routeName) || $routeName === 'Closure') { + $source = TransactionSource::route(); $routeName = '/' . ltrim($route->uri(), '/'); } - return $routeName; + return [$routeName, $source]; } /** @@ -206,7 +228,25 @@ public static function sentryTracingMeta(): string } $content = sprintf('', $span->toTraceparent()); - // $content .= sprintf('', $span->getDescription()); + + return $content; + } + + /** + * Retrieve the meta tags with baggage information to link this request to front-end requests. + * This propagates the Dynamic Sampling Context. + * + * @return string + */ + public static function sentryBaggageMeta(): string + { + $span = self::currentTracingSpan(); + + if ($span === null) { + return ''; + } + + $content = sprintf('', $span->toBaggage()); return $content; } diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index ad066c5e..bcba08b7 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -16,11 +16,14 @@ use Sentry\Tracing\SpanContext; use Sentry\Tracing\SpanStatus; use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; class EventHandler { public const QUEUE_PAYLOAD_TRACE_PARENT_DATA = 'sentry_trace_parent_data'; + public const QUEUE_PAYLOAD_BAGGAGE_DATA = 'sentry_baggage_data'; + /** * Map event handlers to events. * @@ -262,11 +265,10 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): } if ($parentSpan === null) { + $baggage = $event->job->payload()[self::QUEUE_PAYLOAD_BAGGAGE_DATA] ?? null; $traceParent = $event->job->payload()[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] ?? null; - $context = $traceParent === null - ? new TransactionContext - : TransactionContext::fromSentryTrace($traceParent); + $context = TransactionContext::fromHeaders($traceParent ?? '', $baggage ?? ''); // If the parent transaction was not sampled we also stop the queue job from being recorded if ($context->getParentSampled() === false) { @@ -294,6 +296,7 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): if ($context instanceof TransactionContext) { $context->setName($resolvedJobName ?? $event->job->getName()); + $context->setSource(TransactionSource::task()); } $context->setOp('queue.process'); diff --git a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php index a05f113c..dc443a35 100644 --- a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php +++ b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php @@ -13,6 +13,7 @@ use Sentry\Laravel\Integration; use Sentry\SentrySdk; use Sentry\Tracing\SpanContext; +use Sentry\Tracing\TransactionSource; class LighthouseIntegration implements IntegrationInterface { @@ -157,7 +158,7 @@ private function updateTransactionName(): void return; } - array_walk($groupedOperations, static function (array &$operations, string $operationType) { + array_walk($groupedOperations, static function (&$operations, string $operationType) { sort($operations, SORT_STRING); $operations = "{$operationType}{" . implode(',', $operations) . '}'; @@ -168,6 +169,7 @@ private function updateTransactionName(): void $transactionName = 'lighthouse?' . implode('&', $groupedOperations); $transaction->setName($transactionName); + $transaction->getMetadata()->setSource(TransactionSource::custom()); Integration::setTransaction($transactionName); } diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php index 3257d075..460ec941 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -11,6 +11,7 @@ use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; use Symfony\Component\HttpFoundation\Response; class Middleware @@ -101,11 +102,11 @@ public function setBootedTimestamp(?float $timestamp = null): void private function startTransaction(Request $request, HubInterface $sentry): void { $requestStartTime = $request->server('REQUEST_TIME_FLOAT', microtime(true)); - $sentryTraceHeader = $request->header('sentry-trace'); - $context = $sentryTraceHeader - ? TransactionContext::fromSentryTrace($sentryTraceHeader) - : new TransactionContext; + $context = TransactionContext::fromHeaders( + $request->header('sentry-trace', ''), + $request->header('baggage', '') + ); $context->setOp('http.server'); $context->setData([ @@ -179,9 +180,9 @@ private function hydrateRequestData(Request $request): void $route = $request->route(); if ($route instanceof Route) { - $this->updateTransactionNameIfDefault( - Integration::extractNameForRoute($route) - ); + [$transactionName, $transactionSource] = Integration::extractNameAndSourceForRoute($route); + + $this->updateTransactionNameIfDefault($transactionName, $transactionSource); $this->transaction->setData([ 'name' => $route->getName(), @@ -190,7 +191,7 @@ private function hydrateRequestData(Request $request): void ]); } - $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/')); + $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/'), TransactionSource::url()); } private function hydrateResponseData(Response $response): void @@ -198,7 +199,7 @@ private function hydrateResponseData(Response $response): void $this->transaction->setHttpStatus($response->getStatusCode()); } - private function updateTransactionNameIfDefault(?string $name): void + private function updateTransactionNameIfDefault(?string $name, ?TransactionSource $source): void { // Ignore empty names (and `null`) for caller convenience if (empty($name)) { @@ -213,5 +214,6 @@ private function updateTransactionNameIfDefault(?string $name): void } $this->transaction->setName($name); + $this->transaction->getMetadata()->setSource($source ?? TransactionSource::custom()); } } From 3cfb4621ec99a8dc8a6a6d29da66773b0da2c753 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 12 Oct 2022 11:06:11 +0200 Subject: [PATCH 06/12] Add combined meta method (#586) --- src/Sentry/Laravel/Integration.php | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 49cd62e8..6254d84b 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -216,6 +216,17 @@ private static function extractNameForActionRoute(string $action): string /** * Retrieve the meta tags with tracing information to link this request to front-end requests. + * This propagates the Dynamic Sampling Context. + * + * @return string + */ + public static function sentryMeta(): string + { + return self::sentryTracingMeta() . self::sentryBaggageMeta(); + } + + /** + * Retrieve the `sentry-trace` meta tag with tracing information to link this request to front-end requests. * * @return string */ @@ -227,13 +238,11 @@ public static function sentryTracingMeta(): string return ''; } - $content = sprintf('', $span->toTraceparent()); - - return $content; + return sprintf('', $span->toTraceparent()); } /** - * Retrieve the meta tags with baggage information to link this request to front-end requests. + * Retrieve the `baggage` meta tag with information to link this request to front-end requests. * This propagates the Dynamic Sampling Context. * * @return string @@ -246,9 +255,7 @@ public static function sentryBaggageMeta(): string return ''; } - $content = sprintf('', $span->toBaggage()); - - return $content; + return sprintf('', $span->toBaggage()); } /** From 79a135d28d96b41f76d12a07ee1485b0f27c35af Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 12 Oct 2022 11:10:58 +0200 Subject: [PATCH 07/12] Improve tracing (#580) --- CHANGELOG.md | 3 + config/sentry.php | 5 +- src/Sentry/Laravel/Integration.php | 114 +++----------------- src/Sentry/Laravel/ServiceProvider.php | 13 +-- src/Sentry/Laravel/Tracing/EventHandler.php | 99 +++++++++++++++-- src/Sentry/Laravel/Tracing/Middleware.php | 75 ++++--------- test/Sentry/IntegrationTest.php | 45 ++------ 7 files changed, 145 insertions(+), 209 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35fb5a55..dd158488 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config/sentry.php b/config/sentry.php index b0e4b700..08c918bf 100644 --- a/config/sentry.php +++ b/config/sentry.php @@ -45,6 +45,9 @@ // 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 @@ -52,6 +55,4 @@ 'traces_sample_rate' => (float)(env('SENTRY_TRACES_SAMPLE_RATE', 0.0)), - 'controllers_base_namespace' => env('SENTRY_CONTROLLERS_BASE_NAMESPACE', 'App\\Http\\Controllers'), - ]; diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 6254d84b..8f683781 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/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('', $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. * diff --git a/src/Sentry/Laravel/ServiceProvider.php b/src/Sentry/Laravel/ServiceProvider.php index 9edb5015..5b8bf12a 100644 --- a/src/Sentry/Laravel/ServiceProvider.php +++ b/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,7 +156,7 @@ 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); @@ -169,7 +164,7 @@ protected function configureAndRegisterClient(): void $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 diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index bcba08b7..d971d32a 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/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()); } diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php index 460ec941..33d2afe2 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -4,15 +4,13 @@ use Closure; use Illuminate\Http\Request; -use Illuminate\Routing\Route; -use Sentry\Laravel\Integration; use Sentry\SentrySdk; use Sentry\State\HubInterface; use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; use Sentry\Tracing\TransactionContext; use Sentry\Tracing\TransactionSource; -use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\Response as SymfonyResponse; class Middleware { @@ -45,7 +43,7 @@ class Middleware * * @return mixed */ - public function handle($request, Closure $next) + public function handle(Request $request, Closure $next) { if (app()->bound(HubInterface::class)) { $this->startTransaction($request, app(HubInterface::class)); @@ -57,14 +55,19 @@ public function handle($request, Closure $next) /** * Handle the application termination. * - * @param \Illuminate\Http\Request $request - * @param \Symfony\Component\HttpFoundation\Response $response + * @param \Illuminate\Http\Request $request + * @param mixed $response * * @return void */ - public function terminate($request, $response): void + public function terminate(Request $request, $response): void { if ($this->transaction !== null && app()->bound(HubInterface::class)) { + // We stop here if a route has not been matched unless we are configured to trace missing routes + if (config('sentry.tracing.missing_routes', false) === false && $request->route() === null) { + return; + } + if ($this->appSpan !== null) { $this->appSpan->finish(); } @@ -73,11 +76,7 @@ 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) { + if ($response instanceof SymfonyResponse) { $this->hydrateResponseData($response); } @@ -108,12 +107,17 @@ private function startTransaction(Request $request, HubInterface $sentry): void $request->header('baggage', '') ); + $requestPath = '/' . ltrim($request->path(), '/'); + $context->setOp('http.server'); + $context->setName($requestPath); + $context->setSource(TransactionSource::url()); + $context->setStartTimestamp($requestStartTime); + $context->setData([ - 'url' => '/' . ltrim($request->path(), '/'), + 'url' => $requestPath, 'method' => strtoupper($request->method()), ]); - $context->setStartTimestamp($requestStartTime); $this->transaction = $sentry->startTransaction($context); @@ -122,7 +126,7 @@ private function startTransaction(Request $request, HubInterface $sentry): void $bootstrapSpan = $this->addAppBootstrapSpan($request); - $appContextStart = new SpanContext(); + $appContextStart = new SpanContext; $appContextStart->setOp('middleware.handle'); $appContextStart->setStartTimestamp($bootstrapSpan ? $bootstrapSpan->getEndTimestamp() : microtime(true)); @@ -143,7 +147,7 @@ private function addAppBootstrapSpan(Request $request): ?Span return null; } - $spanContextStart = new SpanContext(); + $spanContextStart = new SpanContext; $spanContextStart->setOp('app.bootstrap'); $spanContextStart->setStartTimestamp($laravelStartTime); $spanContextStart->setEndTimestamp($this->bootedTimestamp); @@ -175,45 +179,8 @@ private function addBootDetailTimeSpans(Span $bootstrap): void $bootstrap->startChild($autoload); } - private function hydrateRequestData(Request $request): void - { - $route = $request->route(); - - if ($route instanceof Route) { - [$transactionName, $transactionSource] = Integration::extractNameAndSourceForRoute($route); - - $this->updateTransactionNameIfDefault($transactionName, $transactionSource); - - $this->transaction->setData([ - 'name' => $route->getName(), - 'action' => $route->getActionName(), - 'method' => $request->getMethod(), - ]); - } - - $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/'), TransactionSource::url()); - } - - private function hydrateResponseData(Response $response): void + private function hydrateResponseData(SymfonyResponse $response): void { $this->transaction->setHttpStatus($response->getStatusCode()); } - - private function updateTransactionNameIfDefault(?string $name, ?TransactionSource $source): void - { - // Ignore empty names (and `null`) for caller convenience - if (empty($name)) { - return; - } - - // If the transaction already has a name other than the default - // ignore the new name, this will most occur if the user has set a - // transaction name themself before the application reaches this point - if ($this->transaction->getName() !== TransactionContext::DEFAULT_NAME) { - return; - } - - $this->transaction->setName($name); - $this->transaction->getMetadata()->setSource($source ?? TransactionSource::custom()); - } } diff --git a/test/Sentry/IntegrationTest.php b/test/Sentry/IntegrationTest.php index 2365931b..a2ed7af2 100644 --- a/test/Sentry/IntegrationTest.php +++ b/test/Sentry/IntegrationTest.php @@ -9,6 +9,7 @@ use Sentry\Event; use Sentry\Laravel\Integration; use Sentry\State\Scope; +use Sentry\Tracing\TransactionSource; use function Sentry\withScope; class IntegrationTest extends SentryLaravelTestCase @@ -88,36 +89,11 @@ public function testTransactionIsNotAppliedToEventWhenTransactionIsAlreadySet(): }); } - public function testExtractingNameForRouteWithName(): void - { - $route = (new Route('GET', '/foo', []))->name($routeName = 'foo-bar'); - - $this->assertSame($routeName, Integration::extractNameForRoute($route)); - } - - public function testExtractingNameForRouteWithAction(): void - { - $route = (new Route('GET', '/foo', [ - 'controller' => $controller = 'SomeController@someAction', - ])); - - $this->assertSame($controller, Integration::extractNameForRoute($route)); - } - public function testExtractingNameForRouteWithoutName(): void { $route = new Route('GET', $url = '/foo', []); - $this->assertSame($url, Integration::extractNameForRoute($route)); - } - - public function testExtractingNameForRouteWithActionAndName(): void - { - $route = (new Route('GET', '/foo', [ - 'controller' => 'SomeController@someAction', - ]))->name($routeName = 'foo-bar'); - - $this->assertSame($routeName, Integration::extractNameForRoute($route)); + $this->assetRouteNameAndSource($route, $url, TransactionSource::route()); } public function testExtractingNameForRouteWithAutoGeneratedName(): void @@ -125,26 +101,21 @@ public function testExtractingNameForRouteWithAutoGeneratedName(): void // We fake a generated name here, Laravel generates them each starting with `generated::` $route = (new Route('GET', $url = '/foo', []))->name('generated::KoAePbpBofo01ey4'); - $this->assertSame($url, Integration::extractNameForRoute($route)); + $this->assetRouteNameAndSource($route, $url, TransactionSource::route()); } public function testExtractingNameForRouteWithIncompleteGroupName(): void { $route = (new Route('GET', $url = '/foo', []))->name('group-name.'); - $this->assertSame($url, Integration::extractNameForRoute($route)); + $this->assetRouteNameAndSource($route, $url, TransactionSource::route()); } - public function testExtractingNameForRouteWithStrippedBaseNamespaceFromAction(): void + private function assetRouteNameAndSource(Route $route, string $expectedName, TransactionSource $expectedSource): void { - Integration::setControllersBaseNamespace('BaseNamespace'); - - $route = (new Route('GET', '/foo', [ - 'controller' => 'BaseNamespace\\SomeController@someAction', - ])); - - $this->assertSame('SomeController@someAction', Integration::extractNameForRoute($route)); + [$actualName, $actualSource] = Integration::extractNameAndSourceForRoute($route); - Integration::setControllersBaseNamespace(null); + $this->assertSame($expectedName, $actualName); + $this->assertSame($expectedSource, $actualSource); } } From efecb3e294669dc2d5c61904c96ade2898e687eb Mon Sep 17 00:00:00 2001 From: Michi Hoffmann Date: Thu, 13 Oct 2022 11:30:15 +0200 Subject: [PATCH 08/12] docs: Add 3.0.0 changelog (#590) Co-authored-by: Alex Bouma --- CHANGELOG.md | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd158488..b6a0cbdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,23 @@ ## 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) +## 3.0.0 + +**Breaking changes** + +- Laravel Lumen is no longer supported + - Drop support for Laravel Lumen (#579) +- Laravel versions 5.0 - 5.8 are no longer supported + - Drop support for Laravel 5.x (#581) - Remove `Sentry\Integration::extractNameForRoute()`, it's alternative `Sentry\Integration::extractNameAndSourceForRoute()` is marked as `@internal` (#580) -- Drop support for Laravel 5.x (#581) +- Remove extracting route name or controller for transaction names (#583). This unifies the transaction names to a more concise format. + +**Other changes** + +- Add support for Dynamic Sampling (#572) +- Set the tracing transaction name on the `Illuminate\Routing\Events\RouteMatched` instead of at the end of the request (#580) +- Add tracing span for Laravel HTTP client requests (#585) +- Simplify Sentry meta tag retrieval, by adding `Sentry\Laravel\Integration::sentryMeta()` (#586) ## 2.14.1 From 0d5d9a008b5e5f4ee1dfdc2a0c9a0577f55b4a7e Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Mon, 17 Oct 2022 14:05:48 +0200 Subject: [PATCH 09/12] Cleanup event handlers (#592) --- CHANGELOG.md | 4 + src/Sentry/Laravel/EventHandler.php | 72 ++---- src/Sentry/Laravel/Integration.php | 33 +-- src/Sentry/Laravel/ServiceProvider.php | 27 ++- src/Sentry/Laravel/Tracing/EventHandler.php | 215 +++++++++--------- .../Integrations/LighthouseIntegration.php | 2 +- .../TracingCallableDispatcherTracing.php | 24 ++ .../TracingControllerDispatcherTracing.php | 29 +++ .../Routing/TracingRoutingDispatcher.php | 36 +++ .../Laravel/Tracing/ServiceProvider.php | 39 +++- .../Laravel/Tracing/ViewEngineDecorator.php | 3 +- test/Sentry/Tracing/EventHandlerTest.php | 4 +- 12 files changed, 276 insertions(+), 212 deletions(-) create mode 100644 src/Sentry/Laravel/Tracing/Routing/TracingCallableDispatcherTracing.php create mode 100644 src/Sentry/Laravel/Tracing/Routing/TracingControllerDispatcherTracing.php create mode 100644 src/Sentry/Laravel/Tracing/Routing/TracingRoutingDispatcher.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 96e945f4..a5eefcec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Drop support for Laravel 5.x (#581) - Remove `Sentry\Integration::extractNameForRoute()`, it's alternative `Sentry\Integration::extractNameAndSourceForRoute()` is marked as `@internal` (#580) - Remove extracting route name or controller for transaction names (#583). This unifies the transaction names to a more concise format. +- Remove internal `Sentry\Integration::currentTracingSpan()`, use `SentrySdk::getCurrentHub()->getSpan()` if you were using this internal method. (#592) **Other changes** @@ -19,6 +20,9 @@ - Set the tracing transaction name on the `Illuminate\Routing\Events\RouteMatched` instead of at the end of the request (#580) - Add tracing span for Laravel HTTP client requests (#585) - Simplify Sentry meta tag retrieval, by adding `Sentry\Laravel\Integration::sentryMeta()` (#586) +- Fix tracing with nested queue jobs (mostly when running jobs in the `sync` driver) (#592) +- Add a `http.route` span, this span indicates the time that is spent inside a controller method or route closure (#593) +- Add a `db.transaction` span, this span indicates the time that is spent inside a database transaction (#594) ## 2.14.2 diff --git a/src/Sentry/Laravel/EventHandler.php b/src/Sentry/Laravel/EventHandler.php index 41d87a06..e2f32924 100644 --- a/src/Sentry/Laravel/EventHandler.php +++ b/src/Sentry/Laravel/EventHandler.php @@ -140,9 +140,9 @@ class EventHandler /** * Indicates if we pushed a scope for the queue. * - * @var bool + * @var int */ - private $pushedQueueScope = false; + private $pushedQueueScopeCount = 0; /** * Indicates if we pushed a scope for Octane. @@ -173,76 +173,40 @@ public function __construct(Container $container, array $config) /** * Attach all event handlers. */ - public function subscribe(): void + public function subscribe(Dispatcher $dispatcher): void { - /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ - try { - $dispatcher = $this->container->make(Dispatcher::class); - - foreach (static::$eventHandlerMap as $eventName => $handler) { - $dispatcher->listen($eventName, [$this, $handler]); - } - } catch (BindingResolutionException $e) { - // If we cannot resolve the event dispatcher we also cannot listen to events + foreach (static::$eventHandlerMap as $eventName => $handler) { + $dispatcher->listen($eventName, [$this, $handler]); } } /** * Attach all authentication event handlers. */ - public function subscribeAuthEvents(): void + public function subscribeAuthEvents(Dispatcher $dispatcher): void { - /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ - try { - $dispatcher = $this->container->make(Dispatcher::class); - - foreach (static::$authEventHandlerMap as $eventName => $handler) { - $dispatcher->listen($eventName, [$this, $handler]); - } - } catch (BindingResolutionException $e) { - // If we cannot resolve the event dispatcher we also cannot listen to events + foreach (static::$authEventHandlerMap as $eventName => $handler) { + $dispatcher->listen($eventName, [$this, $handler]); } } /** * Attach all queue event handlers. */ - public function subscribeOctaneEvents(): void + public function subscribeOctaneEvents(Dispatcher $dispatcher): void { - /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ - try { - $dispatcher = $this->container->make(Dispatcher::class); - - foreach (static::$octaneEventHandlerMap as $eventName => $handler) { - $dispatcher->listen($eventName, [$this, $handler]); - } - } catch (BindingResolutionException $e) { - // If we cannot resolve the event dispatcher we also cannot listen to events + foreach (static::$octaneEventHandlerMap as $eventName => $handler) { + $dispatcher->listen($eventName, [$this, $handler]); } } /** * Attach all queue event handlers. - * - * @param \Illuminate\Queue\QueueManager $queue */ - public function subscribeQueueEvents(QueueManager $queue): void + public function subscribeQueueEvents(Dispatcher $dispatcher): void { - $queue->looping(function () { - $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScope); - - $this->pushedQueueScope = false; - }); - - /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ - try { - $dispatcher = $this->container->make(Dispatcher::class); - - foreach (static::$queueEventHandlerMap as $eventName => $handler) { - $dispatcher->listen($eventName, [$this, $handler]); - } - } catch (BindingResolutionException $e) { - // If we cannot resolve the event dispatcher we also cannot listen to events + foreach (static::$queueEventHandlerMap as $eventName => $handler) { + $dispatcher->listen($eventName, [$this, $handler]); } } @@ -382,11 +346,9 @@ private function configureUserScopeFromModel($authUser): void protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void { - $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScope); - $this->prepareScopeForTaskWithinLongRunningProcess(); - $this->pushedQueueScope = true; + ++$this->pushedQueueScopeCount; if (!$this->recordQueueInfo) { return; @@ -415,11 +377,15 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event): void { + $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScopeCount > 0); + $this->afterTaskWithinLongRunningProcess(); } protected function queueJobProcessedHandler(QueueEvents\JobProcessed $event): void { + $this->cleanupScopeForTaskWithinLongRunningProcessWhen($this->pushedQueueScopeCount > 0); + $this->afterTaskWithinLongRunningProcess(); } diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 8f683781..a9df0948 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -3,10 +3,7 @@ namespace Sentry\Laravel; use Illuminate\Routing\Route; -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; @@ -118,7 +115,7 @@ public static function extractNameAndSourceForRoute(Route $route): array { return [ '/' . ltrim($route->uri(), '/'), - TransactionSource::route() + TransactionSource::route(), ]; } @@ -140,7 +137,7 @@ public static function sentryMeta(): string */ public static function sentryTracingMeta(): string { - $span = self::currentTracingSpan(); + $span = SentrySdk::getCurrentHub()->getSpan(); if ($span === null) { return ''; @@ -157,7 +154,7 @@ public static function sentryTracingMeta(): string */ public static function sentryBaggageMeta(): string { - $span = self::currentTracingSpan(); + $span = SentrySdk::getCurrentHub()->getSpan(); if ($span === null) { return ''; @@ -165,28 +162,4 @@ public static function sentryBaggageMeta(): string return sprintf('', $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. - * - * @return \Sentry\Tracing\Span|null - * - * @internal This is used internally as an easy way to retrieve the current active tracing span. - */ - public static function currentTracingSpan(): ?Span - { - return SentrySdk::getCurrentHub()->getSpan(); - } } diff --git a/src/Sentry/Laravel/ServiceProvider.php b/src/Sentry/Laravel/ServiceProvider.php index 5b8bf12a..5e01a2c6 100644 --- a/src/Sentry/Laravel/ServiceProvider.php +++ b/src/Sentry/Laravel/ServiceProvider.php @@ -2,6 +2,8 @@ namespace Sentry\Laravel; +use Illuminate\Contracts\Container\BindingResolutionException; +use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Contracts\Http\Kernel as HttpKernelInterface; use Illuminate\Foundation\Application as Laravel; use Illuminate\Foundation\Http\Kernel as HttpKernel; @@ -95,18 +97,25 @@ protected function bindEvents(): void $handler = new EventHandler($this->app, $userConfig); - $handler->subscribe(); + try { + /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ + $dispatcher = $this->app->make(Dispatcher::class); - if ($this->app->bound('octane')) { - $handler->subscribeOctaneEvents(); - } + $handler->subscribe($dispatcher); - if ($this->app->bound('queue')) { - $handler->subscribeQueueEvents($this->app->make('queue')); - } + if ($this->app->bound('octane')) { + $handler->subscribeOctaneEvents($dispatcher); + } - if (isset($userConfig['send_default_pii']) && $userConfig['send_default_pii'] !== false) { - $handler->subscribeAuthEvents(); + if ($this->app->bound('queue')) { + $handler->subscribeQueueEvents($dispatcher, $this->app->make('queue')); + } + + if (isset($userConfig['send_default_pii']) && $userConfig['send_default_pii'] !== false) { + $handler->subscribeAuthEvents($dispatcher); + } + } catch (BindingResolutionException $e) { + // If we cannot resolve the event dispatcher we also cannot listen to events } } diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index d971d32a..3850d540 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -3,8 +3,6 @@ namespace Sentry\Laravel\Tracing; use Exception; -use Illuminate\Contracts\Container\BindingResolutionException; -use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Database\Events as DatabaseEvents; use Illuminate\Http\Client\Events as HttpClientEvents; @@ -15,6 +13,7 @@ use RuntimeException; use Sentry\Laravel\Integration; use Sentry\SentrySdk; +use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; use Sentry\Tracing\SpanStatus; use Sentry\Tracing\TransactionContext; @@ -22,9 +21,8 @@ class EventHandler { - public const QUEUE_PAYLOAD_TRACE_PARENT_DATA = 'sentry_trace_parent_data'; - public const QUEUE_PAYLOAD_BAGGAGE_DATA = 'sentry_baggage_data'; + public const QUEUE_PAYLOAD_TRACE_PARENT_DATA = 'sentry_trace_parent_data'; /** * Map event handlers to events. @@ -37,6 +35,9 @@ class EventHandler HttpClientEvents\RequestSending::class => 'httpClientRequestSending', HttpClientEvents\ResponseReceived::class => 'httpClientResponseReceived', HttpClientEvents\ConnectionFailed::class => 'httpClientConnectionFailed', + DatabaseEvents\TransactionBeginning::class => 'transactionBeginning', + DatabaseEvents\TransactionCommitted::class => 'transactionCommitted', + DatabaseEvents\TransactionRolledBack::class => 'transactionRolledBack', ]; /** @@ -50,13 +51,6 @@ class EventHandler QueueEvents\JobExceptionOccurred::class => 'queueJobExceptionOccurred', ]; - /** - * The Laravel container. - * - * @var \Illuminate\Contracts\Container\Container - */ - private $container; - /** * Indicates if we should we add SQL queries as spans. * @@ -86,32 +80,18 @@ class EventHandler private $traceQueueJobsAsTransactions; /** - * Holds a reference to the parent queue job span. - * - * @var \Sentry\Tracing\Span|null - */ - private $parentQueueJobSpan; - - /** - * Holds a reference to the current queue job span or transaction. + * Hold the stack of parent spans that need to be put back on the scope. * - * @var \Sentry\Tracing\Transaction|\Sentry\Tracing\Span|null + * @var array */ - private $currentQueueJobSpan; + private $parentSpanStack = []; /** - * Holds a reference to the parent http client request span. + * Hold the stack of current spans that need to be finished still. * - * @var \Sentry\Tracing\Span|null + * @var array */ - private $parentHttpClientRequestSpan; - - /** - * Holds a reference to the current http client request span. - * - * @var \Sentry\Tracing\Span|null - */ - private $currentHttpClientRequestSpan; + private $currentSpanStack = []; /** * The backtrace helper. @@ -122,21 +102,16 @@ class EventHandler /** * EventHandler constructor. - * - * @param \Illuminate\Contracts\Container\Container $container - * @param \Sentry\Laravel\Tracing\BacktraceHelper $backtraceHelper - * @param array $config */ - public function __construct(Container $container, BacktraceHelper $backtraceHelper, array $config) + public function __construct(array $config, BacktraceHelper $backtraceHelper) { - $this->container = $container; - $this->backtraceHelper = $backtraceHelper; - $this->traceSqlQueries = ($config['sql_queries'] ?? true) === true; $this->traceSqlQueryOrigins = ($config['sql_origin'] ?? true) === true; $this->traceQueueJobs = ($config['queue_jobs'] ?? false) === true; $this->traceQueueJobsAsTransactions = ($config['queue_job_transactions'] ?? false) === true; + + $this->backtraceHelper = $backtraceHelper; } /** @@ -144,31 +119,28 @@ public function __construct(Container $container, BacktraceHelper $backtraceHelp * * @uses self::routeMatchedHandler() * @uses self::queryExecutedHandler() + * @uses self::transactionBeginningHandler() + * @uses self::transactionCommittedHandler() + * @uses self::transactionRolledBackHandler() + * @uses self::httpClientRequestSendingHandler() + * @uses self::httpClientResponseReceivedHandler() + * @uses self::httpClientConnectionFailedHandler() */ - public function subscribe(): void + public function subscribe(Dispatcher $dispatcher): void { - try { - /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ - $dispatcher = $this->container->make(Dispatcher::class); - - foreach (static::$eventHandlerMap as $eventName => $handler) { - $dispatcher->listen($eventName, [$this, $handler]); - } - } catch (BindingResolutionException $e) { - // If we cannot resolve the event dispatcher we also cannot listen to events + foreach (static::$eventHandlerMap as $eventName => $handler) { + $dispatcher->listen($eventName, [$this, $handler]); } } /** * Attach all queue event handlers. * - * @param \Illuminate\Queue\QueueManager $queue - * * @uses self::queueJobProcessingHandler() * @uses self::queueJobProcessedHandler() * @uses self::queueJobExceptionOccurredHandler() */ - public function subscribeQueueEvents(QueueManager $queue): void + public function subscribeQueueEvents(Dispatcher $dispatcher, QueueManager $queue): void { // If both types of queue job tracing is disabled also do not register the events if (!$this->traceQueueJobs && !$this->traceQueueJobsAsTransactions) { @@ -176,7 +148,7 @@ public function subscribeQueueEvents(QueueManager $queue): void } Queue::createPayloadUsing(static function (?string $connection, ?string $queue, ?array $payload): ?array { - $currentSpan = Integration::currentTracingSpan(); + $currentSpan = SentrySdk::getCurrentHub()->getSpan(); if ($currentSpan !== null && $payload !== null) { $payload[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] = $currentSpan->toTraceparent(); @@ -186,19 +158,8 @@ public function subscribeQueueEvents(QueueManager $queue): void return $payload; }); - $queue->looping(function () { - $this->afterQueuedJob(); - }); - - try { - /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ - $dispatcher = $this->container->make(Dispatcher::class); - - foreach (static::$queueEventHandlerMap as $eventName => $handler) { - $dispatcher->listen($eventName, [$this, $handler]); - } - } catch (BindingResolutionException $e) { - // If we cannot resolve the event dispatcher we also cannot listen to events + foreach (static::$queueEventHandlerMap as $eventName => $handler) { + $dispatcher->listen($eventName, [$this, $handler]); } } @@ -218,14 +179,14 @@ public function __call(string $method, array $arguments) try { call_user_func_array([$this, $handlerMethod], $arguments); - } catch (Exception $exception) { - // Ignore + } catch (Exception $e) { + // Ignore to prevent bubbling up errors in the SDK } } protected function routeMatchedHandler(RoutingEvents\RouteMatched $match): void { - $transaction = Integration::currentTransaction(); + $transaction = SentrySdk::getCurrentHub()->getTransaction(); if ($transaction === null) { return; @@ -243,7 +204,7 @@ protected function queryExecutedHandler(DatabaseEvents\QueryExecuted $query): vo return; } - $parentSpan = Integration::currentTracingSpan(); + $parentSpan = SentrySdk::getCurrentHub()->getSpan(); // If there is no tracing span active there is no need to handle the event if ($parentSpan === null) { @@ -285,59 +246,79 @@ private function resolveQueryOriginFromBacktrace(): ?string return "{$filePath}:{$firstAppFrame->getLine()}"; } - protected function httpClientRequestSendingHandler(HttpClientEvents\RequestSending $event): void + protected function transactionBeginningHandler(DatabaseEvents\TransactionBeginning $event): void { - $parentSpan = Integration::currentTracingSpan(); + $parentSpan = SentrySdk::getCurrentHub()->getSpan(); if ($parentSpan === null) { return; } $context = new SpanContext; + $context->setOp('db.transaction'); - $context->setOp('http.client'); - $context->setDescription($event->request->method() . ' ' . $event->request->url()); - $context->setStartTimestamp(microtime(true)); - - $this->currentHttpClientRequestSpan = $parentSpan->startChild($context); + $this->pushSpan($parentSpan->startChild($context)); + } - $this->parentHttpClientRequestSpan = $parentSpan; + protected function transactionCommittedHandler(DatabaseEvents\TransactionCommitted $event): void + { + $span = $this->popSpan(); - SentrySdk::getCurrentHub()->setSpan($this->currentHttpClientRequestSpan); + if ($span !== null) { + $span->finish(); + $span->setStatus(SpanStatus::ok()); + } } - protected function httpClientResponseReceivedHandler(HttpClientEvents\ResponseReceived $event): void + protected function transactionRolledBackHandler(DatabaseEvents\TransactionRolledBack $event): void { - if ($this->currentHttpClientRequestSpan !== null) { - $this->currentHttpClientRequestSpan->setHttpStatus($event->response->status()); - $this->afterHttpClientRequest(); + $span = $this->popSpan(); + + if ($span !== null) { + $span->finish(); + $span->setStatus(SpanStatus::internalError()); } } - protected function httpClientConnectionFailedHandler(HttpClientEvents\ConnectionFailed $event): void + protected function httpClientRequestSendingHandler(HttpClientEvents\RequestSending $event): void { - if ($this->currentHttpClientRequestSpan !== null) { - $this->currentHttpClientRequestSpan->setStatus(SpanStatus::internalError()); - $this->afterHttpClientRequest(); + $parentSpan = SentrySdk::getCurrentHub()->getSpan(); + + if ($parentSpan === null) { + return; } + + $context = new SpanContext; + + $context->setOp('http.client'); + $context->setDescription($event->request->method() . ' ' . $event->request->url()); + + $this->pushSpan($parentSpan->startChild($context)); } - private function afterHttpClientRequest(): void + protected function httpClientResponseReceivedHandler(HttpClientEvents\ResponseReceived $event): void { - if ($this->currentHttpClientRequestSpan === null) { - return; + $span = $this->popSpan(); + + if ($span !== null) { + $span->finish(); + $span->setHttpStatus($event->response->status()); } + } - $this->currentHttpClientRequestSpan->finish(); - $this->currentHttpClientRequestSpan = null; + protected function httpClientConnectionFailedHandler(HttpClientEvents\ConnectionFailed $event): void + { + $span = $this->popSpan(); - SentrySdk::getCurrentHub()->setSpan($this->parentHttpClientRequestSpan); - $this->parentHttpClientRequestSpan = null; + if ($span !== null) { + $span->finish(); + $span->setStatus(SpanStatus::internalError()); + } } protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): void { - $parentSpan = Integration::currentTracingSpan(); + $parentSpan = SentrySdk::getCurrentHub()->getSpan(); // If there is no tracing span active and we don't trace jobs as transactions there is no need to handle the event if ($parentSpan === null && !$this->traceQueueJobsAsTransactions) { @@ -368,7 +349,7 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): $job = [ 'job' => $event->job->getName(), 'queue' => $event->job->getQueue(), - 'resolved' => $event->job->resolveName(), + 'resolved' => $resolvedJobName, 'attempts' => $event->job->attempts(), 'connection' => $event->connectionName, ]; @@ -384,14 +365,12 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event): // When the parent span is null we start a new transaction otherwise we start a child of the current span if ($parentSpan === null) { - $this->currentQueueJobSpan = SentrySdk::getCurrentHub()->startTransaction($context); + $span = SentrySdk::getCurrentHub()->startTransaction($context); } else { - $this->currentQueueJobSpan = $parentSpan->startChild($context); + $span = $parentSpan->startChild($context); } - $this->parentQueueJobSpan = $parentSpan; - - SentrySdk::getCurrentHub()->setSpan($this->currentQueueJobSpan); + $this->pushSpan($span); } protected function queueJobExceptionOccurredHandler(QueueEvents\JobExceptionOccurred $event): void @@ -406,15 +385,37 @@ protected function queueJobProcessedHandler(QueueEvents\JobProcessed $event): vo private function afterQueuedJob(?SpanStatus $status = null): void { - if ($this->currentQueueJobSpan === null) { + $span = $this->popSpan(); + + if ($span === null) { return; } - $this->currentQueueJobSpan->setStatus($status); - $this->currentQueueJobSpan->finish(); - $this->currentQueueJobSpan = null; + $span->setStatus($status); + $span->finish(); + } + + private function pushSpan(Span $span): void + { + $hub = SentrySdk::getCurrentHub(); + + $this->parentSpanStack[] = $hub->getSpan(); + + $hub->setSpan($span); + + $this->currentSpanStack[] = $span; + } + + private function popSpan(): ?Span + { + if (count($this->currentSpanStack) === 0) { + return null; + } + + $parent = array_pop($this->parentSpanStack); + + SentrySdk::getCurrentHub()->setSpan($parent); - SentrySdk::getCurrentHub()->setSpan($this->parentQueueJobSpan); - $this->parentQueueJobSpan = null; + return array_pop($this->currentSpanStack); } } diff --git a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php index dc443a35..3a36257a 100644 --- a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php +++ b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php @@ -57,7 +57,7 @@ public function setupOnce(): void public function handleStartRequest(StartRequest $startRequest): void { - $this->previousSpan = Integration::currentTracingSpan(); + $this->previousSpan = SentrySdk::getCurrentHub()->getSpan(); if ($this->previousSpan === null) { return; diff --git a/src/Sentry/Laravel/Tracing/Routing/TracingCallableDispatcherTracing.php b/src/Sentry/Laravel/Tracing/Routing/TracingCallableDispatcherTracing.php new file mode 100644 index 00000000..7ccb1062 --- /dev/null +++ b/src/Sentry/Laravel/Tracing/Routing/TracingCallableDispatcherTracing.php @@ -0,0 +1,24 @@ +dispatcher = $dispatcher; + } + + public function dispatch(Route $route, $callable) + { + return $this->wrapRouteDispatch(function () use ($route, $callable) { + return $this->dispatcher->dispatch($route, $callable); + }, $route); + } +} diff --git a/src/Sentry/Laravel/Tracing/Routing/TracingControllerDispatcherTracing.php b/src/Sentry/Laravel/Tracing/Routing/TracingControllerDispatcherTracing.php new file mode 100644 index 00000000..1cedda2c --- /dev/null +++ b/src/Sentry/Laravel/Tracing/Routing/TracingControllerDispatcherTracing.php @@ -0,0 +1,29 @@ +dispatcher = $dispatcher; + } + + public function dispatch(Route $route, $controller, $method) + { + return $this->wrapRouteDispatch(function () use ($route, $controller, $method) { + return $this->dispatcher->dispatch($route, $controller, $method); + }, $route); + } + + public function getMiddleware($controller, $method) + { + return $this->dispatcher->getMiddleware($controller, $method); + } +} diff --git a/src/Sentry/Laravel/Tracing/Routing/TracingRoutingDispatcher.php b/src/Sentry/Laravel/Tracing/Routing/TracingRoutingDispatcher.php new file mode 100644 index 00000000..98212973 --- /dev/null +++ b/src/Sentry/Laravel/Tracing/Routing/TracingRoutingDispatcher.php @@ -0,0 +1,36 @@ +getSpan(); + + // When there is no active transaction we can skip doing anything and just immediately return the callable + if ($parentSpan === null) { + return $dispatch(); + } + + $context = new SpanContext; + $context->setOp('http.route'); + $context->setDescription($route->getActionName()); + + $span = $parentSpan->startChild($context); + + SentrySdk::getCurrentHub()->setSpan($span); + + try { + return $dispatch(); + } finally { + $span->finish(); + + SentrySdk::getCurrentHub()->setSpan($parentSpan); + } + } +} diff --git a/src/Sentry/Laravel/Tracing/ServiceProvider.php b/src/Sentry/Laravel/Tracing/ServiceProvider.php index 336e6025..cd56d2d4 100644 --- a/src/Sentry/Laravel/Tracing/ServiceProvider.php +++ b/src/Sentry/Laravel/Tracing/ServiceProvider.php @@ -2,14 +2,20 @@ namespace Sentry\Laravel\Tracing; +use Illuminate\Contracts\Container\BindingResolutionException; +use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Contracts\Http\Kernel as HttpKernelInterface; use Illuminate\Contracts\View\Engine; use Illuminate\Contracts\View\View; use Illuminate\Foundation\Http\Kernel as HttpKernel; +use Illuminate\Routing\Contracts\CallableDispatcher; +use Illuminate\Routing\Contracts\ControllerDispatcher; use Illuminate\View\Engines\EngineResolver; use Illuminate\View\Factory as ViewFactory; use InvalidArgumentException; use Sentry\Laravel\BaseServiceProvider; +use Sentry\Laravel\Tracing\Routing\TracingCallableDispatcherTracing; +use Sentry\Laravel\Tracing\Routing\TracingControllerDispatcherTracing; use Sentry\Serializer\RepresentationSerializer; class ServiceProvider extends BaseServiceProvider @@ -27,6 +33,8 @@ public function boot(): void $this->bindViewEngine($tracingConfig); + $this->decorateRoutingDispatchers(); + if ($this->app->bound(HttpKernelInterface::class)) { /** @var \Illuminate\Foundation\Http\Kernel $httpKernel */ $httpKernel = $this->app->make(HttpKernelInterface::class); @@ -59,17 +67,21 @@ public function register(): void private function bindEvents(array $tracingConfig): void { $handler = new EventHandler( - $this->app, - $this->app->make(BacktraceHelper::class), - $tracingConfig + $tracingConfig, + $this->app->make(BacktraceHelper::class) ); - $handler->subscribe(); + try { + /** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */ + $dispatcher = $this->app->make(Dispatcher::class); + + $handler->subscribe($dispatcher); - if ($this->app->bound('queue')) { - $handler->subscribeQueueEvents( - $this->app->make('queue') - ); + if ($this->app->bound('queue')) { + $handler->subscribeQueueEvents($dispatcher, $this->app->make('queue')); + } + } catch (BindingResolutionException $e) { + // If we cannot resolve the event dispatcher we also cannot listen to events } } @@ -113,4 +125,15 @@ private function wrapViewEngine(Engine $realEngine): Engine return new ViewEngineDecorator($realEngine, $viewFactory); } + + private function decorateRoutingDispatchers(): void + { + $this->app->extend(CallableDispatcher::class, static function (CallableDispatcher $dispatcher) { + return new TracingCallableDispatcherTracing($dispatcher); + }); + + $this->app->extend(ControllerDispatcher::class, static function (ControllerDispatcher $dispatcher) { + return new TracingControllerDispatcherTracing($dispatcher); + }); + } } diff --git a/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php b/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php index 79e262b5..a153243f 100644 --- a/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php +++ b/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php @@ -4,7 +4,6 @@ use Illuminate\Contracts\View\Engine; use Illuminate\View\Factory; -use Sentry\Laravel\Integration; use Sentry\SentrySdk; use Sentry\Tracing\SpanContext; @@ -29,7 +28,7 @@ public function __construct(Engine $engine, Factory $viewFactory) */ public function get($path, array $data = []): string { - $parentSpan = Integration::currentTracingSpan(); + $parentSpan = SentrySdk::getCurrentHub()->getSpan(); if ($parentSpan === null) { return $this->engine->get($path, $data); diff --git a/test/Sentry/Tracing/EventHandlerTest.php b/test/Sentry/Tracing/EventHandlerTest.php index f71faac8..4e67b9cb 100644 --- a/test/Sentry/Tracing/EventHandlerTest.php +++ b/test/Sentry/Tracing/EventHandlerTest.php @@ -17,7 +17,7 @@ public function test_missing_event_handler_throws_exception() { $this->safeExpectException(RuntimeException::class); - $handler = new EventHandler($this->app, $this->app->make(BacktraceHelper::class), []); + $handler = new EventHandler([], $this->app->make(BacktraceHelper::class)); $handler->thisIsNotAHandlerAndShouldThrowAnException(); } @@ -31,7 +31,7 @@ public function test_all_mapped_event_handlers_exist() private function tryAllEventHandlerMethods(array $methods): void { - $handler = new EventHandler($this->app, $this->app->make(BacktraceHelper::class), []); + $handler = new EventHandler([], $this->app->make(BacktraceHelper::class)); $methods = array_map(static function ($method) { return "{$method}Handler"; From ba8b49013bcf0e30e71455c7d60a473ea0ca6ca4 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Mon, 17 Oct 2022 14:07:33 +0200 Subject: [PATCH 10/12] Cleanup and restructure tests (#596) --- src/Sentry/Laravel/LogChannel.php | 2 +- test/Sentry/ClientBuilderDecoratorTest.php | 27 +++++++++ .../ConsoleEventsTest.php} | 18 +++--- .../DatabaseEventsTest.php} | 59 +++++++++++++++---- .../LogEventsTest.php} | 12 ++-- test/Sentry/EventHandlerTest.php | 29 +++++---- test/Sentry/ExpectsException.php | 25 -------- .../ExceptionContextIntegrationTest.php | 7 +-- test/Sentry/IntegrationTest.php | 2 +- .../LaravelIntegrationsOptionTest.php} | 59 +++++++++---------- test/Sentry/{Laravel => }/LogChannelTest.php | 14 +++-- .../ServiceClientBuilderDecoratorTest.php | 35 ----------- test/Sentry/ServiceProviderTest.php | 23 ++++---- .../ServiceProviderWithCustomAliasTest.php | 19 +++--- ...eProviderWithEnvironmentFromConfigTest.php | 18 +++--- test/Sentry/ServiceProviderWithoutDsnTest.php | 12 ++-- ...readcrumbsWithOldConfigKeyDisabledTest.php | 42 ------------- ...BreadcrumbsWithOldConfigKeyEnabledTest.php | 44 -------------- test/Sentry/SqlQueriesInBreadcrumbsTest.php | 51 ---------------- ...SentryLaravelTestCase.php => TestCase.php} | 14 +++-- test/Sentry/Tracing/EventHandlerTest.php | 29 +++++---- 21 files changed, 206 insertions(+), 335 deletions(-) create mode 100644 test/Sentry/ClientBuilderDecoratorTest.php rename test/Sentry/{CommandInfoInBreadcrumbsTest.php => EventHandler/ConsoleEventsTest.php} (71%) rename test/Sentry/{SqlBindingsInBreadcrumbsTest.php => EventHandler/DatabaseEventsTest.php} (50%) rename test/Sentry/{LaravelLogsInBreadcrumbsTest.php => EventHandler/LogEventsTest.php} (79%) delete mode 100644 test/Sentry/ExpectsException.php rename test/Sentry/{IntegrationsOptionTest.php => Laravel/LaravelIntegrationsOptionTest.php} (52%) rename test/Sentry/{Laravel => }/LogChannelTest.php (72%) delete mode 100644 test/Sentry/ServiceClientBuilderDecoratorTest.php delete mode 100644 test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php delete mode 100644 test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php delete mode 100644 test/Sentry/SqlQueriesInBreadcrumbsTest.php rename test/Sentry/{SentryLaravelTestCase.php => TestCase.php} (85%) diff --git a/src/Sentry/Laravel/LogChannel.php b/src/Sentry/Laravel/LogChannel.php index 77d3b65a..30d926f6 100644 --- a/src/Sentry/Laravel/LogChannel.php +++ b/src/Sentry/Laravel/LogChannel.php @@ -14,7 +14,7 @@ class LogChannel extends LogManager * * @return Logger */ - public function __invoke(array $config): Logger + public function __invoke(array $config = []): Logger { $handler = new SentryHandler( $this->app->make(HubInterface::class), diff --git a/test/Sentry/ClientBuilderDecoratorTest.php b/test/Sentry/ClientBuilderDecoratorTest.php new file mode 100644 index 00000000..dc23ae2f --- /dev/null +++ b/test/Sentry/ClientBuilderDecoratorTest.php @@ -0,0 +1,27 @@ +extend(ClientBuilderInterface::class, function (ClientBuilderInterface $clientBuilder) { + $clientBuilder->getOptions()->setEnvironment('from_service_container'); + + return $clientBuilder; + }); + } + + public function testClientHasEnvironmentSetFromDecorator(): void + { + $this->assertEquals( + 'from_service_container', + $this->getClientFromContainer()->getOptions()->getEnvironment() + ); + } +} diff --git a/test/Sentry/CommandInfoInBreadcrumbsTest.php b/test/Sentry/EventHandler/ConsoleEventsTest.php similarity index 71% rename from test/Sentry/CommandInfoInBreadcrumbsTest.php rename to test/Sentry/EventHandler/ConsoleEventsTest.php index 7632685c..ccad1562 100644 --- a/test/Sentry/CommandInfoInBreadcrumbsTest.php +++ b/test/Sentry/EventHandler/ConsoleEventsTest.php @@ -1,14 +1,15 @@ resetApplicationWithConfig([ 'sentry.breadcrumbs.command_info' => true, @@ -24,7 +25,7 @@ public function testCommandInfoAreRecordedWhenEnabled() $this->assertEquals('--foo=bar', $lastBreadcrumb->getMetadata()['input']); } - public function testCommandInfoAreRecordedWhenDisabled() + public function testCommandBreadcrumIsNotRecordedWhenDisabled(): void { $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.command_info' => false, @@ -37,14 +38,9 @@ public function testCommandInfoAreRecordedWhenDisabled() $this->assertEmpty($this->getCurrentBreadcrumbs()); } - private function dispatchCommandStartEvent() + private function dispatchCommandStartEvent(): void { - $dispatcher = $this->app['events']; - - $method = method_exists($dispatcher, 'dispatch') ? 'dispatch' : 'fire'; - - $this->app['events']->$method( - CommandStarting::class, + $this->dispatchLaravelEvent( new CommandStarting( 'test:command', new ArgvInput(['artisan', '--foo=bar']), diff --git a/test/Sentry/SqlBindingsInBreadcrumbsTest.php b/test/Sentry/EventHandler/DatabaseEventsTest.php similarity index 50% rename from test/Sentry/SqlBindingsInBreadcrumbsTest.php rename to test/Sentry/EventHandler/DatabaseEventsTest.php index e8e09300..0c517298 100644 --- a/test/Sentry/SqlBindingsInBreadcrumbsTest.php +++ b/test/Sentry/EventHandler/DatabaseEventsTest.php @@ -1,13 +1,34 @@ resetApplicationWithConfig([ + 'sentry.breadcrumbs.sql_queries' => true, + ]); + + $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.sql_queries')); + + $this->dispatchLaravelEvent(new QueryExecuted( + $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', + ['1'], + 10, + $this->getMockedConnection() + )); + + $lastBreadcrumb = $this->getLastBreadcrumb(); + + $this->assertEquals($query, $lastBreadcrumb->getMessage()); + } + public function testSqlBindingsAreRecordedWhenEnabled(): void { $this->resetApplicationWithConfig([ @@ -16,14 +37,11 @@ public function testSqlBindingsAreRecordedWhenEnabled(): void $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.sql_bindings')); - $connection = Mockery::mock(Connection::class) - ->shouldReceive('getName')->andReturn('test'); - $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', $bindings = ['1'], 10, - $connection + $this->getMockedConnection() )); $lastBreadcrumb = $this->getLastBreadcrumb(); @@ -32,6 +50,24 @@ public function testSqlBindingsAreRecordedWhenEnabled(): void $this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']); } + public function testSqlQueriesAreRecordedWhenDisabled(): void + { + $this->resetApplicationWithConfig([ + 'sentry.breadcrumbs.sql_queries' => false, + ]); + + $this->assertFalse($this->app['config']->get('sentry.breadcrumbs.sql_queries')); + + $this->dispatchLaravelEvent(new QueryExecuted( + 'SELECT * FROM breadcrumbs WHERE bindings = ?;', + ['1'], + 10, + $this->getMockedConnection() + )); + + $this->assertEmpty($this->getCurrentBreadcrumbs()); + } + public function testSqlBindingsAreRecordedWhenDisabled(): void { $this->resetApplicationWithConfig([ @@ -40,14 +76,11 @@ public function testSqlBindingsAreRecordedWhenDisabled(): void $this->assertFalse($this->app['config']->get('sentry.breadcrumbs.sql_bindings')); - $connection = Mockery::mock(Connection::class) - ->shouldReceive('getName')->andReturn('test'); - $this->dispatchLaravelEvent(new QueryExecuted( $query = 'SELECT * FROM breadcrumbs WHERE bindings <> ?;', ['1'], 10, - $connection + $this->getMockedConnection() )); $lastBreadcrumb = $this->getLastBreadcrumb(); @@ -55,4 +88,10 @@ public function testSqlBindingsAreRecordedWhenDisabled(): void $this->assertEquals($query, $lastBreadcrumb->getMessage()); $this->assertFalse(isset($lastBreadcrumb->getMetadata()['bindings'])); } + + private function getMockedConnection() + { + return Mockery::mock(Connection::class) + ->shouldReceive('getName')->andReturn('test'); + } } diff --git a/test/Sentry/LaravelLogsInBreadcrumbsTest.php b/test/Sentry/EventHandler/LogEventsTest.php similarity index 79% rename from test/Sentry/LaravelLogsInBreadcrumbsTest.php rename to test/Sentry/EventHandler/LogEventsTest.php index 724bec96..3f0b19cf 100644 --- a/test/Sentry/LaravelLogsInBreadcrumbsTest.php +++ b/test/Sentry/EventHandler/LogEventsTest.php @@ -1,11 +1,11 @@ assertFalse($this->app['config']->get('sentry.breadcrumbs.logs')); - $this->dispatchLaravelEvent('illuminate.log', [ - $level = 'debug', - $message = 'test message', - $context = ['1'], - ]); + $this->dispatchLaravelEvent(new MessageLogged('debug', 'test message')); $this->assertEmpty($this->getCurrentBreadcrumbs()); } diff --git a/test/Sentry/EventHandlerTest.php b/test/Sentry/EventHandlerTest.php index 4d1472c8..d5113527 100644 --- a/test/Sentry/EventHandlerTest.php +++ b/test/Sentry/EventHandlerTest.php @@ -9,42 +9,41 @@ class EventHandlerTest extends TestCase { - use ExpectsException; - - public function test_missing_event_handler_throws_exception() + public function testMissingEventHandlerThrowsException(): void { $handler = new EventHandler($this->app, []); - $this->safeExpectException(RuntimeException::class); + $this->expectException(RuntimeException::class); + /** @noinspection PhpUndefinedMethodInspection */ $handler->thisIsNotAHandlerAndShouldThrowAnException(); } - public function test_all_mapped_event_handlers_exist() + public function testAllMappedEventHandlersExist(): void { $this->tryAllEventHandlerMethods( - $this->getStaticPropertyValueFromClass(EventHandler::class, 'eventHandlerMap') + $this->getEventHandlerMapFromEventHandler('eventHandlerMap') ); } - public function test_all_mapped_auth_event_handlers_exist() + public function testAllMappedAuthEventHandlersExist(): void { $this->tryAllEventHandlerMethods( - $this->getStaticPropertyValueFromClass(EventHandler::class, 'authEventHandlerMap') + $this->getEventHandlerMapFromEventHandler('authEventHandlerMap') ); } - public function test_all_mapped_queue_event_handlers_exist() + public function testAllMappedQueueEventHandlersExist(): void { $this->tryAllEventHandlerMethods( - $this->getStaticPropertyValueFromClass(EventHandler::class, 'queueEventHandlerMap') + $this->getEventHandlerMapFromEventHandler('queueEventHandlerMap') ); } - public function test_all_mapped_octane_event_handlers_exist() + public function testAllMappedOctaneEventHandlersExist(): void { $this->tryAllEventHandlerMethods( - $this->getStaticPropertyValueFromClass(EventHandler::class, 'octaneEventHandlerMap') + $this->getEventHandlerMapFromEventHandler('octaneEventHandlerMap') ); } @@ -61,12 +60,12 @@ private function tryAllEventHandlerMethods(array $methods): void } } - private function getStaticPropertyValueFromClass($className, $attributeName) + private function getEventHandlerMapFromEventHandler($eventHandlerMapName) { - $class = new ReflectionClass($className); + $class = new ReflectionClass(EventHandler::class); $attributes = $class->getStaticProperties(); - return $attributes[$attributeName]; + return $attributes[$eventHandlerMapName]; } } diff --git a/test/Sentry/ExpectsException.php b/test/Sentry/ExpectsException.php deleted file mode 100644 index 67f657b7..00000000 --- a/test/Sentry/ExpectsException.php +++ /dev/null @@ -1,25 +0,0 @@ -expectException($class); - - return; - } - - if (method_exists($this, 'setExpectedException')) { - $this->setExpectedException($class); - - return; - } - - throw new RuntimeException('Could not expect an exception.'); - } -} diff --git a/test/Sentry/Integration/ExceptionContextIntegrationTest.php b/test/Sentry/Integration/ExceptionContextIntegrationTest.php index 664bab35..2adeb583 100644 --- a/test/Sentry/Integration/ExceptionContextIntegrationTest.php +++ b/test/Sentry/Integration/ExceptionContextIntegrationTest.php @@ -6,12 +6,11 @@ use Sentry\Event; use Sentry\EventHint; use Sentry\Laravel\Integration\ExceptionContextIntegration; -use Sentry\Laravel\Tests\SentryLaravelTestCase; -use Sentry\SentrySdk; +use Sentry\Laravel\Tests\TestCase; use Sentry\State\Scope; use function Sentry\withScope; -class ExceptionContextIntegrationTest extends SentryLaravelTestCase +class ExceptionContextIntegrationTest extends TestCase { public function testExceptionContextIntegrationIsRegistered(): void { @@ -58,7 +57,7 @@ public function invokeDataProvider(): iterable ]; } - private function generateExceptionWithContext($context) + private function generateExceptionWithContext($context): Exception { return new class($context) extends Exception { private $context; diff --git a/test/Sentry/IntegrationTest.php b/test/Sentry/IntegrationTest.php index a2ed7af2..590ca4ca 100644 --- a/test/Sentry/IntegrationTest.php +++ b/test/Sentry/IntegrationTest.php @@ -12,7 +12,7 @@ use Sentry\Tracing\TransactionSource; use function Sentry\withScope; -class IntegrationTest extends SentryLaravelTestCase +class IntegrationTest extends TestCase { public function testIntegrationIsRegistered(): void { diff --git a/test/Sentry/IntegrationsOptionTest.php b/test/Sentry/Laravel/LaravelIntegrationsOptionTest.php similarity index 52% rename from test/Sentry/IntegrationsOptionTest.php rename to test/Sentry/Laravel/LaravelIntegrationsOptionTest.php index eb47b129..fc5ed404 100644 --- a/test/Sentry/IntegrationsOptionTest.php +++ b/test/Sentry/Laravel/LaravelIntegrationsOptionTest.php @@ -1,19 +1,18 @@ resetApplicationWithConfig([ 'sentry.integrations' => [ @@ -30,10 +29,10 @@ public function testCustomIntegrationIsResolvedFromContainerByAlias() ], ]); - $this->assertNotNull($this->getHubFromContainer()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); + $this->assertNotNull($this->getClientFromContainer()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); } - public function testCustomIntegrationIsResolvedFromContainerByClass() + public function testCustomIntegrationIsResolvedFromContainerByClass(): void { $this->resetApplicationWithConfig([ 'sentry.integrations' => [ @@ -41,10 +40,10 @@ public function testCustomIntegrationIsResolvedFromContainerByClass() ], ]); - $this->assertNotNull($this->getHubFromContainer()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); + $this->assertNotNull($this->getClientFromContainer()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); } - public function testCustomIntegrationByInstance() + public function testCustomIntegrationByInstance(): void { $this->resetApplicationWithConfig([ 'sentry.integrations' => [ @@ -52,15 +51,12 @@ public function testCustomIntegrationByInstance() ], ]); - $this->assertNotNull($this->getHubFromContainer()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); + $this->assertNotNull($this->getClientFromContainer()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); } - /** - * Throws \ReflectionException in <=5.8 and \Illuminate\Contracts\Container\BindingResolutionException since 6.0 - */ - public function testCustomIntegrationThrowsExceptionIfNotResolvable() + public function testCustomIntegrationThrowsExceptionIfNotResolvable(): void { - $this->safeExpectException(Exception::class); + $this->expectException(BindingResolutionException::class); $this->resetApplicationWithConfig([ 'sentry.integrations' => [ @@ -69,9 +65,9 @@ public function testCustomIntegrationThrowsExceptionIfNotResolvable() ]); } - public function testIncorrectIntegrationEntryThrowsException() + public function testIncorrectIntegrationEntryThrowsException(): void { - $this->safeExpectException(RuntimeException::class); + $this->expectException(RuntimeException::class); $this->resetApplicationWithConfig([ 'sentry.integrations' => [ @@ -81,18 +77,16 @@ static function () { ]); } - public function testDisabledIntegrationsAreNotPresent() + public function testDisabledIntegrationsAreNotPresent(): void { - $integrations = $this->getHubFromContainer()->getClient()->getOptions()->getIntegrations(); - - foreach ($integrations as $integration) { - $this->ensureIsNotDisabledIntegration($integration); - } + $client = $this->getClientFromContainer(); - $this->assertTrue(true, 'Not all disabled integrations are actually disabled.'); + $this->assertNull($client->getIntegration(ErrorListenerIntegration::class)); + $this->assertNull($client->getIntegration(ExceptionListenerIntegration::class)); + $this->assertNull($client->getIntegration(FatalErrorListenerIntegration::class)); } - public function testDisabledIntegrationsAreNotPresentWithCustomIntegrations() + public function testDisabledIntegrationsAreNotPresentWithCustomIntegrations(): void { $this->resetApplicationWithConfig([ 'sentry.integrations' => [ @@ -100,10 +94,13 @@ public function testDisabledIntegrationsAreNotPresentWithCustomIntegrations() ], ]); - $this->assertNotNull($this->getHubFromContainer()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class)); - $this->assertNull($this->getHubFromContainer()->getClient()->getIntegration(ErrorListenerIntegration::class)); - $this->assertNull($this->getHubFromContainer()->getClient()->getIntegration(ExceptionListenerIntegration::class)); - $this->assertNull($this->getHubFromContainer()->getClient()->getIntegration(FatalErrorListenerIntegration::class)); + $client = $this->getClientFromContainer(); + + $this->assertNotNull($client->getIntegration(IntegrationsOptionTestIntegrationStub::class)); + + $this->assertNull($client->getIntegration(ErrorListenerIntegration::class)); + $this->assertNull($client->getIntegration(ExceptionListenerIntegration::class)); + $this->assertNull($client->getIntegration(FatalErrorListenerIntegration::class)); } } diff --git a/test/Sentry/Laravel/LogChannelTest.php b/test/Sentry/LogChannelTest.php similarity index 72% rename from test/Sentry/Laravel/LogChannelTest.php rename to test/Sentry/LogChannelTest.php index e0ecc850..c2514438 100644 --- a/test/Sentry/Laravel/LogChannelTest.php +++ b/test/Sentry/LogChannelTest.php @@ -1,30 +1,32 @@ app); - $logger = $logChannel([]); + + $logger = $logChannel(); $this->assertContainsOnlyInstancesOf(SentryHandler::class, $logger->getHandlers()); } - public function test_creating_handler_with_action_level_config() + public function testCreatingHandlerWithActionLevelConfig(): void { $logChannel = new LogChannel($this->app); + $logger = $logChannel(['action_level' => 'critical']); $this->assertContainsOnlyInstancesOf(FingersCrossedHandler::class, $logger->getHandlers()); $currentHandler = current($logger->getHandlers()); + $this->assertInstanceOf(SentryHandler::class, $currentHandler->getHandler()); $loggerWithoutActionLevel = $logChannel(['action_level' => null]); diff --git a/test/Sentry/ServiceClientBuilderDecoratorTest.php b/test/Sentry/ServiceClientBuilderDecoratorTest.php deleted file mode 100644 index 93342b85..00000000 --- a/test/Sentry/ServiceClientBuilderDecoratorTest.php +++ /dev/null @@ -1,35 +0,0 @@ -set('sentry.dsn', 'http://publickey:secretkey@sentry.dev/123'); - - $app->extend(ClientBuilderInterface::class, function (ClientBuilderInterface $clientBuilder) { - $clientBuilder->getOptions()->setEnvironment('from_service_container'); - - return $clientBuilder; - }); - } - - protected function getPackageProviders($app) - { - return [ - ServiceProvider::class, - ]; - } - - public function testClientHasCustomSerializer() - { - /** @var \Sentry\Options $options */ - $options = $this->app->make('sentry')->getClient()->getOptions(); - - $this->assertEquals('from_service_container', $options->getEnvironment()); - } -} diff --git a/test/Sentry/ServiceProviderTest.php b/test/Sentry/ServiceProviderTest.php index 218d3121..afd95ba3 100644 --- a/test/Sentry/ServiceProviderTest.php +++ b/test/Sentry/ServiceProviderTest.php @@ -2,43 +2,44 @@ namespace Sentry\Laravel\Tests; +use Orchestra\Testbench\TestCase; use Sentry\Laravel\Facade; use Sentry\Laravel\ServiceProvider; use Sentry\State\HubInterface; -class ServiceProviderTest extends \Orchestra\Testbench\TestCase +class ServiceProviderTest extends TestCase { - protected function getEnvironmentSetUp($app) + protected function getEnvironmentSetUp($app): void { - $app['config']->set('sentry.dsn', 'http://publickey:secretkey@sentry.dev/123'); + $app['config']->set('sentry.dsn', 'https://publickey:secretkey@sentry.dev/123'); $app['config']->set('sentry.error_types', E_ALL ^ E_DEPRECATED ^ E_USER_DEPRECATED); } - protected function getPackageProviders($app) + protected function getPackageProviders($app): array { return [ ServiceProvider::class, ]; } - protected function getPackageAliases($app) + protected function getPackageAliases($app): array { return [ 'Sentry' => Facade::class, ]; } - public function testIsBound() + public function testIsBound(): void { $this->assertTrue(app()->bound('sentry')); - $this->assertInstanceOf(HubInterface::class, app('sentry')); $this->assertSame(app('sentry'), Facade::getFacadeRoot()); + $this->assertInstanceOf(HubInterface::class, app('sentry')); } /** * @depends testIsBound */ - public function testEnvironment() + public function testEnvironment(): void { $this->assertEquals('testing', app('sentry')->getClient()->getOptions()->getEnvironment()); } @@ -46,12 +47,12 @@ public function testEnvironment() /** * @depends testIsBound */ - public function testDsnWasSetFromConfig() + public function testDsnWasSetFromConfig(): void { /** @var \Sentry\Options $options */ $options = app('sentry')->getClient()->getOptions(); - $this->assertEquals('http://sentry.dev', $options->getDsn()->getScheme() . '://' . $options->getDsn()->getHost()); + $this->assertEquals('https://sentry.dev', $options->getDsn()->getScheme() . '://' . $options->getDsn()->getHost()); $this->assertEquals(123, $options->getDsn()->getProjectId()); $this->assertEquals('publickey', $options->getDsn()->getPublicKey()); $this->assertEquals('secretkey', $options->getDsn()->getSecretKey()); @@ -60,7 +61,7 @@ public function testDsnWasSetFromConfig() /** * @depends testIsBound */ - public function testErrorTypesWasSetFromConfig() + public function testErrorTypesWasSetFromConfig(): void { $this->assertEquals( E_ALL ^ E_DEPRECATED ^ E_USER_DEPRECATED, diff --git a/test/Sentry/ServiceProviderWithCustomAliasTest.php b/test/Sentry/ServiceProviderWithCustomAliasTest.php index 2f44f3d7..c47b928e 100644 --- a/test/Sentry/ServiceProviderWithCustomAliasTest.php +++ b/test/Sentry/ServiceProviderWithCustomAliasTest.php @@ -2,33 +2,34 @@ namespace Sentry\Laravel\Tests; +use Orchestra\Testbench\TestCase; use Sentry\Laravel\Facade; use Sentry\Laravel\ServiceProvider; use Sentry\State\HubInterface; -class ServiceProviderWithCustomAliasTest extends \Orchestra\Testbench\TestCase +class ServiceProviderWithCustomAliasTest extends TestCase { - protected function getEnvironmentSetUp($app) + protected function getEnvironmentSetUp($app): void { $app['config']->set('custom-sentry.dsn', 'http://publickey:secretkey@sentry.dev/123'); $app['config']->set('custom-sentry.error_types', E_ALL ^ E_DEPRECATED ^ E_USER_DEPRECATED); } - protected function getPackageProviders($app) + protected function getPackageProviders($app): array { return [ CustomSentryServiceProvider::class, ]; } - protected function getPackageAliases($app) + protected function getPackageAliases($app): array { return [ 'CustomSentry' => CustomSentryFacade::class, ]; } - public function testIsBound() + public function testIsBound(): void { $this->assertTrue(app()->bound('custom-sentry')); $this->assertInstanceOf(HubInterface::class, app('custom-sentry')); @@ -38,7 +39,7 @@ public function testIsBound() /** * @depends testIsBound */ - public function testEnvironment() + public function testEnvironment(): void { $this->assertEquals('testing', app('custom-sentry')->getClient()->getOptions()->getEnvironment()); } @@ -46,7 +47,7 @@ public function testEnvironment() /** * @depends testIsBound */ - public function testDsnWasSetFromConfig() + public function testDsnWasSetFromConfig(): void { /** @var \Sentry\Options $options */ $options = app('custom-sentry')->getClient()->getOptions(); @@ -60,7 +61,7 @@ public function testDsnWasSetFromConfig() /** * @depends testIsBound */ - public function testErrorTypesWasSetFromConfig() + public function testErrorTypesWasSetFromConfig(): void { $this->assertEquals( E_ALL ^ E_DEPRECATED ^ E_USER_DEPRECATED, @@ -76,7 +77,7 @@ class CustomSentryServiceProvider extends ServiceProvider class CustomSentryFacade extends Facade { - protected static function getFacadeAccessor() + protected static function getFacadeAccessor(): string { return 'custom-sentry'; } diff --git a/test/Sentry/ServiceProviderWithEnvironmentFromConfigTest.php b/test/Sentry/ServiceProviderWithEnvironmentFromConfigTest.php index 6866b425..0c1e0a00 100644 --- a/test/Sentry/ServiceProviderWithEnvironmentFromConfigTest.php +++ b/test/Sentry/ServiceProviderWithEnvironmentFromConfigTest.php @@ -2,36 +2,36 @@ namespace Sentry; -use Sentry\Laravel\Tests\SentryLaravelTestCase; +use Sentry\Laravel\Tests\TestCase; -class ServiceProviderWithEnvironmentFromConfigTest extends SentryLaravelTestCase +class ServiceProviderWithEnvironmentFromConfigTest extends TestCase { - public function testSentryEnvironmentDefaultsToLaravelEnvironment() + public function testSentryEnvironmentDefaultsToLaravelEnvironment(): void { $this->assertEquals('testing', app()->environment()); } - public function testEmptySentryEnvironmentDefaultsToLaravelEnvironment() + public function testEmptySentryEnvironmentDefaultsToLaravelEnvironment(): void { $this->resetApplicationWithConfig([ 'sentry.environment' => '', ]); - $this->assertEquals('testing', $this->getHubFromContainer()->getClient()->getOptions()->getEnvironment()); + $this->assertEquals('testing', $this->getClientFromContainer()->getOptions()->getEnvironment()); $this->resetApplicationWithConfig([ 'sentry.environment' => null, ]); - $this->assertEquals('testing', $this->getHubFromContainer()->getClient()->getOptions()->getEnvironment()); + $this->assertEquals('testing', $this->getClientFromContainer()->getOptions()->getEnvironment()); } - public function testSentryEnvironmentDefaultGetsOverriddenByConfig() + public function testSentryEnvironmentDefaultGetsOverriddenByConfig(): void { $this->resetApplicationWithConfig([ - 'sentry.environment' => 'not_testing', + 'sentry.environment' => 'override_env', ]); - $this->assertEquals('not_testing', $this->getHubFromContainer()->getClient()->getOptions()->getEnvironment()); + $this->assertEquals('override_env', $this->getClientFromContainer()->getOptions()->getEnvironment()); } } diff --git a/test/Sentry/ServiceProviderWithoutDsnTest.php b/test/Sentry/ServiceProviderWithoutDsnTest.php index 247a3d6f..60c26ae9 100644 --- a/test/Sentry/ServiceProviderWithoutDsnTest.php +++ b/test/Sentry/ServiceProviderWithoutDsnTest.php @@ -7,19 +7,19 @@ class ServiceProviderWithoutDsnTest extends \Orchestra\Testbench\TestCase { - protected function getEnvironmentSetUp($app) + protected function getEnvironmentSetUp($app): void { $app['config']->set('sentry.dsn', null); } - protected function getPackageProviders($app) + protected function getPackageProviders($app): array { return [ ServiceProvider::class, ]; } - public function testIsBound() + public function testIsBound(): void { $this->assertTrue(app()->bound('sentry')); } @@ -27,7 +27,7 @@ public function testIsBound() /** * @depends testIsBound */ - public function testDsnIsNotSet() + public function testDsnIsNotSet(): void { $this->assertNull(app('sentry')->getClient()->getOptions()->getDsn()); } @@ -35,8 +35,8 @@ public function testDsnIsNotSet() /** * @depends testIsBound */ - public function testDidNotRegisterEvents() + public function testDidNotRegisterEvents(): void { - $this->assertEquals(false, app('events')->hasListeners('router.matched') && app('events')->hasListeners(RouteMatched::class)); + $this->assertEquals(false, app('events')->hasListeners(RouteMatched::class)); } } diff --git a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php deleted file mode 100644 index 3b97bfc1..00000000 --- a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php +++ /dev/null @@ -1,42 +0,0 @@ -all(); - - $config['sentry']['breadcrumbs.sql_bindings'] = false; - - $app['config'] = new Repository($config); - } - - public function testSqlBindingsAreRecordedWhenDisabledByOldConfigKey(): void - { - $this->assertFalse($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']); - - $connection = Mockery::mock(Connection::class) - ->shouldReceive('getName')->andReturn('test'); - - $this->dispatchLaravelEvent(new QueryExecuted( - $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', - ['1'], - 10, - $connection - )); - - $lastBreadcrumb = $this->getLastBreadcrumb(); - - $this->assertEquals($query, $lastBreadcrumb->getMessage()); - $this->assertFalse(isset($lastBreadcrumb->getMetadata()['bindings'])); - } -} diff --git a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php b/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php deleted file mode 100644 index f2a15f68..00000000 --- a/test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php +++ /dev/null @@ -1,44 +0,0 @@ -all(); - - $config['sentry']['breadcrumbs.sql_bindings'] = true; - - $app['config'] = new Repository($config); - } - - public function testSqlBindingsAreRecordedWhenEnabledByOldConfigKey(): void - { - $this->assertTrue($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']); - - $connection = Mockery::mock(Connection::class) - ->shouldReceive('getName')->andReturn('test'); - - - - $this->dispatchLaravelEvent(new QueryExecuted( - $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', - $bindings = ['1'], - 10, - $connection - )); - - $lastBreadcrumb = $this->getLastBreadcrumb(); - - $this->assertEquals($query, $lastBreadcrumb->getMessage()); - $this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']); - } -} diff --git a/test/Sentry/SqlQueriesInBreadcrumbsTest.php b/test/Sentry/SqlQueriesInBreadcrumbsTest.php deleted file mode 100644 index 6a26b506..00000000 --- a/test/Sentry/SqlQueriesInBreadcrumbsTest.php +++ /dev/null @@ -1,51 +0,0 @@ -resetApplicationWithConfig([ - 'sentry.breadcrumbs.sql_queries' => true, - ]); - - $this->assertTrue($this->app['config']->get('sentry.breadcrumbs.sql_queries')); - - $connection = Mockery::mock(Connection::class) - ->shouldReceive('getName')->andReturn('test'); - - $this->dispatchLaravelEvent(new QueryExecuted( - $query = 'SELECT * FROM breadcrumbs WHERE bindings = ?;', - ['1'], - 10, - $connection - )); - - $lastBreadcrumb = $this->getLastBreadcrumb(); - - $this->assertEquals($query, $lastBreadcrumb->getMessage()); - } - - public function testSqlQueriesAreRecordedWhenDisabled() - { - $this->resetApplicationWithConfig([ - 'sentry.breadcrumbs.sql_queries' => false, - ]); - - $this->assertFalse($this->app['config']->get('sentry.breadcrumbs.sql_queries')); - - $this->dispatchLaravelEvent('illuminate.query', [ - 'SELECT * FROM breadcrumbs WHERE bindings <> ?;', - ['1'], - 10, - 'test', - ]); - - $this->assertEmpty($this->getCurrentBreadcrumbs()); - } -} diff --git a/test/Sentry/SentryLaravelTestCase.php b/test/Sentry/TestCase.php similarity index 85% rename from test/Sentry/SentryLaravelTestCase.php rename to test/Sentry/TestCase.php index fdb72835..c47f99ea 100644 --- a/test/Sentry/SentryLaravelTestCase.php +++ b/test/Sentry/TestCase.php @@ -4,6 +4,7 @@ use ReflectionMethod; use Sentry\Breadcrumb; +use Sentry\ClientInterface; use Sentry\State\Scope; use ReflectionProperty; use Sentry\Laravel\Tracing; @@ -11,14 +12,14 @@ use Sentry\Laravel\ServiceProvider; use Orchestra\Testbench\TestCase as LaravelTestCase; -abstract class SentryLaravelTestCase extends LaravelTestCase +abstract class TestCase extends LaravelTestCase { protected $setupConfig = [ // Set config here before refreshing the app to set it in the container before Sentry is loaded // or use the `$this->resetApplicationWithConfig([ /* config */ ]);` helper method ]; - protected function getEnvironmentSetUp($app) + protected function getEnvironmentSetUp($app): void { $app['config']->set('sentry.dsn', 'http://publickey:secretkey@sentry.dev/123'); @@ -27,7 +28,7 @@ protected function getEnvironmentSetUp($app) } } - protected function getPackageProviders($app) + protected function getPackageProviders($app): array { return [ ServiceProvider::class, @@ -35,7 +36,7 @@ protected function getPackageProviders($app) ]; } - protected function resetApplicationWithConfig(array $config) + protected function resetApplicationWithConfig(array $config): void { $this->setupConfig = $config; @@ -52,6 +53,11 @@ protected function getHubFromContainer(): HubInterface return $this->app->make('sentry'); } + protected function getClientFromContainer(): ClientInterface + { + return $this->getHubFromContainer()->getClient(); + } + protected function getCurrentScope(): Scope { $hub = $this->getHubFromContainer(); diff --git a/test/Sentry/Tracing/EventHandlerTest.php b/test/Sentry/Tracing/EventHandlerTest.php index 4e67b9cb..8fad3b4f 100644 --- a/test/Sentry/Tracing/EventHandlerTest.php +++ b/test/Sentry/Tracing/EventHandlerTest.php @@ -3,29 +3,34 @@ namespace Sentry\Laravel\Tests\Tracing; use ReflectionClass; -use Sentry\Laravel\Tests\SentryLaravelTestCase; +use Sentry\Laravel\Tests\TestCase; use Sentry\Laravel\Tracing\BacktraceHelper; use RuntimeException; -use Sentry\Laravel\Tests\ExpectsException; use Sentry\Laravel\Tracing\EventHandler; -class EventHandlerTest extends SentryLaravelTestCase +class EventHandlerTest extends TestCase { - use ExpectsException; - - public function test_missing_event_handler_throws_exception() + public function testMissingEventHandlerThrowsException(): void { - $this->safeExpectException(RuntimeException::class); + $this->expectException(RuntimeException::class); $handler = new EventHandler([], $this->app->make(BacktraceHelper::class)); + /** @noinspection PhpUndefinedMethodInspection */ $handler->thisIsNotAHandlerAndShouldThrowAnException(); } - public function test_all_mapped_event_handlers_exist() + public function testAllMappedEventHandlersExist(): void + { + $this->tryAllEventHandlerMethods( + $this->getEventHandlerMapFromEventHandler('eventHandlerMap') + ); + } + + public function testAllMappedQueueEventHandlersExist(): void { $this->tryAllEventHandlerMethods( - $this->getStaticPropertyValueFromClass(EventHandler::class, 'eventHandlerMap') + $this->getEventHandlerMapFromEventHandler('queueEventHandlerMap') ); } @@ -42,12 +47,12 @@ private function tryAllEventHandlerMethods(array $methods): void } } - private function getStaticPropertyValueFromClass($className, $attributeName) + private function getEventHandlerMapFromEventHandler($eventHandlerMapName) { - $class = new ReflectionClass($className); + $class = new ReflectionClass(EventHandler::class); $attributes = $class->getStaticProperties(); - return $attributes[$attributeName]; + return $attributes[$eventHandlerMapName]; } } From df05e857b5ead1a3729bde6657a573e68ae71d65 Mon Sep 17 00:00:00 2001 From: Michi Hoffmann Date: Mon, 17 Oct 2022 17:36:39 +0200 Subject: [PATCH 11/12] docs: Restructure the 3.0.0 changelog (#597) --- CHANGELOG.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5eefcec..fcd95b74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ ## 3.0.0 +**New features** + +- We are now creating more spans to give you better insights into the performance of your application + - Add a `http.client` span. This span indicates the time that is spent when using the Laravel HTTP client (#585) + - Add a `http.route` span. This span indicates the time that is spent inside a controller method or route closure (#593) + - Add a `db.transaction` span. This span indicates the time that is spent inside a database transaction (#594) +- Add support for [Dynamic Sampling](https://docs.sentry.io/product/data-management-settings/dynamic-sampling/), allowing developers to set a server-side sampling rate without the need to re-deploy their applications + - Add support for Dynamic Sampling (#572) + **Breaking changes** - Laravel Lumen is no longer supported @@ -11,18 +20,14 @@ - Laravel versions 5.0 - 5.8 are no longer supported - Drop support for Laravel 5.x (#581) - Remove `Sentry\Integration::extractNameForRoute()`, it's alternative `Sentry\Integration::extractNameAndSourceForRoute()` is marked as `@internal` (#580) -- Remove extracting route name or controller for transaction names (#583). This unifies the transaction names to a more concise format. -- Remove internal `Sentry\Integration::currentTracingSpan()`, use `SentrySdk::getCurrentHub()->getSpan()` if you were using this internal method. (#592) +- Remove internal `Sentry\Integration::currentTracingSpan()`, use `SentrySdk::getCurrentHub()->getSpan()` if you were using this internal method (#592) **Other changes** -- Add support for Dynamic Sampling (#572) - Set the tracing transaction name on the `Illuminate\Routing\Events\RouteMatched` instead of at the end of the request (#580) -- Add tracing span for Laravel HTTP client requests (#585) +- Remove extracting route name or controller for transaction names (#583). This unifies the transaction names to a more concise format. - Simplify Sentry meta tag retrieval, by adding `Sentry\Laravel\Integration::sentryMeta()` (#586) - Fix tracing with nested queue jobs (mostly when running jobs in the `sync` driver) (#592) -- Add a `http.route` span, this span indicates the time that is spent inside a controller method or route closure (#593) -- Add a `db.transaction` span, this span indicates the time that is spent inside a database transaction (#594) ## 2.14.2 From 36a2b74fcf4d735c9c385b8d4d97b40bc27f433a Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 18 Oct 2022 12:38:57 +0200 Subject: [PATCH 12/12] Update branch alias for new branch structure --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 65d7186c..c3643221 100644 --- a/composer.json +++ b/composer.json @@ -64,7 +64,7 @@ }, "extra": { "branch-alias": { - "dev-3.x": "3.x-dev", + "dev-master": "3.x-dev", "dev-2.x": "2.x-dev", "dev-1.x": "1.x-dev", "dev-0.x": "0.x-dev"