From 04b8cbe0d72466123f7aa76d5aa0b835614429f5 Mon Sep 17 00:00:00 2001 From: Jason McCreary Date: Wed, 10 Aug 2022 12:29:45 -0400 Subject: [PATCH 1/5] Boyscouting --- tests/Feature/MonitorWaitTimesTest.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/Feature/MonitorWaitTimesTest.php b/tests/Feature/MonitorWaitTimesTest.php index e35c7541..bf256c25 100644 --- a/tests/Feature/MonitorWaitTimesTest.php +++ b/tests/Feature/MonitorWaitTimesTest.php @@ -20,10 +20,6 @@ public function test_queues_with_long_waits_are_found() { Event::fake(); - $redis = Mockery::mock(RedisFactory::class); - $redis->shouldReceive('setnx')->with('monitor:time-to-clear', 1)->andReturn(1); - $redis->shouldReceive('expire')->with('monitor:time-to-clear', 60); - $calc = Mockery::mock(WaitTimeCalculator::class); $calc->shouldReceive('calculate')->andReturn([ 'redis:test-queue' => 10, @@ -31,10 +27,10 @@ public function test_queues_with_long_waits_are_found() ]); $this->app->instance(WaitTimeCalculator::class, $calc); - $listener = new MonitorWaitTimes(resolve(MetricsRepository::class), $redis); - $listener->lastMonitoredAt = CarbonImmutable::now()->subDays(1); + $listener = new MonitorWaitTimes(resolve(MetricsRepository::class)); + $listener->lastMonitoredAt = CarbonImmutable::now()->subDay(); - $listener->handle(new SupervisorLooped(Mockery::mock(Supervisor::class))); + $listener->handle(null); Event::assertDispatched(LongWaitDetected::class, function ($event) { return $event->connection == 'redis' && $event->queue == 'test-queue-2'; From 973659116ef119b00ff200ee7e718dfcd5fe3dc1 Mon Sep 17 00:00:00 2001 From: Jason McCreary Date: Wed, 10 Aug 2022 12:29:56 -0400 Subject: [PATCH 2/5] WIP: TDD --- tests/Feature/MonitorWaitTimesTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Feature/MonitorWaitTimesTest.php b/tests/Feature/MonitorWaitTimesTest.php index bf256c25..47d40e3c 100644 --- a/tests/Feature/MonitorWaitTimesTest.php +++ b/tests/Feature/MonitorWaitTimesTest.php @@ -36,4 +36,24 @@ public function test_queues_with_long_waits_are_found() return $event->connection == 'redis' && $event->queue == 'test-queue-2'; }); } + + public function test_queue_ignores_long_waits() + { + config(['horizon.waits' => ['redis:ignore-queue' => 0]]); + + Event::fake(); + + $calc = Mockery::mock(WaitTimeCalculator::class); + $calc->expects('calculate')->andReturn([ + 'redis:ignore-queue' => 10, + ]); + $this->app->instance(WaitTimeCalculator::class, $calc); + + $listener = new MonitorWaitTimes(resolve(MetricsRepository::class)); + $listener->lastMonitoredAt = CarbonImmutable::now()->subDays(1); + + $listener->handle(null); + + Event::assertNotDispatched(LongWaitDetected::class); + } } From 2796e20abad3b4faed86a6a26ef944b711cd8b57 Mon Sep 17 00:00:00 2001 From: Jason McCreary Date: Wed, 10 Aug 2022 12:45:58 -0400 Subject: [PATCH 3/5] Code style --- tests/Feature/MonitorWaitTimesTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Feature/MonitorWaitTimesTest.php b/tests/Feature/MonitorWaitTimesTest.php index 47d40e3c..98bccfeb 100644 --- a/tests/Feature/MonitorWaitTimesTest.php +++ b/tests/Feature/MonitorWaitTimesTest.php @@ -3,13 +3,10 @@ namespace Laravel\Horizon\Tests\Feature; use Carbon\CarbonImmutable; -use Illuminate\Contracts\Redis\Factory as RedisFactory; use Illuminate\Support\Facades\Event; use Laravel\Horizon\Contracts\MetricsRepository; use Laravel\Horizon\Events\LongWaitDetected; -use Laravel\Horizon\Events\SupervisorLooped; use Laravel\Horizon\Listeners\MonitorWaitTimes; -use Laravel\Horizon\Supervisor; use Laravel\Horizon\Tests\IntegrationTest; use Laravel\Horizon\WaitTimeCalculator; use Mockery; From 4f0bfa4674930a1771c9288bd23d8e9c4c03d4f1 Mon Sep 17 00:00:00 2001 From: Jason McCreary Date: Wed, 10 Aug 2022 12:50:20 -0400 Subject: [PATCH 4/5] Remove unused event --- src/Listeners/MonitorWaitTimes.php | 3 +-- tests/Feature/MonitorWaitTimesTest.php | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Listeners/MonitorWaitTimes.php b/src/Listeners/MonitorWaitTimes.php index 5653e2c7..ea4b6376 100644 --- a/src/Listeners/MonitorWaitTimes.php +++ b/src/Listeners/MonitorWaitTimes.php @@ -5,7 +5,6 @@ use Carbon\CarbonImmutable; use Laravel\Horizon\Contracts\MetricsRepository; use Laravel\Horizon\Events\LongWaitDetected; -use Laravel\Horizon\Events\SupervisorLooped; use Laravel\Horizon\WaitTimeCalculator; class MonitorWaitTimes @@ -41,7 +40,7 @@ public function __construct(MetricsRepository $metrics) * @param \Laravel\Horizon\Events\SupervisorLooped $event * @return void */ - public function handle(SupervisorLooped $event) + public function handle() { if (! $this->dueToMonitor()) { return; diff --git a/tests/Feature/MonitorWaitTimesTest.php b/tests/Feature/MonitorWaitTimesTest.php index 98bccfeb..1d561bce 100644 --- a/tests/Feature/MonitorWaitTimesTest.php +++ b/tests/Feature/MonitorWaitTimesTest.php @@ -27,7 +27,7 @@ public function test_queues_with_long_waits_are_found() $listener = new MonitorWaitTimes(resolve(MetricsRepository::class)); $listener->lastMonitoredAt = CarbonImmutable::now()->subDay(); - $listener->handle(null); + $listener->handle(); Event::assertDispatched(LongWaitDetected::class, function ($event) { return $event->connection == 'redis' && $event->queue == 'test-queue-2'; @@ -49,7 +49,7 @@ public function test_queue_ignores_long_waits() $listener = new MonitorWaitTimes(resolve(MetricsRepository::class)); $listener->lastMonitoredAt = CarbonImmutable::now()->subDays(1); - $listener->handle(null); + $listener->handle(); Event::assertNotDispatched(LongWaitDetected::class); } From fab4a05027337a47f1ed414deda9435ba765ac67 Mon Sep 17 00:00:00 2001 From: Jason McCreary Date: Wed, 10 Aug 2022 12:54:46 -0400 Subject: [PATCH 5/5] Ignore queues with wait time of 0 --- src/Listeners/MonitorWaitTimes.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Listeners/MonitorWaitTimes.php b/src/Listeners/MonitorWaitTimes.php index ea4b6376..7ac98d53 100644 --- a/src/Listeners/MonitorWaitTimes.php +++ b/src/Listeners/MonitorWaitTimes.php @@ -52,7 +52,8 @@ public function handle() $results = app(WaitTimeCalculator::class)->calculate(); $long = collect($results)->filter(function ($wait, $queue) { - return $wait > (config("horizon.waits.{$queue}") ?? 60); + return config("horizon.waits.{$queue}") !== 0 + && $wait > (config("horizon.waits.{$queue}") ?? 60); }); // Once we have determined which queues have long wait times we will raise the