diff --git a/app/Services/SentinelMetricsClient.php b/app/Services/SentinelMetricsClient.php index b9ad06a164..184c4bc4e7 100644 --- a/app/Services/SentinelMetricsClient.php +++ b/app/Services/SentinelMetricsClient.php @@ -35,6 +35,17 @@ class SentinelMetricsClient }; } + /** + * Sentinel averages rows into `bucket_ms` windows. Older Sentinels ignore the param + * and return raw rows, which seriesRows() buckets the same way. + * + * @return array{from: string, bucket_ms: int} + */ + private static function historyQuery(string $range): array + { + return ['from' => self::rangeFrom($range), 'bucket_ms' => self::bucketMs($range)]; + } + /** * @return array|null */ @@ -112,7 +123,7 @@ class SentinelMetricsClient default => ['/cpu/history', 'percent'], }; - return $this->seriesRows($this->url($path, ['from' => self::rangeFrom($range)]), $field, $range, $metric === 'disk'); + return $this->seriesRows($this->url($path, self::historyQuery($range)), $field, $range, $metric === 'disk'); } /** @@ -120,7 +131,7 @@ class SentinelMetricsClient */ public function networkHistory(string $range): array { - $url = $this->url('/network/history', ['from' => self::rangeFrom($range)]); + $url = $this->url('/network/history', self::historyQuery($range)); return [ 'rx' => $this->seriesRows($url, 'rxBytesPerSec', $range), @@ -149,8 +160,9 @@ class SentinelMetricsClient $rows = array_filter($rows, fn ($row) => ($row['mount'] ?? null) === $mount); } - // Average raw samples into epoch-aligned buckets so every server shares the same - // timestamps and the fleet aggregator can sum them point by point. + // Average samples into epoch-aligned buckets so every server shares the same + // timestamps and the fleet aggregator can sum them point by point. A no-op for + // rows Sentinel already bucketed. $bucketMs = self::bucketMs($range); $sums = []; $counts = []; diff --git a/app/Traits/HasMetrics.php b/app/Traits/HasMetrics.php index d39675276d..a9fe454e3a 100644 --- a/app/Traits/HasMetrics.php +++ b/app/Traits/HasMetrics.php @@ -35,7 +35,7 @@ trait HasMetrics // only has letters, digits and `._-+/=` (ServerSetting::isValidSentinelToken), so it // cannot end the heredoc. Responses are separated by a 0x1E record separator. $script = implode("\nprintf '\\036'\n", array_map( - fn (string $type) => 'curl -s -H @- "'.$this->getMetricsEndpoint($type, $from)."\" <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH", + fn (string $type) => 'curl -s -H @- "'.$this->getMetricsEndpoint($type, $from, $mins)."\" <<'COOLIFY_SENTINEL_AUTH'\nAuthorization: Bearer {$token}\nCOOLIFY_SENTINEL_AUTH", $types )); @@ -147,11 +147,11 @@ trait HasMetrics unset($this->prefetchedMetricResponses[$key]); } else { $from = now()->subMinutes($mins)->toIso8601ZuluString(); - $endpoint = $this->getMetricsEndpoint($type, $from); + $endpoint = $this->getMetricsEndpoint($type, $from, $mins); $token = $this->metricsToken($server); $response = instant_remote_process( - ["docker exec coolify-sentinel sh -c 'curl -H \"Authorization: Bearer {$token}\" {$endpoint}'"], + ["docker exec coolify-sentinel sh -c 'curl -H \"Authorization: Bearer {$token}\" \"{$endpoint}\"'"], $server, false ); @@ -248,13 +248,23 @@ trait HasMetrics return $this->isServerMetrics() ? $this : $this->destination->server; } - private function getMetricsEndpoint(string $type, string $from): string + /** + * Ranges over an hour ask Sentinel to average rows into whole-minute windows, at most + * 1000 per range, so it sends far fewer rows. Older Sentinels ignore `bucket_ms`; + * mapRows() still downsamples their raw rows. + */ + private function getMetricsEndpoint(string $type, string $from, int $mins): string { $base = 'http://localhost:8888/api'; - if ($this->isServerMetrics()) { - return "{$base}/{$type}/history?from={$from}"; + $query = "from={$from}"; + if ($mins > 60) { + $query .= '&bucket_ms='.((int) ceil($mins / 1000) * 60_000); } - return "{$base}/container/{$this->uuid}/{$type}/history?from={$from}"; + if ($this->isServerMetrics()) { + return "{$base}/{$type}/history?{$query}"; + } + + return "{$base}/container/{$this->uuid}/{$type}/history?{$query}"; } } diff --git a/tests/Feature/ServerMetricsPrefetchTest.php b/tests/Feature/ServerMetricsPrefetchTest.php index 85a51e1ac4..578e39c726 100644 --- a/tests/Feature/ServerMetricsPrefetchTest.php +++ b/tests/Feature/ServerMetricsPrefetchTest.php @@ -68,6 +68,21 @@ it('falls back to one fetch per metric when the batch fails', function () { ->and($commands)->toHaveCount(2); }); +it('quotes the bucketed history URL in a single fetch for a non-root user', function () { + $commands = []; + Process::fake(function (PendingProcess $process) use (&$commands) { + $commands[] = $process->command; + + return Process::result('[{"time":60000,"percent":5}]'); + }); + $this->server->forceFill(['user' => 'deploy'])->saveQuietly(); + + expect($this->server->getCpuMetrics(720))->toBe([[60000, 5.0]]) + ->and($commands)->toHaveCount(1) + ->and($commands[0])->toContain('sudo bash -c') + ->and($commands[0])->toMatch('/"http:\/\/localhost:8888\/api\/cpu\/history\?from=[^"]+&bucket_ms=60000"/'); +}); + it('runs the batched metrics script with sudo for a non-root user', function () { $commands = []; Process::fake(function (PendingProcess $process) use (&$commands) { diff --git a/tests/Unit/Metrics/HasMetricsMappingTest.php b/tests/Unit/Metrics/HasMetricsMappingTest.php index d8b4b805bd..e1f2425343 100644 --- a/tests/Unit/Metrics/HasMetricsMappingTest.php +++ b/tests/Unit/Metrics/HasMetricsMappingTest.php @@ -13,10 +13,17 @@ class MetricsMappingDouble /** @var array type => rows */ public array $rowsByType = []; + public string $uuid = 'app-uuid'; + protected function fetchMetricRows(string $type, int $mins): ?array { return $this->rowsByType[$type] ?? null; } + + public function endpoint(string $type, int $mins): string + { + return $this->getMetricsEndpoint($type, '2026-01-01T00:00:00Z', $mins); + } } it('keeps only the root mount for disk metrics', function () { @@ -73,3 +80,12 @@ it('supports older disk rows without a mount field', function () { expect($double->getDiskMetrics(5))->toBe([[1000, 40.0]]); }); + +it('asks sentinel to bucket ranges over an hour into at most 1000 whole-minute rows', function () { + $double = new MetricsMappingDouble; + + expect($double->endpoint('cpu', 60))->not->toContain('bucket_ms') + ->and($double->endpoint('cpu', 720))->toBe('http://localhost:8888/api/container/app-uuid/cpu/history?from=2026-01-01T00:00:00Z&bucket_ms=60000') + ->and($double->endpoint('cpu', 10080))->toEndWith('&bucket_ms=660000') + ->and($double->endpoint('cpu', 43200))->toEndWith('&bucket_ms=2640000'); +}); diff --git a/tests/Unit/Services/SentinelMetricsClientTest.php b/tests/Unit/Services/SentinelMetricsClientTest.php index 4bf1793d4c..c8c42286b0 100644 --- a/tests/Unit/Services/SentinelMetricsClientTest.php +++ b/tests/Unit/Services/SentinelMetricsClientTest.php @@ -12,8 +12,12 @@ class FakeMetricsClient extends SentinelMetricsClient /** @var array */ public array $failing = []; + /** @var array */ + public array $urls = []; + protected function raw(string $url): string { + $this->urls[] = $url; foreach ($this->failing as $needle) { if (str_contains($url, $needle)) { throw new RuntimeException('boom'); @@ -247,3 +251,16 @@ it('prefers the root mount over the host mount of the Coolify data directory', f expect($client->history('disk', '24h'))->toBe([[1699999800000, 60.0]]); }); + +it('asks sentinel to bucket history rows by range', function (string $range, int $bucketMs) { + $client = fakeMetricsClient(); + + $client->history('memory', $range); + $client->networkHistory($range); + + expect($client->urls)->each->toContain("bucket_ms={$bucketMs}"); +})->with([ + ['24h', 300_000], + ['7d', 1_800_000], + ['30d', 7_200_000], +]);