From 7e077d9501dddb01ff5327ac946a43dc3eeb10ad Mon Sep 17 00:00:00 2001 From: Aditya Tripathi Date: Thu, 15 Oct 2026 19:00:58 +0530 Subject: [PATCH] feat(metrics): sortable tables and one-click enable on the Metrics page (#12162) Co-authored-by: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> --- .ai/lessons.md | 2 + app/Livewire/Fleet/Metrics.php | 68 ++++++- resources/js/traffic-table.js | 12 +- .../views/livewire/fleet/metrics.blade.php | 167 ++++++++++-------- .../livewire/traffic/_sort-header.blade.php | 3 +- .../Fleet/MetricsChartPayloadTest.php | 3 + .../Livewire/Fleet/MetricsDataTest.php | 67 +++++++ .../v4/Browser/FleetMetricsTableSortTest.php | 141 +++++++++++++++ 8 files changed, 386 insertions(+), 77 deletions(-) create mode 100644 tests/v4/Browser/FleetMetricsTableSortTest.php diff --git a/.ai/lessons.md b/.ai/lessons.md index 4740622e14..cd64c0a27e 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -16,6 +16,8 @@ - A new worktree has no `vendor/` or `public/build/`. Run `composer install --ignore-platform-req=ext-zip` on the host and copy `public/build` from the main checkout before page-rendering tests; otherwise they fail with a Vite manifest error. - Never run `php artisan test` inside a dev container (`./scripts/dev exec` or `docker exec coolify-dev-*`). The container sets `DB_CONNECTION=pgsql` as a real environment variable, which wins over `phpunit.xml`, so `RefreshDatabase` runs `migrate:fresh` on the dev database and wipes it. Run tests on the host only. - Do not symlink `vendor` into another worktree. Composer resolves the base path from the real `vendor` path, so the tests load the app classes of the other checkout. Run `composer install --ignore-platform-req=ext-zip` in the worktree. +- Do not call `sleep()` in a browser test. The app server runs in the test process, so `sleep()` stops it; use `$page->wait()`. +- In tests, turning on `is_metrics_enabled` (and other Sentinel settings) with `save()`/`update()` runs the settings hook, which starts Sentinel over a real SSH connection on the sync queue. Use `saveQuietly()` or `Queue::fake()`. - Call `visit()` directly in each `tests/v4/Browser` test body; Pest does not mark a test that only uses helper-wrapped `visit()` as a browser test, so it fails with `sendText() on null`. - SQLite returns `0`/`1` for boolean columns without a `boolean` cast, while Postgres returns real booleans. A strict `=== false` check on such a column never matches in tests, so tests can pass on behavior that Postgres blocks. Add the cast or compare loosely. diff --git a/app/Livewire/Fleet/Metrics.php b/app/Livewire/Fleet/Metrics.php index bd9580a8cc..bcf6917499 100644 --- a/app/Livewire/Fleet/Metrics.php +++ b/app/Livewire/Fleet/Metrics.php @@ -29,6 +29,9 @@ class Metrics extends Component /** @var array uuid => name */ public array $serverOptions = []; + /** @var array Names of the servers where the user can turn metrics on; filled only when no server has metrics. */ + public array $enableableServers = []; + #[Url(as: 'range')] public string $range = '24h'; @@ -87,9 +90,13 @@ class Metrics extends Component private function refreshServers(): void { abort_unless(currentTeam() !== null, 403); - $this->servers = Server::ownedByCurrentTeam()->get() + $teamServers = Server::ownedByCurrentTeam()->get(); + $this->servers = $teamServers ->filter(fn (Server $server) => $server->isMetricsEnabled()) ->values(); + $this->enableableServers = $this->servers->isEmpty() + ? $this->serversWithoutMetrics($teamServers)->pluck('name')->all() + : []; foreach ($this->servers as $server) { $this->authorize('view', $server); @@ -104,6 +111,65 @@ class Metrics extends Component } } + /** + * Servers that can report metrics, have them off, are reachable, and that the user can change. + * An unreachable server cannot start Sentinel, so it stays out. + * + * @param Collection $teamServers + * @return Collection + */ + private function serversWithoutMetrics(Collection $teamServers): Collection + { + return $teamServers + ->filter(fn (Server $server) => ! $server->isMetricsEnabled() + && $server->isSentinelEnabled() + && auth()->user()->can('update', $server) + && $server->isFunctional()) + ->values(); + } + + public function enableMetrics(): void + { + try { + abort_unless(currentTeam() !== null, 403); + $servers = $this->serversWithoutMetrics(Server::ownedByCurrentTeam()->get()); + if ($servers->isEmpty()) { + return; + } + + // One failed server must not stop the others, so each server has its own try. + $failed = []; + foreach ($servers as $server) { + try { + $server->settings->is_metrics_enabled = true; + // Saved quietly: Sentinel is started below, so the settings hook must not start it a second time. + $server->settings->saveQuietly(); + auditLog('ui.server.sentinel.updated', [ + 'team_id' => $server->team_id, + 'server_uuid' => $server->uuid, + 'server_name' => $server->name, + 'changed_fields' => ['is_metrics_enabled'], + ]); + $server->restartSentinel(); + } catch (\Throwable $e) { + report($e); + $failed[] = $server->name; + } + } + + $this->loadData(); + $enabled = $servers->count() - count($failed); + if ($enabled > 0) { + $this->dispatch('success', 'Metrics enabled on '.$enabled.' '.str('server')->plural($enabled).'. Sentinel is starting, data shows in about one minute.'); + } + if ($failed !== []) { + $this->dispatch('error', 'Could not enable metrics on '.implode(', ', $failed).'. Check the server settings and try again.'); + } + } catch (\Throwable $e) { + handleError($e, $this); + } + } + public function updatedServerUuid(): void { $this->loadData(); diff --git a/resources/js/traffic-table.js b/resources/js/traffic-table.js index f8355b1708..0012b65d52 100644 --- a/resources/js/traffic-table.js +++ b/resources/js/traffic-table.js @@ -75,14 +75,16 @@ function trafficTable({ per = 10, key = 'requests', dir = 'desc' } = {}) { values = {}; } - return { el, index, pinned: 'pinned' in el.dataset ? 1 : 0, value: values[this.key] ?? '' }; + const value = values[this.key] ?? null; + + return { el, index, pinned: 'pinned' in el.dataset ? 1 : 0, missing: value === null ? 1 : 0, value }; }); - // Pinned rows (the "Other" remainder) stay last in each sort. - rows.sort((a, b) => a.pinned - b.pinned || compareValues(a.value, b.value) * sign || a.index - b.index); + // Pinned rows (the "Other" remainder) stay last in each sort, then rows without a value. + rows.sort((a, b) => a.pinned - b.pinned || a.missing - b.missing || compareValues(a.value, b.value) * sign || a.index - b.index); rows.forEach((row, rank) => ranks.set(row.el, rank)); - // All rows share one parent. Move them only when the order changed, so the - // mutation that the move causes finds them sorted and stops. + // All rows share one parent. Move them only when the order changed: the mutation + // that the move causes then finds them sorted, and a focused link is not blurred. if (rows.some((row, rank) => row.index !== rank)) { rows.forEach((row) => row.el.parentElement.appendChild(row.el)); observer?.takeRecords(); diff --git a/resources/views/livewire/fleet/metrics.blade.php b/resources/views/livewire/fleet/metrics.blade.php index 4ff5b14ddb..79103f6de2 100644 --- a/resources/views/livewire/fleet/metrics.blade.php +++ b/resources/views/livewire/fleet/metrics.blade.php @@ -81,12 +81,21 @@ $serverListboxOptions = array_merge( @if ($servers->isEmpty()) - - View servers - +
+ @if (! empty($enableableServers)) + + {{ count($enableableServers) === 1 + ? 'Enable metrics on '.\Illuminate\Support\Str::limit($enableableServers[0], 24) + : 'Enable metrics on '.count($enableableServers).' servers' }} + + @endif + + View servers + +
@else @@ -226,41 +235,55 @@ $serverListboxOptions = array_merge( {{-- Servers table --}} - -
-
- Server - CPU - Memory - Disk - Load - Network - Containers + +
+
+ @include('livewire.traffic._sort-header', ['column' => 'name', 'label' => 'Server', 'text' => true, 'class' => 'justify-self-start']) + @foreach (['cpu' => 'CPU', 'memory' => 'Memory', 'disk' => 'Disk', 'load' => 'Load', 'network' => 'Network'] as $column => $label) + @include('livewire.traffic._sort-header', ['column' => $column, 'label' => $label, 'text' => true, 'dir' => 'desc', 'class' => 'justify-self-start']) + @endforeach + @include('livewire.traffic._sort-header', ['column' => 'containers', 'label' => 'Containers', 'class' => 'justify-self-end'])
- @foreach (collect($serverRows)->sortByDesc(fn ($r) => $r['cpu'] ?? -1) as $row) -
-
- {{ $row['name'] }} - @unless ($row['online']) - Offline - @endunless +
+ @foreach (collect($serverRows)->sortByDesc(fn ($r) => $r['cpu'] ?? -1) as $row) + @php + $memPercent = ($row['memTotal'] ?? 0) ? round($row['memUsed'] / $row['memTotal'] * 100, 1) : null; + $sortValues = [ + 'name' => $row['name'], + 'cpu' => $row['cpu'], + 'memory' => $memPercent, + 'disk' => $row['diskPercent'], + 'load' => $row['load1'], + 'network' => $row['netRx'] === null ? null : $row['netRx'] + $row['netTx'], + 'containers' => $row['containers'], + ]; + @endphp + {{-- Offline servers have no values, so they stay last in each sort. --}} +
+
+ {{ $row['name'] }} + @unless ($row['online']) + Offline + @endunless +
+
@include('livewire.fleet._usage-cell', ['percent' => $row['cpu'], 'kind' => 'cpu'])
+
@include('livewire.fleet._usage-cell', ['percent' => $memPercent, 'kind' => 'memory'])
+
@include('livewire.fleet._usage-cell', ['percent' => $row['diskPercent'], 'kind' => 'disk'])
+
{{ $row['load1'] === null ? '-' : number_format($row['load1'], 2) }}
+
{{ $row['netRx'] === null ? '-' : formatBytes($row['netRx'] + $row['netTx']).'/s' }}
+
{{ $row['containers'] ?? '-' }}
-
@include('livewire.fleet._usage-cell', ['percent' => $row['cpu'], 'kind' => 'cpu'])
-
@include('livewire.fleet._usage-cell', ['percent' => ($row['memTotal'] ?? 0) ? round($row['memUsed'] / $row['memTotal'] * 100, 1) : null, 'kind' => 'memory'])
-
@include('livewire.fleet._usage-cell', ['percent' => $row['diskPercent'], 'kind' => 'disk'])
-
{{ $row['load1'] === null ? '-' : number_format($row['load1'], 2) }}
-
{{ $row['netRx'] === null ? '-' : formatBytes($row['netRx'] + $row['netTx']).'/s' }}
-
{{ $row['containers'] ?? '-' }}
-
- @endforeach + @endforeach +
{{-- Hottest containers --}} -
@@ -294,47 +317,51 @@ $serverListboxOptions = array_merge( $maxMetric = max(1, collect($topContainers)->map($metricOf)->max()); @endphp
-
+ wire:target="containerMetric" x-data="trafficTable({ per: Number.MAX_SAFE_INTEGER, key: 'value' })"> +
# - Container - Server - {{ ['cpu' => 'CPU', 'memory' => 'Memory', 'disk' => 'Disk', 'network' => 'Network'][$containerMetric] }} + @include('livewire.traffic._sort-header', ['column' => 'name', 'label' => 'Container', 'text' => true, 'class' => 'justify-self-start']) + @include('livewire.traffic._sort-header', ['column' => 'server', 'label' => 'Server', 'text' => true, 'class' => 'justify-self-start']) + @include('livewire.traffic._sort-header', ['column' => 'value', 'label' => ['cpu' => 'CPU', 'memory' => 'Memory', 'disk' => 'Disk', 'network' => 'Network'][$containerMetric], 'class' => 'justify-self-end'])
- @foreach ($topContainers as $c) - @php - $meta = $this->containerMeta($c['id']); - $pct = min(100, round($metricOf($c) / $maxMetric * 100, 1)); - $display = match ($containerMetric) { - 'memory' => formatBytes($c['memUsed'] ?? 0), - 'disk' => formatBytes($c['diskBytes'] ?? 0), - 'network' => formatBytes($c['net'] ?? 0).'/s', - default => round($c['cpu'] ?? 0).'%', - }; - @endphp -
-
{{ $loop->iteration }}
-
- @if ($meta['link']) - {{ $meta['name'] }} - @else - {{ $meta['name'] }} - @endif - @if ($meta['image']) - {{ $meta['image'] }} - @endif -
-
{{ $c['server'] }}
-
-
@endif diff --git a/resources/views/livewire/traffic/_sort-header.blade.php b/resources/views/livewire/traffic/_sort-header.blade.php index e5b53499e0..82602ee4cf 100644 --- a/resources/views/livewire/traffic/_sort-header.blade.php +++ b/resources/views/livewire/traffic/_sort-header.blade.php @@ -8,10 +8,11 @@ hides on small screens passes `hidden :inline-flex` @param ?string $title tooltip @param ?bool $text true for a text column: left-aligned, first sort ascending + @param ?string $dir first sort direction, when it differs from the default of the alignment --}} @php $text = $text ?? false; - $firstDir = $text ? 'asc' : 'desc'; + $firstDir = $dir ?? ($text ? 'asc' : 'desc'); $class = $class ?? ''; // `hidden` and `inline-flex` on one element conflict, so add the display only when the caller sets none. $display = str_contains($class, 'hidden') ? '' : 'inline-flex'; diff --git a/tests/Feature/Livewire/Fleet/MetricsChartPayloadTest.php b/tests/Feature/Livewire/Fleet/MetricsChartPayloadTest.php index 5091fc9b2c..130b4c5ebe 100644 --- a/tests/Feature/Livewire/Fleet/MetricsChartPayloadTest.php +++ b/tests/Feature/Livewire/Fleet/MetricsChartPayloadTest.php @@ -9,6 +9,7 @@ use App\Models\User; use App\Services\SentinelMetricsClient; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Concurrency; +use Illuminate\Support\Facades\Queue; use Livewire\Livewire; uses(RefreshDatabase::class); @@ -63,6 +64,8 @@ class DenseTestableMetrics extends Metrics } beforeEach(function () { + // The settings hook starts Sentinel when metrics turn on. A sync queue would run it and make a real SSH call. + Queue::fake(); Concurrency::setDefaultInstance('sync'); InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); $user = User::factory()->create(); diff --git a/tests/Feature/Livewire/Fleet/MetricsDataTest.php b/tests/Feature/Livewire/Fleet/MetricsDataTest.php index d70df7e9b0..90693900af 100644 --- a/tests/Feature/Livewire/Fleet/MetricsDataTest.php +++ b/tests/Feature/Livewire/Fleet/MetricsDataTest.php @@ -1,5 +1,6 @@ toBeTrue(); expect($token)->not->toBe('invalid token'); }); + +/** A server that Coolify can reach, with metrics off. */ +function reachableServerWithoutMetrics(int $teamId, PrivateKey $privateKey): Server +{ + $server = Server::factory()->create(['team_id' => $teamId, 'private_key_id' => $privateKey->id]); + $server->settings->forceFill(['is_reachable' => true, 'is_usable' => true])->saveQuietly(); + + return $server; +} + +it('enables metrics on each eligible server of the team from the empty state', function () { + $first = reachableServerWithoutMetrics($this->team->id, $this->privateKey); + $second = reachableServerWithoutMetrics($this->team->id, $this->privateKey); + $build = reachableServerWithoutMetrics($this->team->id, $this->privateKey); + $build->settings->update(['server_role' => 'build', 'is_build_server' => true]); + $unreachable = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $this->privateKey->id]); + $foreign = reachableServerWithoutMetrics(Team::factory()->create()->id, $this->privateKey); + + $component = Livewire::test(TestableMetrics::class) + ->assertSee('Enable metrics on 2 servers') + ->call('enableMetrics'); + + expect($first->settings->fresh()->is_metrics_enabled)->toBeTruthy(); + expect($second->settings->fresh()->is_metrics_enabled)->toBeTruthy(); + expect($build->settings->fresh()->is_metrics_enabled)->toBeFalsy(); + expect($unreachable->settings->fresh()->is_metrics_enabled)->toBeFalsy(); + expect($foreign->settings->fresh()->is_metrics_enabled)->toBeFalsy(); + StartSentinel::assertPushed(2); + + expect(array_column($component->get('serverRows'), 'uuid'))->toEqualCanonicalizing([$first->uuid, $second->uuid]); + $component->assertDontSee('No server metrics yet'); +}); + +it('keeps enabling metrics on the other servers when one server fails', function () { + $broken = reachableServerWithoutMetrics($this->team->id, $this->privateKey); + $healthy = reachableServerWithoutMetrics($this->team->id, $this->privateKey); + DB::statement("CREATE TRIGGER fail_settings_update BEFORE UPDATE ON server_settings WHEN NEW.server_id = {$broken->id} BEGIN SELECT RAISE(ABORT, 'write failed'); END"); + + Livewire::test(TestableMetrics::class) + ->call('enableMetrics') + ->assertDispatched('success', fn (string $event, array $params) => str_contains($params[0], 'Metrics enabled on 1 server.')) + ->assertDispatched('error', fn (string $event, array $params) => str_contains($params[0], $broken->name)); + + expect($broken->settings->fresh()->is_metrics_enabled)->toBeFalsy(); + expect($healthy->settings->fresh()->is_metrics_enabled)->toBeTruthy(); + StartSentinel::assertPushed(1); +}); + +it('does not let a team member enable metrics', function () { + $server = reachableServerWithoutMetrics($this->team->id, $this->privateKey); + $member = User::factory()->create(); + $member->teams()->attach($this->team, ['role' => 'member']); + $this->actingAs($member); + + Livewire::test(TestableMetrics::class) + ->assertSee('No server metrics yet') + ->assertDontSee('Enable metrics on '.$server->name) + ->call('enableMetrics'); + + expect($server->settings->fresh()->is_metrics_enabled)->toBeFalsy(); + StartSentinel::assertNotPushed(); +}); diff --git a/tests/v4/Browser/FleetMetricsTableSortTest.php b/tests/v4/Browser/FleetMetricsTableSortTest.php new file mode 100644 index 0000000000..3d3c30bf3c --- /dev/null +++ b/tests/v4/Browser/FleetMetricsTableSortTest.php @@ -0,0 +1,141 @@ + ['cpu' => 10.0, 'memUsed' => 80], + // bravo runs an older Sentinel that does not send the load. + 'bravo' => ['cpu' => 90.0, 'memUsed' => 20, 'load1' => null], + ][$this->server->name] ?? null; + + // charlie does not answer. + if ($usage === null) { + return null; + } + + return array_merge([ + 'online' => true, 'memTotal' => 100, + 'diskUsed' => 10, 'diskTotal' => 100, 'diskPercent' => 10.0, + 'load1' => 0.4, 'netRx' => 5.0, 'netTx' => 1.0, + ], $usage); + } + + public function containersCurrent(): array + { + return match ($this->server->name) { + 'alpha' => [['id' => 'zeta', 'cpu' => 70.0, 'memUsed' => 10, 'memPercent' => 1.0, 'diskBytes' => 1, 'net' => 0.0]], + 'bravo' => [['id' => 'beta', 'cpu' => 5.0, 'memUsed' => 20, 'memPercent' => 2.0, 'diskBytes' => 2, 'net' => 0.0]], + default => [], + }; + } + + public function history(string $metric, string $range): array + { + return []; + } + + public function networkHistory(string $range): array + { + return ['rx' => [], 'tx' => []]; + } +} + +beforeEach(function () { + $this->stack = seedBrowserResourceStack(['serverName' => 'alpha']); + Concurrency::setDefaultInstance('sync'); + // The dashboard chart reads metrics over SSH for each server with metrics on. + Process::fake(); + + $servers = [$this->stack['server']]; + foreach (['bravo', 'charlie'] as $name) { + $servers[] = Server::factory()->create([ + 'name' => $name, 'team_id' => 0, 'private_key_id' => $this->stack['privateKey']->id, + ]); + } + foreach ($servers as $server) { + // Saved quietly, so the settings hook does not start Sentinel. + $server->settings->forceFill(['is_metrics_enabled' => true, 'is_reachable' => true, 'is_usable' => true])->saveQuietly(); + } + + app()->bind(SentinelMetricsClient::class, fn ($app, array $parameters) => new FakeBrowserFleetMetricsClient($parameters['server'])); +}); + +/** The visible row names of one metrics table, in DOM order (the order focus and screen readers follow). */ +function visibleFleetMetricsRowNames(mixed $page, string $section): array +{ + return $page->script(<< getComputedStyle(row).display !== 'none') + .map((row) => JSON.parse(row.dataset.row).name) + JS); +} + +it('sorts the servers and containers tables when a column header is selected', function () { + loginAndSkipBoarding(); + + $page = visit('/metrics'); + + // Default: CPU, largest first. The offline server is last. + $page->assertSee('charlie'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['bravo', 'alpha', 'charlie']); + + $page->click('#metrics-servers-section button[title="Sort by server"]'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['alpha', 'bravo', 'charlie']); + + $page->click('#metrics-servers-section button[title="Sort by memory"]'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['alpha', 'bravo', 'charlie']); + + // A second click reverses the direction. The offline server stays last. + $page->click('#metrics-servers-section button[title="Sort by memory"]'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['bravo', 'alpha', 'charlie']); + + // A server without a value stays after the servers with one, in both directions. + $page->click('#metrics-servers-section button[title="Sort by load"]'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['alpha', 'bravo', 'charlie']); + $page->click('#metrics-servers-section button[title="Sort by load"]'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['alpha', 'bravo', 'charlie']); + + $page->click('#metrics-servers-section button[title="Sort by memory"]'); + $page->click('#metrics-servers-section button[title="Sort by memory"]'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['bravo', 'alpha', 'charlie']); + + // A data refresh keeps the sort. + $page->click('button:visible:has-text("7 days")') + ->assertQueryStringHas('range', '7d'); + expect(visibleFleetMetricsRowNames($page, 'metrics-servers-section'))->toBe(['bravo', 'alpha', 'charlie']); + + expect(visibleFleetMetricsRowNames($page, 'metrics-containers-section'))->toBe(['zeta', 'beta']); + // The notification banner at the bottom of the page covers this header, so a pointer click cannot reach it. + $page->script('document.querySelector(\'#metrics-containers-section button[title="Sort by container"]\').click()'); + expect(visibleFleetMetricsRowNames($page, 'metrics-containers-section'))->toBe(['beta', 'zeta']); + + $page->assertNoJavaScriptErrors() + ->screenshot(filename: 'fleet-metrics-tables-sorted'); +}); + +it('enables metrics from the empty state', function () { + Queue::fake(); + Server::query()->each(fn (Server $server) => $server->settings->forceFill(['is_metrics_enabled' => false])->saveQuietly()); + + loginAndSkipBoarding(); + + $page = visit('/metrics'); + + $page->assertSee('No server metrics yet') + ->click('button:visible:has-text("Enable metrics on 3 servers")') + ->assertSee('Hottest containers') + ->assertDontSee('No server metrics yet') + ->assertNoJavaScriptErrors() + ->screenshot(filename: 'fleet-metrics-enabled-from-empty-state'); +});