diff --git a/ProcessMaker/Console/Commands/ScheduleMultitenantRun.php b/ProcessMaker/Console/Commands/ScheduleMultitenantRun.php new file mode 100644 index 0000000000..15c28115c7 --- /dev/null +++ b/ProcessMaker/Console/Commands/ScheduleMultitenantRun.php @@ -0,0 +1,111 @@ +get()) { + $message = 'schedule:tenants-run is already running; skipping this invocation.'; + Log::error($message); + $this->error($message); + + return self::FAILURE; + } + + $startedAt = microtime(true); + + try { + return $this->runSchedules(); + } catch (Throwable $e) { + Log::error('schedule:tenants-run failed: ' . $e->getMessage(), [ + 'exception' => $e, + ]); + + throw $e; + } finally { + $lock->release(); + Metrics::histogramObserve( + self::DURATION_METRIC, + 'Duration of the multitenant scheduler run in seconds', + [], + self::DURATION_BUCKETS, + microtime(true) - $startedAt + ); + } + } + + private function runSchedules(): int + { + if (config('app.multitenancy') === false) { + return $this->call('schedule:run'); + } + + $schedule = app(Schedule::class); + $tenants = Tenant::query()->cursor(); + + $ranForTenant = false; + + foreach ($tenants as $tenant) { + $ranForTenant = true; + $this->info("Running schedule for tenant [{$tenant->id}]"); + + try { + $tenant->makeCurrent(); + $this->call('schedule:run'); + } finally { + Tenant::forgetCurrent(); + + if ($schedule instanceof FastSchedule) { + $schedule->clearTenantEvents(); + } + } + } + + if (!$ranForTenant) { + $this->info('No tenants found.'); + } + + return self::SUCCESS; + } +} diff --git a/ProcessMaker/Console/Kernel.php b/ProcessMaker/Console/Kernel.php index df4ce44dd8..e98f90a3b0 100644 --- a/ProcessMaker/Console/Kernel.php +++ b/ProcessMaker/Console/Kernel.php @@ -4,6 +4,7 @@ use Illuminate\Console\Scheduling\Schedule; use Illuminate\Foundation\Console\Kernel as ConsoleKernel; +use ProcessMaker\Console\Scheduling\FastSchedule; class Kernel extends ConsoleKernel { @@ -14,6 +15,18 @@ class Kernel extends ConsoleKernel */ protected $commands = []; + /** + * Resolve a console schedule instance that runs Artisan commands in-process. + * + * @return Schedule + */ + public function resolveConsoleSchedule() + { + return tap(new FastSchedule($this->scheduleTimezone()), function ($schedule) { + $this->schedule($schedule->useCache($this->scheduleCache())); + }); + } + /** * Define the application's command schedule. * @@ -24,6 +37,7 @@ protected function schedule(Schedule $schedule) { $schedule->command('bpmn:timer') ->everyMinute() + ->name('bpmn:timer') ->onOneServer() ->withoutOverlapping(config('app.scheduler.bpmn_timer_overlap_minutes', 5)); diff --git a/ProcessMaker/Console/Scheduling/FastCommandEvent.php b/ProcessMaker/Console/Scheduling/FastCommandEvent.php new file mode 100644 index 0000000000..589e7fcfee --- /dev/null +++ b/ProcessMaker/Console/Scheduling/FastCommandEvent.php @@ -0,0 +1,60 @@ + + */ + protected array $artisanParameters; + + /** + * @param array $artisanParameters + * @param \DateTimeZone|string|null $timezone + */ + public function __construct( + EventMutex $mutex, + string $shellCommand, + string $artisanCommand, + array $artisanParameters = [], + $timezone = null + ) { + parent::__construct($mutex, $shellCommand, $timezone); + + $this->artisanCommand = $artisanCommand; + $this->artisanParameters = $artisanParameters; + } + + public function artisanCommandName(): string + { + return (new StringInput($this->artisanCommand))->getFirstArgument() ?? $this->artisanCommand; + } + + /** + * Run the command in-process, or shell out when runInBackground is set. + * + * @param \Illuminate\Contracts\Container\Container $container + * @return int + */ + protected function execute($container) + { + if ($this->runInBackground) { + return parent::execute($container); + } + + return Artisan::call($this->artisanCommand, $this->artisanParameters); + } +} diff --git a/ProcessMaker/Console/Scheduling/FastSchedule.php b/ProcessMaker/Console/Scheduling/FastSchedule.php new file mode 100644 index 0000000000..bc72b2b7d0 --- /dev/null +++ b/ProcessMaker/Console/Scheduling/FastSchedule.php @@ -0,0 +1,155 @@ +tenantEventStartIndex !== null) { + throw new LogicException( + 'Tenant schedule event registration is already active.' + ); + } + + $this->tenantEventStartIndex = count($this->events); + $this->mutexCache = []; + } + + /** + * Remove events registered for the current tenant. + * + * @return void + */ + public function clearTenantEvents(): void + { + if ($this->tenantEventStartIndex === null) { + return; + } + + $this->events = array_slice($this->events, 0, $this->tenantEventStartIndex); + $this->tenantEventStartIndex = null; + $this->mutexCache = []; + } + + /** + * Re-point the event and scheduling mutexes at the given cache factory. + * + * The mutexes hold a hard reference to the CacheManager resolved when the + * schedule was created. When the tenant cache prefix changes, that stale + * manager keeps writing mutex keys under the previous prefix, so + * onOneServer()/withoutOverlapping() locks leak across tenants. Refreshing + * the factory (and clearing the in-process mutex cache) ensures locks use + * the currently active, tenant-prefixed cache store. + * + * @param CacheFactory|null $cache + * @return void + */ + public function resetCache(?CacheFactory $cache = null): void + { + $cache ??= Container::getInstance()->make('cache'); + + if ($this->eventMutex instanceof CacheAware) { + $this->eventMutex->cache = $cache; + } + + if ($this->schedulingMutex instanceof CacheAware) { + $this->schedulingMutex->cache = $cache; + } + + $this->mutexCache = []; + } + + /** + * Add a new Artisan command event that runs in-process when possible. + * + * Falls back to a normal shell Event when the command string contains + * shell metacharacters (redirects, pipes, etc.). + * + * @param SymfonyCommand|string $command + * @param array $parameters + * @return \Illuminate\Console\Scheduling\Event + */ + public function command($command, array $parameters = []) + { + $commandDescription = null; + + if ($command instanceof SymfonyCommand) { + $command = Container::getInstance()->make(get_class($command)); + $commandDescription = $command->getDescription(); + $artisanCommand = $command->getName(); + } elseif (is_string($command) && class_exists($command)) { + $command = Container::getInstance()->make($command); + $commandDescription = $command->getDescription(); + $artisanCommand = $command->getName(); + } else { + $artisanCommand = $command; + } + + $artisanSignature = $artisanCommand; + if ($parameters !== []) { + $artisanSignature .= ' ' . $this->compileParameters($parameters); + } + + if ($this->containsShellMetacharacters($artisanSignature)) { + $event = parent::command($artisanCommand, $parameters); + + if ($commandDescription !== null) { + $event->description($commandDescription); + } + + return $event; + } + + $shellCommand = Application::formatCommandString($artisanCommand); + if ($parameters !== []) { + $shellCommand .= ' ' . $this->compileParameters($parameters); + } + + $this->events[] = $event = new FastCommandEvent( + $this->eventMutex, + $shellCommand, + $artisanCommand, + $parameters, + $this->timezone + ); + + $this->mergePendingAttributes($event); + + if ($commandDescription !== null) { + $event->description($commandDescription); + } + + if (empty($event->description)) { + $event->name($artisanSignature); + } + + return $event; + } + + /** + * Determine if the artisan signature requires a shell (redirects, pipes, etc.). + */ + protected function containsShellMetacharacters(string $command): bool + { + return (bool) preg_match('/(&&|&|\||>>|>)/', $command); + } +} diff --git a/ProcessMaker/Facades/Metrics.php b/ProcessMaker/Facades/Metrics.php index 878526722e..ba331ddf32 100644 --- a/ProcessMaker/Facades/Metrics.php +++ b/ProcessMaker/Facades/Metrics.php @@ -6,14 +6,14 @@ use ProcessMaker\Services\MetricsService; /** - * @method static \Prometheus\Counter counter(string $name, string $help = null, array $labels = []) - * @method static \Prometheus\Gauge gauge(string $name, string $help = null, array $labels = []) - * @method static \Prometheus\Histogram histogram(string $name, string $help = null, array $labels = [], array $buckets = [0.1, 1, 5, 10]) + * @method static \Prometheus\Counter counter(string $name, string|null $help = null, array $labels = []) + * @method static \Prometheus\Gauge gauge(string $name, string|null $help = null, array $labels = []) + * @method static \Prometheus\Histogram histogram(string $name, string|null $help = null, array $labels = [], array $buckets = []) * @method static void setGauge(string $name, float $value, array $labelValues = []) * @method static string renderMetrics() * @method static \Prometheus\CollectorRegistry getCollectionRegistry() - * @method static void counterInc(string $name, string $help = null, array $labels = []) - * @method static void histogramObserve(string $name, string $help = null, array $labels = [], array $buckets = [0.1, 1, 5, 10], float $executionTime) + * @method static void counterInc(string $name, string|null $help = null, array $labels = []) + * @method static void histogramObserve(string $name, string|null $help = null, array $labels = [], array $buckets = [], float $executionTime = 0) * @method static void clearMetrics() */ class Metrics extends Facade diff --git a/ProcessMaker/Listeners/ScheduledTaskMetricsSubscriber.php b/ProcessMaker/Listeners/ScheduledTaskMetricsSubscriber.php new file mode 100644 index 0000000000..b3fe9b0a3e --- /dev/null +++ b/ProcessMaker/Listeners/ScheduledTaskMetricsSubscriber.php @@ -0,0 +1,113 @@ + + */ + private WeakMap $startedAt; + + public function __construct() + { + $this->startedAt = new WeakMap(); + } + + public function handleStarting(ScheduledTaskStarting $event): void + { + $this->startedAt[$event->task] = microtime(true); + + if (Tenant::current() !== null) { + Metrics::clearResolvedInstance(MetricsService::class); + app()->forgetInstance(MetricsService::class); + } + } + + public function handleFinished(ScheduledTaskFinished $event): void + { + $job = $this->jobName($event->task); + + Metrics::gauge( + self::DURATION_SECONDS, + 'Duration of the last scheduled job run in seconds', + ['job'] + )->set($event->runtime, [$job]); + + unset($this->startedAt[$event->task]); + + if ($event->task->runInBackground || $event->task->exitCode !== 0) { + return; + } + + $this->recordResult($job, 'success'); + } + + public function handleFailed(ScheduledTaskFailed $event): void + { + $job = $this->jobName($event->task); + + if (isset($this->startedAt[$event->task])) { + Metrics::gauge( + self::DURATION_SECONDS, + 'Duration of the last scheduled job run in seconds', + ['job'] + )->set(microtime(true) - $this->startedAt[$event->task], [$job]); + + unset($this->startedAt[$event->task]); + } + + $this->recordResult($job, 'failure'); + } + + private function recordResult(string $job, string $status): void + { + $timestampMetric = $status === 'success' + ? self::LAST_SUCCESS_TIMESTAMP + : self::LAST_FAILURE_TIMESTAMP; + $resultDescription = $status === 'success' ? 'successful' : 'failed'; + + Metrics::gauge( + $timestampMetric, + "Unix timestamp of the last {$resultDescription} scheduled job run", + ['job'] + )->set(now()->timestamp, [$job]); + + Metrics::counter( + self::RUNS_TOTAL, + 'Total number of scheduled job runs', + ['job', 'status'] + )->inc([$job, $status]); + } + + private function jobName(Event $task): string + { + if ($task instanceof FastCommandEvent) { + return $task->artisanCommandName(); + } + + if (preg_match('/(?:^|\s)[\'"]?artisan[\'"]?\s+([^\s\'"]+)/', $task->command, $matches)) { + return $matches[1]; + } + + return $task->getSummaryForDisplay(); + } +} diff --git a/ProcessMaker/Multitenancy/PrefixCacheTask.php b/ProcessMaker/Multitenancy/PrefixCacheTask.php index 30aa0c127a..c695e0e7df 100644 --- a/ProcessMaker/Multitenancy/PrefixCacheTask.php +++ b/ProcessMaker/Multitenancy/PrefixCacheTask.php @@ -2,11 +2,16 @@ namespace ProcessMaker\Multitenancy; +use Illuminate\Console\Scheduling\Schedule; +use ProcessMaker\Console\Scheduling\FastSchedule; +use Prometheus\Storage\Redis as PrometheusRedis; use Spatie\Multitenancy\Contracts\IsTenant; use Spatie\Multitenancy\Tasks\PrefixCacheTask as SpatiePrefixCacheTask; class PrefixCacheTask extends SpatiePrefixCacheTask { + private const LANDLORD_PROMETHEUS_PREFIX = 'PROMETHEUS_'; + private $originalSettingsPrefix; public function makeCurrent(IsTenant $tenant): void @@ -19,6 +24,10 @@ public function makeCurrent(IsTenant $tenant): void config()->set('cache.stores.cache_settings.prefix', $tenantSettingsPrefix); $this->storeName = 'cache_settings'; $this->setCachePrefix($cachePrefix); + + PrometheusRedis::setPrefix($cachePrefix . self::LANDLORD_PROMETHEUS_PREFIX); + + $this->resetScheduleCache(); } public function forgetCurrent(): void @@ -28,5 +37,27 @@ public function forgetCurrent(): void config()->set('cache.stores.cache_settings.prefix', $this->originalSettingsPrefix); $this->storeName = 'cache_settings'; $this->setCachePrefix($this->originalPrefix); + + PrometheusRedis::setPrefix(self::LANDLORD_PROMETHEUS_PREFIX); + + $this->resetScheduleCache(); + } + + /** + * Point the scheduler's mutexes at the freshly prefixed cache store so + * onOneServer()/withoutOverlapping() locks are written under the current + * (tenant or landlord) cache prefix. + */ + private function resetScheduleCache(): void + { + if (!app()->resolved(Schedule::class)) { + return; + } + + $schedule = app(Schedule::class); + + if ($schedule instanceof FastSchedule) { + $schedule->resetCache(); + } } } diff --git a/ProcessMaker/Multitenancy/Services/TenantSchedulingService.php b/ProcessMaker/Multitenancy/Services/TenantSchedulingService.php index de7acb76f4..444bc75df5 100644 --- a/ProcessMaker/Multitenancy/Services/TenantSchedulingService.php +++ b/ProcessMaker/Multitenancy/Services/TenantSchedulingService.php @@ -3,9 +3,7 @@ namespace ProcessMaker\Multitenancy\Services; use Illuminate\Console\Scheduling\Schedule; -use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Request; use ProcessMaker\Multitenancy\Tenant; class TenantSchedulingService @@ -58,11 +56,12 @@ public function registerScheduledTasksForTenant(Schedule $schedule, Tenant $tena */ protected function createScheduledEvent(Schedule $schedule, Tenant $tenant, string $command) { - $fullCommand = "tenants:artisan {$command} --tenant={$tenant->id}"; + // Run the tenant command directly. schedule:multitenant-run already + // makeCurrent()'s the tenant before schedule:run, and FastSchedule + // executes commands in-process so tenant context is preserved. + $event = $schedule->command($command); - $event = $schedule->command($fullCommand); - - Log::info("Created scheduled event for tenant {$tenant->id}: {$fullCommand}"); + Log::info("Created scheduled event for tenant {$tenant->id}: {$command}"); return $event; } diff --git a/ProcessMaker/Multitenancy/SwitchTenant.php b/ProcessMaker/Multitenancy/SwitchTenant.php index e85c1421f7..338740e16a 100644 --- a/ProcessMaker/Multitenancy/SwitchTenant.php +++ b/ProcessMaker/Multitenancy/SwitchTenant.php @@ -3,11 +3,14 @@ namespace ProcessMaker\Multitenancy; use Illuminate\Broadcasting\BroadcastManager; +use Illuminate\Console\Scheduling\Schedule; use Illuminate\Contracts\Routing\UrlGenerator; use Illuminate\Support\Arr; use Illuminate\Support\Env; +use Illuminate\Support\Facades\Log; use Monolog\Handler\RotatingFileHandler; use ProcessMaker\Application; +use ProcessMaker\Console\Scheduling\FastSchedule; use ProcessMaker\Multitenancy\Broadcasting\TenantAwareBroadcastManager; use Spatie\Multitenancy\Concerns\UsesMultitenancyConfig; use Spatie\Multitenancy\Contracts\IsTenant; @@ -31,6 +34,15 @@ public function makeCurrent(IsTenant $tenant): void \Log::debug('SwitchTenant: ' . $tenant->id, ['domain' => request()->getHost()]); + // Keep track of tenant-specific schedule registrations so we can clear them when the tenant is forgotten. + if ($app->resolved(Schedule::class)) { + $schedule = $app->make(Schedule::class); + + if ($schedule instanceof FastSchedule) { + $schedule->beginTenantEventRegistration(); + } + } + // Save the landlord values for later use if (!self::$landlordValues) { self::$landlordValues = $app->make('config')->all(); @@ -55,6 +67,15 @@ public function makeCurrent(IsTenant $tenant): void public function forgetCurrent(): void { $app = app(); + + if ($app->resolved(Schedule::class)) { + $schedule = $app->make(Schedule::class); + + if ($schedule instanceof FastSchedule) { + $schedule->clearTenantEvents(); + } + } + $app->useStoragePath(base_path('storage')); $this->setConfig('logging.channels.daily.path', storage_path('logs/processmaker.log')); @@ -140,7 +161,7 @@ private function overrideConfigs(Application $app, IsTenant $tenant) // url() helper app(UrlGenerator::class)->useOrigin($tenant->config['app.url']); - // NOTE: Cache prefix and cache settings prefix are handled in PrefixCacheTask + // NOTE: Cache, cache settings, and Prometheus prefixes are handled in PrefixCacheTask if (!isset($tenant->config['app.docker_host_url'])) { // There is no specific override in the tenant's config so set it to the app url diff --git a/ProcessMaker/Providers/ProcessMakerServiceProvider.php b/ProcessMaker/Providers/ProcessMakerServiceProvider.php index 4d6040591f..06bb9e7b71 100644 --- a/ProcessMaker/Providers/ProcessMakerServiceProvider.php +++ b/ProcessMaker/Providers/ProcessMakerServiceProvider.php @@ -45,6 +45,7 @@ use ProcessMaker\ImportExport\SignalHelper; use ProcessMaker\Jobs\SmartInbox; use ProcessMaker\LicensedPackageManifest; +use ProcessMaker\Listeners\ScheduledTaskMetricsSubscriber; use ProcessMaker\Managers; use ProcessMaker\Managers\MenuManager; use ProcessMaker\Managers\ScreenCompiledManager; @@ -139,6 +140,8 @@ public function register(): void return new Managers\LoginManager(); }); + $this->app->singleton(ScheduledTaskMetricsSubscriber::class); + /* * Maps our Index Manager as a singleton. The Index Manager is used * to manage customizations to the search indexer. diff --git a/ProcessMaker/Services/MetricsService.php b/ProcessMaker/Services/MetricsService.php index 824f12b6cc..ba3b849f9c 100644 --- a/ProcessMaker/Services/MetricsService.php +++ b/ProcessMaker/Services/MetricsService.php @@ -7,14 +7,12 @@ use Laravel\Horizon\Contracts\MetricsRepository; use Laravel\Horizon\Contracts\WorkloadRepository; use ProcessMaker\Facades\Metrics; -use ProcessMaker\Multitenancy\Tenant; use Prometheus\CollectorRegistry; use Prometheus\Counter; use Prometheus\Gauge; use Prometheus\Histogram; use Prometheus\RenderTextFormat; use Prometheus\Storage\Redis as PrometheusRedis; -use Redis; use RuntimeException; class MetricsService @@ -46,10 +44,6 @@ public function __construct(private $adapter = null) if ($adapter === null) { $redis = app('redis')->client(); $adapter = PrometheusRedis::fromExistingConnection($redis); - if (app()->has(Tenant::BOOTSTRAPPED_TENANT)) { - $tenantInfo = app(Tenant::BOOTSTRAPPED_TENANT); - $adapter->setPrefix('tenant_' . $tenantInfo['id'] . ':PROMETHEUS_'); - } } $this->collectionRegistry = new CollectorRegistry($adapter); } catch (Exception $e) { @@ -211,9 +205,6 @@ public function addSystemLabels(array $labels) // Add system labels $labels['app_version'] = $this->getApplicationVersion(); $labels['app_name'] = config('app.name'); - if (config('app.prometheus_custom_label')) { - $labels['app_custom_label'] = config('app.prometheus_custom_label'); - } return $labels; } diff --git a/tests/Feature/Console/FastScheduleTest.php b/tests/Feature/Console/FastScheduleTest.php new file mode 100644 index 0000000000..4fe9db56fc --- /dev/null +++ b/tests/Feature/Console/FastScheduleTest.php @@ -0,0 +1,129 @@ +assertInstanceOf(FastSchedule::class, app(Schedule::class)); + } + + public function testCommandCreatesFastCommandEventAndAutoNames() + { + $schedule = new FastSchedule(); + $event = $schedule->command('foo:bar --queue'); + + $this->assertInstanceOf(FastCommandEvent::class, $event); + $this->assertSame('foo:bar --queue', $event->description); + } + + public function testCommandPreservesExplicitName() + { + $schedule = new FastSchedule(); + $event = $schedule->command('foo:bar')->name('custom-name'); + + $this->assertInstanceOf(FastCommandEvent::class, $event); + $this->assertSame('custom-name', $event->description); + } + + public function testCommandFallsBackToShellEventForRedirects() + { + $schedule = new FastSchedule(); + $event = $schedule->command('cache:metrics --format=json > storage/logs/metrics.json'); + + $this->assertInstanceOf(Event::class, $event); + $this->assertNotInstanceOf(FastCommandEvent::class, $event); + } + + public function testRunInBackgroundKeepsShellFallbackFlag() + { + $schedule = new FastSchedule(); + $event = $schedule->command('cases:retention:evaluate')->runInBackground(); + + $this->assertInstanceOf(FastCommandEvent::class, $event); + $this->assertTrue($event->runInBackground); + } + + public function testCommandRunsInProcessViaArtisanCall() + { + Artisan::registerCommand(app(FastScheduleProbeCommand::class)); + + app()->instance('fast-schedule-probe-token', 'in-process'); + FastScheduleProbeCommand::$seenToken = null; + + $schedule = new FastSchedule(); + $event = $schedule->command('fast-schedule:probe'); + + $this->assertInstanceOf(FastCommandEvent::class, $event); + + $event->run(app()); + + $this->assertSame( + 'in-process', + FastScheduleProbeCommand::$seenToken, + 'Command should see container state from the current process' + ); + } + + public function testClearTenantEventsPreservesBaseEvents() + { + $schedule = new FastSchedule(); + $baseEvent = $schedule->command('base:command'); + + $schedule->beginTenantEventRegistration(); + $schedule->command('tenant:command'); + $schedule->call(fn () => null); + + $this->assertCount(3, $schedule->events()); + + $schedule->clearTenantEvents(); + + $this->assertSame([$baseEvent], $schedule->events()); + } + + public function testTenantEventRegistrationCanBeRepeatedForDifferentTenants() + { + $schedule = new FastSchedule(); + $baseEvent = $schedule->command('base:command'); + + $schedule->beginTenantEventRegistration(); + $schedule->command('first-tenant:command'); + $schedule->clearTenantEvents(); + + $schedule->beginTenantEventRegistration(); + $secondTenantEvent = $schedule->command('second-tenant:command'); + + $this->assertSame([$baseEvent, $secondTenantEvent], $schedule->events()); + + $schedule->clearTenantEvents(); + + $this->assertSame([$baseEvent], $schedule->events()); + } +} + +class FastScheduleProbeCommand extends Command +{ + public static ?string $seenToken = null; + + protected $signature = 'fast-schedule:probe'; + + protected $description = 'Probe command for FastSchedule tests'; + + public function handle(): int + { + self::$seenToken = app()->bound('fast-schedule-probe-token') + ? app('fast-schedule-probe-token') + : null; + + return self::SUCCESS; + } +} diff --git a/tests/Feature/Console/ScheduleMultitenantRunTest.php b/tests/Feature/Console/ScheduleMultitenantRunTest.php new file mode 100644 index 0000000000..0a4971890a --- /dev/null +++ b/tests/Feature/Console/ScheduleMultitenantRunTest.php @@ -0,0 +1,118 @@ +assertTrue( + array_key_exists('schedule:tenants-run', Artisan::all()) + ); + } + + public function testCommandFallsBackToScheduleRunWhenMultitenancyDisabled() + { + config(['app.multitenancy' => false]); + + $this->artisan('schedule:tenants-run') + ->assertSuccessful(); + } + + public function testCommandSkipsWhenLockIsAlreadyHeld() + { + config(['app.multitenancy' => false]); + + $lock = Cache::lock(ScheduleMultitenantRun::LOCK_KEY, ScheduleMultitenantRun::LOCK_SECONDS); + $this->assertTrue($lock->get()); + + try { + Log::shouldReceive('error') + ->once() + ->withArgs(function (string $message) { + return str_contains($message, 'already running'); + }); + + $this->artisan('schedule:tenants-run') + ->expectsOutputToContain('already running') + ->assertFailed(); + } finally { + $lock->release(); + } + } + + public function testCommandReleasesLockAfterSuccessfulRun() + { + config(['app.multitenancy' => false]); + + $this->artisan('schedule:tenants-run') + ->assertSuccessful(); + + $lock = Cache::lock(ScheduleMultitenantRun::LOCK_KEY, ScheduleMultitenantRun::LOCK_SECONDS); + $this->assertTrue($lock->get(), 'Lock should be available after a successful run'); + $lock->release(); + } + + public function testCommandRecordsDurationHistogram() + { + config(['app.multitenancy' => false]); + + $this->artisan('schedule:tenants-run') + ->assertSuccessful(); + + $ns = config('app.prometheus_namespace', 'app'); + $histogram = Metrics::getCollectionRegistry()->getHistogram( + $ns, + ScheduleMultitenantRun::DURATION_METRIC + ); + + $this->assertInstanceOf(Histogram::class, $histogram); + $this->assertStringContainsString( + ScheduleMultitenantRun::DURATION_METRIC, + Metrics::renderMetrics() + ); + } + + public function testCommandDoesNotRecordDurationWhenLockIsAlreadyHeld() + { + config(['app.multitenancy' => false]); + + $lock = Cache::lock(ScheduleMultitenantRun::LOCK_KEY, ScheduleMultitenantRun::LOCK_SECONDS); + $this->assertTrue($lock->get()); + + try { + Log::shouldReceive('error') + ->once() + ->withArgs(function (string $message) { + return str_contains($message, 'already running'); + }); + + $this->artisan('schedule:tenants-run') + ->assertFailed(); + + $this->assertStringNotContainsString( + ScheduleMultitenantRun::DURATION_METRIC, + Metrics::renderMetrics() + ); + } finally { + $lock->release(); + } + } +} diff --git a/tests/Feature/Console/ScheduledTaskMetricsSubscriberTest.php b/tests/Feature/Console/ScheduledTaskMetricsSubscriberTest.php new file mode 100644 index 0000000000..fced5c465a --- /dev/null +++ b/tests/Feature/Console/ScheduledTaskMetricsSubscriberTest.php @@ -0,0 +1,116 @@ + false]); + + App::instance(MetricsService::class, new MetricsService(new InMemory())); + } + + public function testRecordsSuccessfulScheduledTaskMetrics(): void + { + $task = (new FastSchedule())->command('emails:send --queue'); + + Event::dispatch(new ScheduledTaskStarting($task)); + $task->exitCode = 0; + Event::dispatch(new ScheduledTaskFinished($task, 2.41)); + + $metrics = Metrics::renderMetrics(); + + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::LAST_SUCCESS_TIMESTAMP . '{job="emails:send"}', + $metrics + ); + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::DURATION_SECONDS . '{job="emails:send"} 2.41', + $metrics + ); + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::RUNS_TOTAL . '{job="emails:send",status="success"} 1', + $metrics + ); + } + + public function testRecordsFailedScheduledTaskMetricsAndDuration(): void + { + $task = (new FastSchedule())->command('emails:send'); + + Event::dispatch(new ScheduledTaskStarting($task)); + Event::dispatch(new ScheduledTaskFailed($task, new RuntimeException('Sending failed'))); + + $metrics = Metrics::renderMetrics(); + + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::LAST_FAILURE_TIMESTAMP . '{job="emails:send"}', + $metrics + ); + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::DURATION_SECONDS . '{job="emails:send"}', + $metrics + ); + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::RUNS_TOTAL . '{job="emails:send",status="failure"} 1', + $metrics + ); + $this->assertStringNotContainsString( + ScheduledTaskMetricsSubscriber::LAST_SUCCESS_TIMESTAMP . '{job="emails:send"}', + $metrics + ); + } + + public function testNonZeroExitIsOnlyRecordedAsFailure(): void + { + $task = (new FastSchedule())->command('emails:send'); + + Event::dispatch(new ScheduledTaskStarting($task)); + $task->exitCode = 1; + Event::dispatch(new ScheduledTaskFinished($task, 1.25)); + Event::dispatch(new ScheduledTaskFailed($task, new RuntimeException('Exit code 1'))); + + $metrics = Metrics::renderMetrics(); + + $this->assertStringContainsString( + ScheduledTaskMetricsSubscriber::RUNS_TOTAL . '{job="emails:send",status="failure"} 1', + $metrics + ); + $this->assertStringNotContainsString( + ScheduledTaskMetricsSubscriber::RUNS_TOTAL . '{job="emails:send",status="success"}', + $metrics + ); + } + + public function testTenantMetricsServiceIsResolvedAfterTenantBecomesCurrent(): void + { + config(['app.multitenancy' => true]); + app()->instance(config('multitenancy.current_tenant_container_key'), new Tenant(['id' => 123])); + + app(ScheduledTaskMetricsSubscriber::class)->handleStarting( + new ScheduledTaskStarting((new FastSchedule())->command('emails:send')) + ); + + $instances = (new ReflectionProperty(Container::class, 'instances'))->getValue(app()); + $this->assertArrayNotHasKey(MetricsService::class, $instances); + + app()->forgetInstance(config('multitenancy.current_tenant_container_key')); + } +} diff --git a/tests/unit/MetricsServiceTest.php b/tests/unit/MetricsServiceTest.php index b73f9ad831..636d99db5e 100644 --- a/tests/unit/MetricsServiceTest.php +++ b/tests/unit/MetricsServiceTest.php @@ -14,7 +14,6 @@ use ReflectionClass; use Tests\TestCase; - class MetricsServiceTest extends TestCase { /** @@ -44,7 +43,7 @@ public function testCounterRegistrationAndIncrement(): void $counter = $this->metricsService->counter('test_counter', 'Test Counter', ['label1']); // Assert the counter is registered - $this->assertInstanceOf(\Prometheus\Counter::class, $counter); + $this->assertInstanceOf(Counter::class, $counter); // Increment the counter and assert the value $counter->inc(['value1']); @@ -83,7 +82,7 @@ public function testHistogramRegistrationAndObserve(): void ); // Assert the histogram is registered - $this->assertInstanceOf(\Prometheus\Histogram::class, $histogram); + $this->assertInstanceOf(Histogram::class, $histogram); // Observe a value and assert it is recorded $histogram->observe(0.5, ['value1']); @@ -112,7 +111,7 @@ public function testDefaultNamespace(): void $counter = $this->metricsService->counter('namespace_test'); // Assert default namespace is applied - $this->assertInstanceOf(\Prometheus\Counter::class, $counter); + $this->assertInstanceOf(Counter::class, $counter); $counter->inc(); $samples = $this->metricsService->renderMetrics(); @@ -131,6 +130,7 @@ public function testSetGaugeValue(): void $this->assertStringContainsString('test_set_gauge', $samples); $this->assertStringContainsString('5', $samples); } + /** * Test that counterInc calls Metrics::counter() and then inc() with the correct labels. */ @@ -138,7 +138,6 @@ public function testCounterInc() { // Set configuration values used by addSystemLabels() Config::set('app.name', 'TestApp'); - Config::set('app.prometheus_custom_label', 'customValue'); // Create an instance of MetricsService. $service = new MetricsService(); @@ -152,9 +151,8 @@ public function testCounterInc() $systemLabels = $service->addSystemLabels([]); // Merge the initial labels with system labels. $expectedLabels = array_merge($initialLabels, [ - 'app_version' => $systemLabels['app_version'], - 'app_name' => 'TestApp', - 'app_custom_label' => 'customValue', + 'app_version' => $systemLabels['app_version'], + 'app_name' => 'TestApp', ]); $expectedLabelKeys = array_keys($expectedLabels); @@ -182,7 +180,6 @@ public function testHistogramObserve() { // Set configuration values used by addSystemLabels() Config::set('app.name', 'TestApp'); - Config::set('app.prometheus_custom_label', 'customValue'); $service = new MetricsService(); @@ -195,9 +192,8 @@ public function testHistogramObserve() // Determine what system labels will be added. $systemLabels = $service->addSystemLabels([]); $expectedLabels = array_merge($initialLabels, [ - 'app_version' => $systemLabels['app_version'], - 'app_name' => 'TestApp', - 'app_custom_label' => 'customValue', + 'app_version' => $systemLabels['app_version'], + 'app_name' => 'TestApp', ]); $expectedLabelKeys = array_keys($expectedLabels); @@ -225,7 +221,6 @@ public function testAddSystemLabels() { // Set configuration values used by addSystemLabels() Config::set('app.name', 'TestApp'); - Config::set('app.prometheus_custom_label', 'customValue'); $service = new MetricsService(); @@ -238,9 +233,8 @@ public function testAddSystemLabels() // Assert that the system labels were added. $this->assertArrayHasKey('app_version', $result); $this->assertArrayHasKey('app_name', $result); - $this->assertArrayHasKey('app_custom_label', $result); + $this->assertArrayNotHasKey('app_custom_label', $result); $this->assertEquals('TestApp', $result['app_name']); - $this->assertEquals('customValue', $result['app_custom_label']); $this->assertNotEmpty($result['app_version']); // Assuming composer.json defines a version. }