mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 21:18:11 -04:00
perf(sentinel): cache traffic per minute, skip unsafe keys, batch owner lookups
- Align the traffic window to the minute so the 60 s cache is used. - Skip only an unsafe key instead of hiding the whole server. - Look up unknown container owners in one query per push.
This commit is contained in:
@@ -235,6 +235,14 @@ class PushServerUpdateJob implements ShouldBeEncrypted, ShouldQueue, Silenced
|
||||
$this->allServiceApplicationIds = $this->serviceApplicationsById->keys();
|
||||
$this->allServiceDatabaseIds = $this->serviceDatabasesById->keys();
|
||||
|
||||
// Owners of application containers outside this server's applications, in one query.
|
||||
$foreignApplicationIdsByUuid = containerApplicationIdsByUuid(
|
||||
$this->applications,
|
||||
$this->containers
|
||||
->map(fn ($container) => collect(data_get($container, 'labels')))
|
||||
->filter(fn (Collection $labels) => $labels->has('coolify.managed') && isContainerOfType($labels, 'application'))
|
||||
);
|
||||
|
||||
foreach ($this->containers as $container) {
|
||||
$containerStatus = data_get($container, 'state', 'exited');
|
||||
$rawHealthStatus = data_get($container, 'health_status');
|
||||
@@ -259,7 +267,7 @@ class PushServerUpdateJob implements ShouldBeEncrypted, ShouldQueue, Silenced
|
||||
}
|
||||
// Containers are matched by owner UUID; numeric ids differ between instances.
|
||||
if (isContainerOfType($labels, 'application')) {
|
||||
$applicationId = resolveContainerApplicationId($this->applications, $labels);
|
||||
$applicationId = resolveContainerApplicationId($this->applications, $labels, $foreignApplicationIdsByUuid);
|
||||
if ($applicationId === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -560,14 +560,7 @@ class Analytics extends Component
|
||||
*/
|
||||
private function window(): array
|
||||
{
|
||||
$to = now();
|
||||
$from = match ($this->range) {
|
||||
'7d' => now()->subDays(7),
|
||||
'30d' => now()->subDays(30),
|
||||
default => now()->subDay(),
|
||||
};
|
||||
|
||||
return [$from->toIso8601ZuluString(), $to->toIso8601ZuluString()];
|
||||
return SentinelTrafficClient::rangeWindow($this->range);
|
||||
}
|
||||
|
||||
public function placeholder(array $params = []): View
|
||||
|
||||
@@ -141,14 +141,7 @@ class TrafficAnalytics extends Component
|
||||
*/
|
||||
private function window(): array
|
||||
{
|
||||
$to = now();
|
||||
$from = match ($this->range) {
|
||||
'7d' => now()->subDays(7),
|
||||
'30d' => now()->subDays(30),
|
||||
default => now()->subDay(),
|
||||
};
|
||||
|
||||
return [$from->toIso8601ZuluString(), $to->toIso8601ZuluString()];
|
||||
return SentinelTrafficClient::rangeWindow($this->range);
|
||||
}
|
||||
|
||||
public function placeholder(): View
|
||||
|
||||
@@ -9,6 +9,7 @@ use App\Data\Traffic\TrafficSeriesBucketData;
|
||||
use App\Models\Server;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
class SentinelTrafficClient
|
||||
{
|
||||
@@ -57,15 +58,20 @@ class SentinelTrafficClient
|
||||
/**
|
||||
* Convert a UI range key (24h/7d/30d) into ISO-8601 Zulu from/to bounds.
|
||||
*
|
||||
* The window ends at the next full minute instead of the current second: the bounds are
|
||||
* part of the 60s response cache key, so every load within that minute reuses the cached
|
||||
* response. The window still covers "now" (Sentinel rolls traffic up per minute, so the
|
||||
* end in the future adds nothing) and spans exactly the requested range.
|
||||
*
|
||||
* @return array{0: string, 1: string}
|
||||
*/
|
||||
public static function rangeWindow(string $range): array
|
||||
{
|
||||
$to = now();
|
||||
$to = now()->utc()->ceilMinute();
|
||||
$from = match ($range) {
|
||||
'7d' => now()->subDays(7),
|
||||
'30d' => now()->subDays(30),
|
||||
default => now()->subDay(),
|
||||
'7d' => $to->copy()->subDays(7),
|
||||
'30d' => $to->copy()->subDays(30),
|
||||
default => $to->copy()->subDay(),
|
||||
};
|
||||
|
||||
return [$from->toIso8601ZuluString(), $to->toIso8601ZuluString()];
|
||||
@@ -352,10 +358,7 @@ class SentinelTrafficClient
|
||||
return [];
|
||||
}
|
||||
|
||||
return array_values(array_filter(
|
||||
array_map(fn ($app) => is_array($app) ? ($app['uuid'] ?? null) : null, $bundle['apps'] ?? []),
|
||||
fn ($uuid) => is_string($uuid) && $uuid !== ''
|
||||
));
|
||||
return $this->safeReportedKeys(array_map(fn ($app) => is_array($app) ? ($app['uuid'] ?? null) : null, $bundle['apps'] ?? []));
|
||||
}
|
||||
|
||||
// Fallback for older Sentinel without /traffic/dashboard: batch the individual endpoints.
|
||||
@@ -374,10 +377,29 @@ class SentinelTrafficClient
|
||||
return [];
|
||||
}
|
||||
|
||||
return array_values(array_filter(
|
||||
$this->apps(),
|
||||
fn ($uuid) => is_string($uuid) && $uuid !== ''
|
||||
));
|
||||
return $this->safeReportedKeys($this->apps());
|
||||
}
|
||||
|
||||
/**
|
||||
* The usable keys of a Sentinel-reported key list. A key that would be unsafe in a request
|
||||
* url is dropped (and logged) on its own, so it doesn't fail the whole server's analytics.
|
||||
*
|
||||
* @param array<int, mixed> $keys
|
||||
* @return array<int, string>
|
||||
*/
|
||||
private function safeReportedKeys(array $keys): array
|
||||
{
|
||||
$keys = array_values(array_filter($keys, fn ($key) => is_string($key) && $key !== ''));
|
||||
$safe = array_values(array_filter($keys, fn (string $key) => $this->isSafeKey($key)));
|
||||
|
||||
if (count($safe) !== count($keys)) {
|
||||
Log::warning('Traffic analytics skipped unsafe app keys reported by Sentinel', [
|
||||
'server' => $this->server->uuid,
|
||||
'skipped' => count($keys) - count($safe),
|
||||
]);
|
||||
}
|
||||
|
||||
return $safe;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -509,7 +531,7 @@ class SentinelTrafficClient
|
||||
|
||||
foreach ($bundle['apps'] ?? [] as $app) {
|
||||
$uuid = is_array($app) ? ($app['uuid'] ?? null) : null;
|
||||
if (is_string($uuid) && $uuid !== '' && isset($app['overview'])) {
|
||||
if (is_string($uuid) && $this->isSafeKey($uuid) && isset($app['overview'])) {
|
||||
$put($this->overviewUrl($uuid, $from, $to), $app['overview']);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -118,8 +118,11 @@ function resolveContainerOwner(Collection $resources, Collection|array|string $l
|
||||
/**
|
||||
* Id of the application that owns a container, also for applications not in $applications
|
||||
* (for example the parent of a preview). Same order as resolveContainerOwner().
|
||||
*
|
||||
* @param array<string, int>|null $applicationIdsByUuid Owners outside $applications, preloaded with
|
||||
* containerApplicationIdsByUuid(); null queries per call.
|
||||
*/
|
||||
function resolveContainerApplicationId(Collection $applications, Collection|array|string $labels): ?int
|
||||
function resolveContainerApplicationId(Collection $applications, Collection|array|string $labels, ?array $applicationIdsByUuid = null): ?int
|
||||
{
|
||||
$labels = is_string($labels) ? format_docker_labels_to_json($labels) : collect($labels);
|
||||
|
||||
@@ -129,11 +132,47 @@ function resolveContainerApplicationId(Collection $applications, Collection|arra
|
||||
}
|
||||
|
||||
$uuid = containerOwnerUuid($labels, 'application');
|
||||
$id = $uuid ? Application::withTrashed()->where('uuid', $uuid)->value('id') : null;
|
||||
if (! $uuid) {
|
||||
return null;
|
||||
}
|
||||
if ($applicationIdsByUuid !== null) {
|
||||
return $applicationIdsByUuid[$uuid] ?? null;
|
||||
}
|
||||
|
||||
$id = Application::withTrashed()->where('uuid', $uuid)->value('id');
|
||||
|
||||
return $id !== null ? (int) $id : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ids of the applications (including trashed ones) that own the given containers but are not in
|
||||
* $applications, keyed by owner UUID, in one query. Pass the result to resolveContainerApplicationId()
|
||||
* so a batch of containers does not query once per container.
|
||||
*
|
||||
* @param iterable<Collection|array|string> $containerLabels
|
||||
* @return array<string, int>
|
||||
*/
|
||||
function containerApplicationIdsByUuid(Collection $applications, iterable $containerLabels): array
|
||||
{
|
||||
$knownUuids = $applications->pluck('uuid')->flip();
|
||||
$uuids = [];
|
||||
foreach ($containerLabels as $labels) {
|
||||
$uuid = containerOwnerUuid($labels, 'application');
|
||||
if ($uuid && ! $knownUuids->has($uuid)) {
|
||||
$uuids[$uuid] = true;
|
||||
}
|
||||
}
|
||||
if ($uuids === []) {
|
||||
return [];
|
||||
}
|
||||
|
||||
return Application::withTrashed()
|
||||
->whereIn('uuid', array_keys($uuids))
|
||||
->pluck('id', 'uuid')
|
||||
->map(fn ($id) => (int) $id)
|
||||
->all();
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a container belongs to a resource of the given type (by UUID or older numeric id label).
|
||||
*
|
||||
|
||||
@@ -16,6 +16,7 @@ use App\Models\StandaloneDocker;
|
||||
use App\Models\StandalonePostgresql;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
@@ -198,6 +199,53 @@ describe('status updates after a server transfer', function () {
|
||||
->and($this->application->fresh()->status)->toStartWith('running');
|
||||
});
|
||||
|
||||
test('Sentinel push looks up foreign application owners with one query for all containers', function () {
|
||||
$deleted = Application::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'destination_id' => $this->destination->id,
|
||||
'destination_type' => $this->destination->getMorphClass(),
|
||||
]);
|
||||
$deleted->delete();
|
||||
|
||||
$containers = collect(range(1, 5))->map(fn (int $i) => [
|
||||
'name' => "foreign-{$i}",
|
||||
'state' => 'running',
|
||||
'health_status' => 'healthy',
|
||||
'labels' => [
|
||||
'coolify.managed' => 'true',
|
||||
'coolify.applicationUuid' => "foreign-app-{$i}",
|
||||
'coolify.pullRequestId' => '0',
|
||||
'com.docker.compose.service' => "foreign-app-{$i}",
|
||||
],
|
||||
])->push([
|
||||
'name' => $deleted->uuid.'-pr-3',
|
||||
'state' => 'running',
|
||||
'health_status' => 'healthy',
|
||||
'labels' => [
|
||||
'coolify.managed' => 'true',
|
||||
'coolify.applicationUuid' => $deleted->uuid,
|
||||
'coolify.pullRequestId' => '3',
|
||||
'com.docker.compose.service' => $deleted->uuid.'-pr-3',
|
||||
],
|
||||
])->all();
|
||||
|
||||
$job = new PushServerUpdateJob($this->server, ['containers' => $containers]);
|
||||
|
||||
DB::enableQueryLog();
|
||||
$job->handle();
|
||||
$ownerLookups = collect(DB::getQueryLog())
|
||||
->filter(fn (array $query) => str_contains($query['query'], 'from "applications"') && preg_match('/"uuid" (=|in) /', $query['query']) === 1)
|
||||
->count();
|
||||
DB::disableQueryLog();
|
||||
|
||||
$previewLabels = end($containers)['labels'];
|
||||
expect($ownerLookups)->toBe(1)
|
||||
// A trashed owner still resolves, with and without the preloaded ids.
|
||||
->and(resolveContainerApplicationId(collect(), $previewLabels))->toBe($deleted->id)
|
||||
->and(resolveContainerApplicationId(collect(), $previewLabels, containerApplicationIdsByUuid(collect(), [$previewLabels])))->toBe($deleted->id)
|
||||
->and(containerApplicationIdsByUuid(collect([$this->application]), [['coolify.applicationUuid' => $this->application->uuid]]))->toBe([]);
|
||||
});
|
||||
|
||||
test('Sentinel push matches a new application container by its UUID label', function () {
|
||||
$job = new PushServerUpdateJob($this->server, ['containers' => [[
|
||||
'name' => $this->application->uuid,
|
||||
|
||||
@@ -12,6 +12,7 @@ use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use App\Services\SentinelTrafficClient;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Livewire\Livewire;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
@@ -425,3 +426,59 @@ it('shows the not-enabled empty state when no server has traffic analytics on',
|
||||
->assertSee('Traffic analytics is not enabled')
|
||||
->assertDontSee('Unique visitors');
|
||||
});
|
||||
|
||||
it('keeps a server visible when Sentinel reports one unsafe app key', function () {
|
||||
$server = bootEnabledGlobalServer();
|
||||
|
||||
$project = Project::factory()->create(['team_id' => $this->team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
$destination = StandaloneDocker::where('server_id', $server->id)->first()
|
||||
?? StandaloneDocker::factory()->create(['server_id' => $server->id, 'network' => 'coolify-test']);
|
||||
$application = Application::factory()->create([
|
||||
'name' => 'Safe Leaderboard App',
|
||||
'environment_id' => $environment->id,
|
||||
'destination_id' => $destination->id,
|
||||
'destination_type' => StandaloneDocker::class,
|
||||
]);
|
||||
|
||||
$fake = new FakeGlobalAnalyticsTrafficClient($server);
|
||||
$fake->responses = fakeGlobalAnalyticsResponses([$application->uuid, 'bad key$(reboot)']);
|
||||
app()->bind(SentinelTrafficClient::class, fn () => $fake);
|
||||
|
||||
loadLazy(Livewire::test(Analytics::class))
|
||||
->assertOk()
|
||||
->assertSee('1,000')
|
||||
->assertSee('Safe Leaderboard App')
|
||||
->assertDontSee('bad key$(reboot)');
|
||||
});
|
||||
|
||||
it('queries Sentinel with the same window for loads within one cache interval', function () {
|
||||
$server = bootEnabledGlobalServer();
|
||||
|
||||
$fake = new class($server) extends FakeGlobalAnalyticsTrafficClient
|
||||
{
|
||||
public array $urls = [];
|
||||
|
||||
protected function raw(string $url): string
|
||||
{
|
||||
$this->urls[] = $url;
|
||||
|
||||
return parent::raw($url);
|
||||
}
|
||||
};
|
||||
$fake->responses = fakeGlobalAnalyticsResponses();
|
||||
app()->bind(SentinelTrafficClient::class, fn () => $fake);
|
||||
|
||||
$this->travelTo(Carbon::parse('2026-09-30 10:00:05', 'UTC'));
|
||||
$component = loadLazy(Livewire::test(Analytics::class));
|
||||
$firstUrls = $fake->urls;
|
||||
$fake->urls = [];
|
||||
|
||||
$this->travelTo(Carbon::parse('2026-09-30 10:00:50', 'UTC'));
|
||||
$component->call('loadData');
|
||||
|
||||
// The second load asks for exactly the windows of the first one (the 60s cache keys).
|
||||
expect($firstUrls)->not->toBeEmpty()
|
||||
->and($fake->urls)->not->toBeEmpty()
|
||||
->and(array_values(array_diff($fake->urls, $firstUrls)))->toBe([]);
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@ use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use App\Services\SentinelTrafficClient;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
|
||||
@@ -48,3 +49,29 @@ it('surfaces the real remote error instead of a TypeError when the fetch fails',
|
||||
'failed docker exec' => [1, '', 'Error response from daemon: No such container: coolify-sentinel', 'No such container: coolify-sentinel'],
|
||||
'literal null body' => [0, 'null', '', 'Traffic analytics returned an invalid response.'],
|
||||
]);
|
||||
|
||||
it('keeps the range window stable within the 60s cache interval so repeated loads hit the cache', function () {
|
||||
Cache::flush();
|
||||
$team = Team::factory()->create();
|
||||
$server = Server::factory()->create(['team_id' => $team->id]);
|
||||
$client = new CountingTrafficClient($server);
|
||||
|
||||
$this->travelTo(Carbon::parse('2026-09-30 10:00:05', 'UTC'));
|
||||
[$from, $to] = SentinelTrafficClient::rangeWindow('24h');
|
||||
$client->overview('app', $from, $to);
|
||||
|
||||
$this->travelTo(Carbon::parse('2026-09-30 10:00:47', 'UTC'));
|
||||
[$laterFrom, $laterTo] = SentinelTrafficClient::rangeWindow('24h');
|
||||
$client->overview('app', $laterFrom, $laterTo);
|
||||
|
||||
expect($client->calls)->toBe(1)
|
||||
->and([$laterFrom, $laterTo])->toBe([$from, $to])
|
||||
// The window still reaches "now" and spans exactly the requested range.
|
||||
->and($to)->toBe('2026-09-30T10:01:00Z')
|
||||
->and($from)->toBe('2026-09-29T10:01:00Z');
|
||||
|
||||
$this->travelTo(Carbon::parse('2026-09-30 10:01:03', 'UTC'));
|
||||
[, $nextTo] = SentinelTrafficClient::rangeWindow('7d');
|
||||
expect($nextTo)->toBe('2026-09-30T10:02:00Z')
|
||||
->and(SentinelTrafficClient::rangeWindow('7d')[0])->toBe('2026-09-23T10:02:00Z');
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@ use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use App\Services\SentinelTrafficClient;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
@@ -302,3 +303,53 @@ it('double-quotes the url in the remote curl command so & is not a shell backgro
|
||||
// container shell backgrounds curl at the `&` and only `from=...` reaches Sentinel.
|
||||
expect($command)->toContain('"'.$url.'"');
|
||||
});
|
||||
|
||||
it('skips an unsafe app key in the dashboard bundle and keeps the other apps', function () {
|
||||
$server = Server::factory()->create(['team_id' => $this->team->id]);
|
||||
Log::spy();
|
||||
|
||||
$bundle = json_encode([
|
||||
'overview' => ['requests' => 7],
|
||||
'paths' => [],
|
||||
'breakdowns' => [],
|
||||
'series' => [],
|
||||
'apps' => [
|
||||
['uuid' => 'app-a', 'overview' => ['requests' => 4]],
|
||||
['uuid' => 'bad key;rm -rf /', 'overview' => ['requests' => 2]],
|
||||
['uuid' => 'app-b', 'overview' => ['requests' => 1]],
|
||||
],
|
||||
]);
|
||||
|
||||
$client = new class($server, $bundle) extends SentinelTrafficClient
|
||||
{
|
||||
public function __construct($server, private string $bundle)
|
||||
{
|
||||
parent::__construct($server);
|
||||
}
|
||||
|
||||
protected function remoteFetch(string $url): string
|
||||
{
|
||||
if (str_contains($url, '/traffic/dashboard')) {
|
||||
return $this->bundle;
|
||||
}
|
||||
throw new RuntimeException("unexpected individual fetch: {$url}");
|
||||
}
|
||||
};
|
||||
|
||||
$apps = $client->prefetchServerWide(null, 'F', 'T', [], '24h');
|
||||
|
||||
expect($apps)->toBe(['app-a', 'app-b'])
|
||||
->and($client->overview('app-b', 'F', 'T')->requests)->toBe(1);
|
||||
Log::shouldHaveReceived('warning')->withArgs(fn ($message) => str_contains($message, 'unsafe'))->once();
|
||||
});
|
||||
|
||||
it('skips an unsafe app key reported by an older Sentinel without the dashboard route', function () {
|
||||
$server = Server::factory()->create(['team_id' => $this->team->id]);
|
||||
Log::spy();
|
||||
|
||||
$client = new FakeTrafficClient($server);
|
||||
$client->response = json_encode(['app-a', 'bad"key', 'app-b']);
|
||||
|
||||
expect($client->prefetchServerWide(null, 'F', 'T', [], '24h'))->toBe(['app-a', 'app-b']);
|
||||
Log::shouldHaveReceived('warning')->withArgs(fn ($message) => str_contains($message, 'unsafe'))->once();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user