From f838c9b9c4644e61b45d02999af6e948deb97bd5 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 7 Jan 2025 23:09:35 +0100 Subject: [PATCH 1/5] Add Scheduled Task Tracing --- .../Features/ConsoleSchedulingIntegration.php | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php b/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php index e359c941..b7af332a 100644 --- a/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php +++ b/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php @@ -4,20 +4,32 @@ use DateTimeZone; use Illuminate\Console\Application as ConsoleApplication; +use Illuminate\Console\Events\ScheduledTaskFailed; +use Illuminate\Console\Events\ScheduledTaskFinished; +use Illuminate\Console\Events\ScheduledTaskStarting; use Illuminate\Console\Scheduling\Event as SchedulingEvent; use Illuminate\Contracts\Cache\Factory as Cache; use Illuminate\Contracts\Cache\Repository; +use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Support\Str; use RuntimeException; use Sentry\CheckIn; use Sentry\CheckInStatus; use Sentry\Event as SentryEvent; +use Sentry\Laravel\Features\Concerns\TracksPushedScopesAndSpans; use Sentry\MonitorConfig; use Sentry\MonitorSchedule; use Sentry\SentrySdk; +use Sentry\Tracing\SpanStatus; +use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; class ConsoleSchedulingIntegration extends Feature { + use TracksPushedScopesAndSpans { + pushScope as private pushScopeTrait; + } + /** * @var string|null */ @@ -105,9 +117,13 @@ public function isApplicable(): bool return true; } - public function onBoot(): void + public function onBoot(Dispatcher $events): void { $this->shouldHandleCheckIn = true; + + $events->listen(ScheduledTaskStarting::class, [$this, 'handleScheduledTaskStarting']); + $events->listen(ScheduledTaskFinished::class, [$this, 'handleScheduledTaskFinished']); + $events->listen(ScheduledTaskFailed::class, [$this, 'handleScheduledTaskFailed']); } public function onBootInactive(): void @@ -120,6 +136,35 @@ public function useCacheStore(?string $name): void $this->cacheStore = $name; } + public function handleScheduledTaskStarting(ScheduledTaskStarting $event): void + { + if (!$event->task) { + return; + } + + $context = TransactionContext::make() + ->setName($event->task->description) + ->setSource(TransactionSource::task()) + ->setOp('console.command') + ->setStartTimestamp(microtime(true)); + + $transaction = SentrySdk::getCurrentHub()->startTransaction($context); + + $this->pushSpan($transaction); + } + + public function handleScheduledTaskFinished(): void + { + $this->maybeFinishSpan(SpanStatus::ok()); + $this->maybePopScope(); + } + + public function handleScheduledTaskFailed(): void + { + $this->maybeFinishSpan(SpanStatus::internalError()); + $this->maybePopScope(); + } + private function startCheckIn( ?string $slug, SchedulingEvent $scheduled, From e31457bf7f99ad1a54b44ce55bad4d984d220abb Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 14 Jan 2025 14:01:23 +0100 Subject: [PATCH 2/5] Resolve name for all queued job types --- .../Features/ConsoleSchedulingIntegration.php | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php b/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php index b7af332a..1aa1d884 100644 --- a/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php +++ b/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php @@ -26,9 +26,7 @@ class ConsoleSchedulingIntegration extends Feature { - use TracksPushedScopesAndSpans { - pushScope as private pushScopeTrait; - } + use TracksPushedScopesAndSpans; /** * @var string|null @@ -142,12 +140,19 @@ public function handleScheduledTaskStarting(ScheduledTaskStarting $event): void return; } + // When scheduling a command class the command name will be the most descriptive + // When a job is scheduled the command name is `null` and the job class name (or display name) is set as the description + // When a closure is scheduled both the command name and description are `null` + $name = $this->getCommandNameForScheduled($event->task) ?? $event->task->description ?? 'Closure'; + + dump($name); + $context = TransactionContext::make() - ->setName($event->task->description) + ->setName($name) ->setSource(TransactionSource::task()) - ->setOp('console.command') + ->setOp('console.command.scheduled') ->setStartTimestamp(microtime(true)); - + $transaction = SentrySdk::getCurrentHub()->startTransaction($context); $this->pushSpan($transaction); @@ -293,6 +298,18 @@ private function makeSlugForScheduled(SchedulingEvent $scheduled): string return "scheduled_{$generatedSlug}"; } + private function getCommandNameForScheduled(SchedulingEvent $scheduled): ?string + { + if (!$scheduled->command) { + return null; + } + + // The command string always starts with the PHP binary, so we remove it since it's not relevant to the slug + return trim( + Str::after($scheduled->command, ConsoleApplication::phpBinary() . ' ' . ConsoleApplication::artisanBinary()) + ); + } + private function resolveCache(): Repository { return $this->container()->make(Cache::class)->store($this->cacheStore); From 70329b1d8009901bfd05e6aad2a5ccb989951932 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 14 Jan 2025 14:01:29 +0100 Subject: [PATCH 3/5] Add tests --- .../ConsoleSchedulingIntegrationTest.php | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php b/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php index b7794298..29283f53 100644 --- a/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php +++ b/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php @@ -4,6 +4,8 @@ use DateTimeZone; use Illuminate\Console\Scheduling\Schedule; +use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Foundation\Queue\Queueable; use RuntimeException; use Sentry\Laravel\Tests\TestCase; use Illuminate\Console\Scheduling\Event; @@ -121,8 +123,59 @@ public function testScheduleMacroIsRegisteredWithoutDsnSet(): void $this->assertTrue(Event::hasMacro('sentryMonitor')); } + /** @define-env envSamplingAllTransactions */ + public function testScheduledCommandCreatesTransaction(): void + { + $this->getScheduler()->command('inspire')->everyMinute(); + + $this->artisan('schedule:run'); + + $this->assertSentryTransactionCount(1); + + $transaction = $this->getLastSentryEvent(); + + $this->assertEquals('inspire', $transaction->getTransaction()); + } + + /** @define-env envSamplingAllTransactions */ + public function testScheduledClosureCreatesTransaction(): void + { + $this->getScheduler()->call(function () {})->everyMinute(); + + $this->artisan('schedule:run'); + + $this->assertSentryTransactionCount(1); + + $transaction = $this->getLastSentryEvent(); + + $this->assertEquals('Closure', $transaction->getTransaction()); + } + + /** @define-env envSamplingAllTransactions */ + public function testScheduledJobCreatesTransaction(): void + { + $this->getScheduler()->job(ScheduledQueuedJob::class)->everyMinute(); + + $this->artisan('schedule:run'); + + $this->assertSentryTransactionCount(1); + + $transaction = $this->getLastSentryEvent(); + + $this->assertEquals(ScheduledQueuedJob::class, $transaction->getTransaction()); + } + private function getScheduler(): Schedule { return $this->app->make(Schedule::class); } } + +class ScheduledQueuedJob implements ShouldQueue +{ + use Queueable; + + public function handle(): void + { + } +} From df4f41c2cee8875cb91df4ce9ac5b1651182ccdb Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Tue, 14 Jan 2025 14:04:02 +0100 Subject: [PATCH 4/5] CS --- src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php b/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php index 1aa1d884..76e10b0f 100644 --- a/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php +++ b/src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php @@ -145,8 +145,6 @@ public function handleScheduledTaskStarting(ScheduledTaskStarting $event): void // When a closure is scheduled both the command name and description are `null` $name = $this->getCommandNameForScheduled($event->task) ?? $event->task->description ?? 'Closure'; - dump($name); - $context = TransactionContext::make() ->setName($name) ->setSource(TransactionSource::task()) @@ -304,7 +302,7 @@ private function getCommandNameForScheduled(SchedulingEvent $scheduled): ?string return null; } - // The command string always starts with the PHP binary, so we remove it since it's not relevant to the slug + // The command string always starts with the PHP binary and artisan binary, so we remove it since it's not relevant to the name return trim( Str::after($scheduled->command, ConsoleApplication::phpBinary() . ' ' . ConsoleApplication::artisanBinary()) ); From bc685b19429994089095d149ff38149c1fb4a98c Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 16 Jan 2025 11:01:24 +0100 Subject: [PATCH 5/5] Use different Queueable trait --- test/Sentry/Features/ConsoleSchedulingIntegrationTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php b/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php index 29283f53..f0bc8c0a 100644 --- a/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php +++ b/test/Sentry/Features/ConsoleSchedulingIntegrationTest.php @@ -3,9 +3,9 @@ namespace Sentry\Laravel\Tests\Features; use DateTimeZone; +use Illuminate\Bus\Queueable; use Illuminate\Console\Scheduling\Schedule; use Illuminate\Contracts\Queue\ShouldQueue; -use Illuminate\Foundation\Queue\Queueable; use RuntimeException; use Sentry\Laravel\Tests\TestCase; use Illuminate\Console\Scheduling\Event;