From 7301039acda11201b7c1ffe33935119b181715d7 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 5 Apr 2023 20:48:22 +0200 Subject: [PATCH 1/2] Allow automatic slug creation for cron monitoring --- .../Laravel/Features/ConsoleIntegration.php | 87 +++++++++++++++---- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/src/Sentry/Laravel/Features/ConsoleIntegration.php b/src/Sentry/Laravel/Features/ConsoleIntegration.php index 196a231f..b5b542a4 100644 --- a/src/Sentry/Laravel/Features/ConsoleIntegration.php +++ b/src/Sentry/Laravel/Features/ConsoleIntegration.php @@ -2,12 +2,17 @@ namespace Sentry\Laravel\Features; +use Illuminate\Console\Application as ConsoleApplication; use Illuminate\Console\Scheduling\Event as SchedulingEvent; use Illuminate\Contracts\Cache\Factory as Cache; use Illuminate\Contracts\Foundation\Application; +use Illuminate\Support\Str; +use RuntimeException; use Sentry\CheckIn; use Sentry\CheckInStatus; use Sentry\Event as SentryEvent; +use Sentry\MonitorConfig; +use Sentry\MonitorSchedule; use Sentry\SentrySdk; class ConsoleIntegration extends Feature @@ -31,27 +36,36 @@ public function setup(Cache $cache): void { $this->cache = $cache; - $startCheckIn = function (string $mutex, string $slug, bool $useCache, int $useCacheTtlInMinutes) { - $this->startCheckIn($mutex, $slug, $useCache, $useCacheTtlInMinutes); + $startCheckIn = function (?string $slug, SchedulingEvent $scheduled, ?int $checkInMargin, ?int $maxRuntime, bool $updateMonitorConfig) { + $this->startCheckIn($slug, $scheduled, $checkInMargin, $maxRuntime, $updateMonitorConfig); }; - $finishCheckIn = function (string $mutex, string $slug, CheckInStatus $status, bool $useCache) { - $this->finishCheckIn($mutex, $slug, $status, $useCache); + $finishCheckIn = function (?string $slug, SchedulingEvent $scheduled, CheckInStatus $status) { + $this->finishCheckIn($slug, $scheduled, $status); }; - SchedulingEvent::macro('sentryMonitor', function (string $monitorSlug) use ($startCheckIn, $finishCheckIn) { + SchedulingEvent::macro('sentryMonitor', function ( + ?string $monitorSlug = null, + ?int $checkInMargin = null, + ?int $maxRuntime = null, + bool $updateMonitorConfig = true + ) use ($startCheckIn, $finishCheckIn) { /** @var SchedulingEvent $this */ + if ($monitorSlug === null && $this->command === null) { + throw new RuntimeException('The command string is null, please set a slug manually for this scheduled command using the `sentryMonitor(\'your-monitor-slug\')` macro.'); + } + return $this - ->before(function () use ($startCheckIn, $monitorSlug) { + ->before(function () use ($startCheckIn, $monitorSlug, $checkInMargin, $maxRuntime, $updateMonitorConfig) { /** @var SchedulingEvent $this */ - $startCheckIn($this->mutexName(), $monitorSlug, $this->runInBackground, $this->expiresAt); + $startCheckIn($monitorSlug, $this, $checkInMargin, $maxRuntime, $updateMonitorConfig); }) ->onSuccess(function () use ($finishCheckIn, $monitorSlug) { /** @var SchedulingEvent $this */ - $finishCheckIn($this->mutexName(), $monitorSlug, CheckInStatus::ok(), $this->runInBackground); + $finishCheckIn($monitorSlug, $this, CheckInStatus::ok()); }) ->onFailure(function () use ($finishCheckIn, $monitorSlug) { /** @var SchedulingEvent $this */ - $finishCheckIn($this->mutexName(), $monitorSlug, CheckInStatus::error(), $this->runInBackground); + $finishCheckIn($monitorSlug, $this, CheckInStatus::error()); }); }); } @@ -64,32 +78,50 @@ public function setupInactive(): void }); } - private function startCheckIn(string $mutex, string $slug, bool $useCache, int $useCacheTtlInMinutes): void + private function startCheckIn(?string $slug, SchedulingEvent $scheduled, ?int $checkInMargin, ?int $maxRuntime, bool $updateMonitorConfig): void { - $checkIn = $this->createCheckIn($slug, CheckInStatus::inProgress()); + $checkInSlug = $slug ?? $this->makeSlugForScheduled($scheduled); + + $checkIn = $this->createCheckIn($checkInSlug, CheckInStatus::inProgress()); + + if ($updateMonitorConfig || $slug === null) { + $checkIn->setMonitorConfig(new MonitorConfig( + new MonitorSchedule( + MonitorSchedule::TYPE_CRONTAB, + $scheduled->getExpression() + ), + $checkInMargin, + $maxRuntime, + $scheduled->timezone, + )); + } - $cacheKey = $this->buildCacheKey($mutex, $slug); + $cacheKey = $this->buildCacheKey($scheduled->mutexName(), $checkInSlug); $this->checkInStore[$cacheKey] = $checkIn; - if ($useCache) { - $this->cache->store()->put($cacheKey, $checkIn->getId(), $useCacheTtlInMinutes * 60); + if ($scheduled->runInBackground) { + $this->cache->store()->put($cacheKey, $checkIn->getId(), $scheduled->expiresAt * 60); } $this->sendCheckIn($checkIn); } - private function finishCheckIn(string $mutex, string $slug, CheckInStatus $status, bool $useCache): void + private function finishCheckIn(?string $slug, SchedulingEvent $scheduled, CheckInStatus $status): void { - $cacheKey = $this->buildCacheKey($mutex, $slug); + $mutex = $scheduled->mutexName(); + + $checkInSlug = $slug ?? $this->makeSlugForScheduled($scheduled); + + $cacheKey = $this->buildCacheKey($mutex, $checkInSlug); $checkIn = $this->checkInStore[$cacheKey] ?? null; - if ($checkIn === null && $useCache) { + if ($checkIn === null && $scheduled->runInBackground) { $checkInId = $this->cache->store()->get($cacheKey); if ($checkInId !== null) { - $checkIn = $this->createCheckIn($slug, $status, $checkInId); + $checkIn = $this->createCheckIn($checkInSlug, $status, $checkInId); } } @@ -101,7 +133,7 @@ private function finishCheckIn(string $mutex, string $slug, CheckInStatus $statu // We don't need to keep the checkIn ID stored since we finished executing the command unset($this->checkInStore[$mutex]); - if ($useCache) { + if ($scheduled->runInBackground) { $this->cache->store()->forget($cacheKey); } @@ -136,4 +168,21 @@ private function buildCacheKey(string $mutex, string $slug): string // We use the mutex name as part of the cache key to avoid collisions between the same commands with the same schedule but with different slugs return 'sentry:checkIn:' . sha1("{$mutex}:{$slug}"); } + + private function makeSlugForScheduled(SchedulingEvent $scheduled): string + { + $generatedSlug = Str::slug( + Str::replace( + // `:` is commonly used in the command name, so we replace it with `-` to avoid it being stripped out by the slug function + ':', + '-', + trim( + // The command string always starts with the PHP binary, so we remove it since it's not relevant to the slug + Str::after($scheduled->command, ConsoleApplication::phpBinary()) + ) + ) + ); + + return "scheduled_{$generatedSlug}"; + } } From ef78e911ca01f2b17eb8fa989e669afb111309e9 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 23 Jun 2023 22:59:08 +0200 Subject: [PATCH 2/2] Update code with latest upstream changes --- composer.json | 2 +- .../Laravel/Features/ConsoleIntegration.php | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index 5c631d10..039969f7 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ "require": { "php": "^7.2 | ^8.0", "illuminate/support": "^6.0 | ^7.0 | ^8.0 | ^9.0 | ^10.0", - "sentry/sentry": "^3.19", + "sentry/sentry": "^3.20", "sentry/sdk": "^3.4", "symfony/psr-http-message-bridge": "^1.0 | ^2.0", "nyholm/psr7": "^1.0" diff --git a/src/Sentry/Laravel/Features/ConsoleIntegration.php b/src/Sentry/Laravel/Features/ConsoleIntegration.php index b5b542a4..d0c55b25 100644 --- a/src/Sentry/Laravel/Features/ConsoleIntegration.php +++ b/src/Sentry/Laravel/Features/ConsoleIntegration.php @@ -36,7 +36,7 @@ public function setup(Cache $cache): void { $this->cache = $cache; - $startCheckIn = function (?string $slug, SchedulingEvent $scheduled, ?int $checkInMargin, ?int $maxRuntime, bool $updateMonitorConfig) { + $startCheckIn = function (?string $slug, SchedulingEvent $scheduled, ?int $checkInMargin, ?int $maxRuntime, bool $updateMonitorConfig) { $this->startCheckIn($slug, $scheduled, $checkInMargin, $maxRuntime, $updateMonitorConfig); }; $finishCheckIn = function (?string $slug, SchedulingEvent $scheduled, CheckInStatus $status) { @@ -72,8 +72,13 @@ public function setup(Cache $cache): void public function setupInactive(): void { - SchedulingEvent::macro('sentryMonitor', function (string $monitorSlug) { - // When there is no Sentry DSN set there is nothing for us to do, but we still want to allow the user to setup the macro + // This is an exact copy of the macro above, but without doing anything so that even when no DSN is configured the user can still use the macro + SchedulingEvent::macro('sentryMonitor', function ( + ?string $monitorSlug = null, + ?int $checkInMargin = null, + ?int $maxRuntime = null, + bool $updateMonitorConfig = true + ) { return $this; }); } @@ -86,13 +91,10 @@ private function startCheckIn(?string $slug, SchedulingEvent $scheduled, ?int $c if ($updateMonitorConfig || $slug === null) { $checkIn->setMonitorConfig(new MonitorConfig( - new MonitorSchedule( - MonitorSchedule::TYPE_CRONTAB, - $scheduled->getExpression() - ), + MonitorSchedule::crontab($scheduled->getExpression()), $checkInMargin, $maxRuntime, - $scheduled->timezone, + $scheduled->timezone )); }