Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Try harder to generate a slug for a cron monitor #977

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Try harder to generate a slug for a cron monitor #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 24 additions & 13 deletions src/Sentry/Laravel/Features/ConsoleSchedulingIntegration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public function register(): void
?int $recoveryThreshold = null
) 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.');
if ($monitorSlug === null && empty($this->command) && empty($this->description)) {
throw new RuntimeException('The command and description are not set, please set a slug manually for this scheduled command using the `sentryMonitor(\'your-monitor-slug\')` macro.');
}

return $this
Expand Down Expand Up @@ -266,7 +266,7 @@ private function createCheckIn(string $slug, CheckInStatus $status, ?string $id
$options = SentrySdk::getCurrentHub()->getClient()->getOptions();

return new CheckIn(
$slug,
Str::limit($slug, 128, ''),
$status,
$id,
$options->getRelease(),
Expand All @@ -282,17 +282,28 @@ private function buildCacheKey(string $mutex, string $slug): string

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())
if (empty($scheduled->command)) {
if (!empty($scheduled->description) && class_exists($scheduled->description)) {
$generatedSlug = Str::slug(
// We reverse the class name to have the class name at the start of the slug instead of at the end (and possibly cut off)
implode('_', array_reverse(explode('\\', $scheduled->description)))
);
} else {
$generatedSlug = Str::slug($scheduled->description);
}
} else {
$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}";
}
Expand Down
23 changes: 21 additions & 2 deletions test/Sentry/Features/ConsoleSchedulingIntegrationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function testScheduleMacroWithTimeZone(): void
$this->assertEquals($expectedTimezone, $finishCheckInEvent->getCheckIn()->getMonitorConfig()->getTimezone());
}

public function testScheduleMacroAutomaticSlug(): void
public function testScheduleMacroAutomaticSlugForCommand(): void
{
/** @var Event $scheduledEvent */
$scheduledEvent = $this->getScheduler()->command('inspire')->sentryMonitor();
Expand All @@ -78,7 +78,26 @@ public function testScheduleMacroAutomaticSlug(): void
$this->assertEquals('scheduled_artisan-inspire', $finishCheckInEvent->getCheckIn()->getMonitorSlug());
}

public function testScheduleMacroWithoutSlugOrCommandName(): void
public function testScheduleMacroAutomaticSlugForJob(): void
{
/** @var Event $scheduledEvent */
$scheduledEvent = $this->getScheduler()->job(ScheduledQueuedJob::class)->sentryMonitor();

$scheduledEvent->run($this->app);

// We expect a total of 2 events to be sent to Sentry:
// 1. The start check-in event
// 2. The finish check-in event
$this->assertSentryCheckInCount(2);

$finishCheckInEvent = $this->getLastSentryEvent();

$this->assertNotNull($finishCheckInEvent->getCheckIn());
// Scheduled is duplicated here because of the class name of the queued job, this is not a bug just unfortunate naming for the test class
$this->assertEquals('scheduled_scheduledqueuedjob-features-tests-laravel-sentry', $finishCheckInEvent->getCheckIn()->getMonitorSlug());
}

public function testScheduleMacroWithoutSlugCommandOrDescriptionOrName(): void
{
$this->expectException(RuntimeException::class);

Expand Down