From 0efbcecea0c4b2bbab729637d94958cd4a371862 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 23 Sep 2026 20:09:25 +0100 Subject: [PATCH 1/7] feat(metrics): show network storage usage The metrics API reports network storage volumes (used by "storage" mounts) under the "storage" mountpoint, but metrics:all and metrics:disk-usage only read "/mnt" and "/tmp". On projects using storage mounts, the only disk the customer sizes could not be monitored from the CLI, and resources:get "Disk" could not be compared to any metrics column. - metrics:all: add storage_used, storage_limit, storage_percent and storage_inodes_* columns. - metrics:disk-usage: add storage_used, storage_limit, storage_percent and storage_i* columns, and a --storage report option. - storage_percent is a default column only when a service reports storage, so output is unchanged for other projects. - Existing disk_* columns are not renamed. - The api.metrics_storage config key (default true) turns this off. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_test.go | 147 ++++++++++++++++++ legacy/config-defaults.yaml | 3 + .../src/Command/Metrics/AllMetricsCommand.php | 61 +++++++- .../src/Command/Metrics/DiskUsageCommand.php | 71 ++++++++- .../Command/Metrics/MetricsCommandBase.php | 35 +++++ 5 files changed, 309 insertions(+), 8 deletions(-) create mode 100644 integration-tests/metrics_test.go diff --git a/integration-tests/metrics_test.go b/integration-tests/metrics_test.go new file mode 100644 index 000000000..4f66a70bd --- /dev/null +++ b/integration-tests/metrics_test.go @@ -0,0 +1,147 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +func mountpointMetrics(diskUsed, diskLimit, inodesUsed, inodesLimit float64) map[string]any { + return map[string]any{ + "disk_used": map[string]any{"avg": diskUsed}, + "disk_limit": map[string]any{"max": diskLimit}, + "inodes_used": map[string]any{"avg": inodesUsed}, + "inodes_limit": map[string]any{"max": inodesLimit}, + } +} + +func setupMetricsTest(t *testing.T) (f *cmdFactory, projectID string) { + authServer := mockapi.NewAuthServer(t) + t.Cleanup(authServer.Close) + + apiHandler := mockapi.NewHandler(t) + apiServer := httptest.NewServer(apiHandler) + t.Cleanup(apiServer.Close) + + projectID = mockapi.ProjectID() + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks("self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments"), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + obsPath := "/projects/" + projectID + "/environments/main/observability" + main.Links["#observability-pipeline"] = mockapi.HALLink{HREF: obsPath} + main.SetCurrentDeployment(&mockapi.Deployment{ + WebApps: map[string]mockapi.App{ + "app": {Name: "app", Type: "php:8.4", Size: "AUTO"}, + }, + Services: map[string]mockapi.App{ + "db": {Name: "db", Type: "mariadb:11.4", Size: "AUTO"}, + }, + Workers: map[string]mockapi.Worker{}, + Routes: map[string]any{}, + Links: mockapi.MakeHALLinks("self=/projects/" + projectID + "/environments/main/deployment/current"), + }) + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get(obsPath+"/resources/overview", func(w http.ResponseWriter, _ *http.Request) { + limits := map[string]any{ + "cpu_used": map[string]any{"avg": 0.1}, + "cpu_limit": map[string]any{"max": 1.0}, + "memory_used": map[string]any{"avg": 256.0}, + "memory_limit": map[string]any{"max": 1024.0}, + } + withMounts := func(mounts map[string]any) map[string]any { + m := map[string]any{"mountpoints": mounts} + for k, v := range limits { + m[k] = v + } + return m + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "_grain": 60, + "_from": 1790189400, + "_to": 1790190000, + "data": []any{map[string]any{ + "timestamp": 1790190000, + "services": map[string]any{ + "app": withMounts(map[string]any{ + "/mnt": mountpointMetrics(100, 1000, 10, 1000), + "/tmp": mountpointMetrics(500, 1000, 40, 1000), + "storage": mountpointMetrics(920, 1000, 30, 100), + }), + "db": withMounts(map[string]any{ + "/mnt": mountpointMetrics(250, 1000, 5, 1000), + "/tmp": mountpointMetrics(10, 1000, 1, 1000), + }), + }, + }}, + }) + }) + + return newCommandFactory(t, apiServer.URL, authServer.URL), projectID +} + +func TestMetricsStorage(t *testing.T) { + f, projectID := setupMetricsTest(t) + + cases := []struct { + name string + args []string + want string + }{ + { + name: "all storage columns", + args: []string{"metrics:all", "-1", "--no-header", + "-c", "service,disk_percent,storage_percent,storage_inodes_percent"}, + want: "app\t10.0%\t92.0%\t30.0%\ndb\t25.0%\t\t\n", + }, + { + name: "all includes storage by default when present", + args: []string{"metrics:all", "-1"}, + want: "Storage %", + }, + { + name: "disk-usage storage columns", + args: []string{"disk", "-1", "--no-header", "-B", + "-c", "service,storage_used,storage_limit,storage_percent,storage_ipercent"}, + want: "app\t920\t1000\t92.0%\t30.0%\ndb\t\t\t\t\n", + }, + { + name: "disk-usage --storage report", + args: []string{"disk", "-1", "--storage"}, + want: "Storage used", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + args := append([]string{}, c.args...) + args = append(args, "-p", projectID, "-e", "main", "--format", "plain") + stdout, stderr, err := f.RunCombinedOutput(args...) + require.NoError(t, err, "stderr: %s", stderr) + assert.Contains(t, stdout, c.want) + }) + } +} + +func TestMetricsStorageDisabled(t *testing.T) { + f, projectID := setupMetricsTest(t) + f.extraEnv = []string{"TEST_CLI_API_METRICS_STORAGE=0"} + + stdout, stderr, err := f.RunCombinedOutput("metrics:all", "-1", "-p", projectID, "-e", "main", "--format", "plain") + require.NoError(t, err, "stderr: %s", stderr) + assert.NotContains(t, stdout, "Storage") + + _, stderr, err = f.RunCombinedOutput("disk", "-1", "--storage", "-p", projectID, "-e", "main", "--format", "plain") + assert.Error(t, err) + assert.Contains(t, stderr, "Column not found: storage_used") +} diff --git a/legacy/config-defaults.yaml b/legacy/config-defaults.yaml index 256665a74..c032c0094 100644 --- a/legacy/config-defaults.yaml +++ b/legacy/config-defaults.yaml @@ -268,6 +268,9 @@ api: # Whether the Metrics API is enabled. metrics: false + # Whether to show network storage metrics (the "storage" mountpoint) in metrics commands. + metrics_storage: true + # Whether the Flexible Resources API (AKA sizing/scaling) is enabled. sizing: false diff --git a/legacy/src/Command/Metrics/AllMetricsCommand.php b/legacy/src/Command/Metrics/AllMetricsCommand.php index b418cafa8..091c680b9 100644 --- a/legacy/src/Command/Metrics/AllMetricsCommand.php +++ b/legacy/src/Command/Metrics/AllMetricsCommand.php @@ -52,6 +52,14 @@ class AllMetricsCommand extends MetricsCommandBase 'tmp_inodes_used' => '/tmp inodes used', 'tmp_inodes_limit' => '/tmp inodes limit', 'tmp_inodes_percent' => '/tmp inodes %', + + 'storage_used' => 'Storage used', + 'storage_limit' => 'Storage limit', + 'storage_percent' => 'Storage %', + + 'storage_inodes_used' => 'Storage inodes used', + 'storage_inodes_limit' => 'Storage inodes limit', + 'storage_inodes_percent' => 'Storage inodes %', ]; /** @var string[] */ @@ -106,7 +114,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $bytes = $input->getOption('bytes'); - $rows = $this->buildRows($values, [ + $fields = [ 'cpu_used' => new Field( Format::Rounded2p, new SourceField(MetricKind::CpuUsed, Aggregation::Avg), @@ -203,7 +211,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int new SourceField(MetricKind::InodesLimit, Aggregation::Max, '/tmp') ), ), - ], $environment); + ]; + if ($this->storageMetricsEnabled()) { + $fields += $this->storageFields($bytes); + } + $rows = $this->buildRows($values, $fields, $environment); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, 'storage_percent'); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; @@ -215,7 +228,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int )); } - $this->table->render($rows, self::TABLE_HEADER, $this->defaultColumns); + $this->table->render($rows, $header, $defaultColumns); if (!$this->table->formatIsMachineReadable()) { $this->explainHighMemoryServices(); @@ -225,4 +238,46 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } + + /** + * @return array + */ + private function storageFields(bool $bytes): array + { + $m = self::STORAGE_MOUNTPOINT; + + return [ + 'storage_used' => new Field( + $bytes ? Format::Rounded : Format::Disk, + new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), + ), + 'storage_limit' => new Field( + $bytes ? Format::Rounded : Format::Disk, + new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m), + ), + 'storage_percent' => new Field( + Format::Percent, + new SourceFieldPercentage( + new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), + new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m) + ), + ), + + 'storage_inodes_used' => new Field( + Format::Rounded, + new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), + ), + 'storage_inodes_limit' => new Field( + Format::Rounded, + new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m), + ), + 'storage_inodes_percent' => new Field( + Format::Percent, + new SourceFieldPercentage( + new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), + new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m) + ), + ), + ]; + } } diff --git a/legacy/src/Command/Metrics/DiskUsageCommand.php b/legacy/src/Command/Metrics/DiskUsageCommand.php index 5142abe59..02419cd5e 100644 --- a/legacy/src/Command/Metrics/DiskUsageCommand.php +++ b/legacy/src/Command/Metrics/DiskUsageCommand.php @@ -39,11 +39,19 @@ class DiskUsageCommand extends MetricsCommandBase 'tmp_iused' => '/tmp inodes used', 'tmp_ilimit' => '/tmp inodes limit', 'tmp_ipercent' => '/tmp inodes %', + 'storage_used' => 'Storage used', + 'storage_limit' => 'Storage limit', + 'storage_percent' => 'Storage %', + 'storage_iused' => 'Storage inodes used', + 'storage_ilimit' => 'Storage inodes limit', + 'storage_ipercent' => 'Storage inodes %', ]; /** @var string[] */ private array $defaultColumns = ['timestamp', 'service', 'used', 'limit', 'percent', 'ipercent', 'tmp_percent']; /** @var string[] */ private array $tmpReportColumns = ['timestamp', 'service', 'tmp_used', 'tmp_limit', 'tmp_percent', 'tmp_ipercent']; + /** @var string[] */ + private array $storageReportColumns = ['timestamp', 'service', 'storage_used', 'storage_limit', 'storage_percent', 'storage_ipercent']; public function __construct( private readonly PropertyFormatter $propertyFormatter, @@ -56,7 +64,8 @@ public function __construct( protected function configure(): void { $this->addOption('bytes', 'B', InputOption::VALUE_NONE, 'Show sizes in bytes') - ->addOption('tmp', null, InputOption::VALUE_NONE, 'Report temporary disk usage (shows columns: ' . implode(', ', $this->tmpReportColumns) . ')'); + ->addOption('tmp', null, InputOption::VALUE_NONE, 'Report temporary disk usage (shows columns: ' . implode(', ', $this->tmpReportColumns) . ')') + ->addOption('storage', null, InputOption::VALUE_NONE, 'Report network storage usage, if available (shows columns: ' . implode(', ', $this->storageReportColumns) . ')'); $this->addMetricsOptions(); $this->selector->addProjectOption($this->getDefinition()); $this->selector->addEnvironmentOption($this->getDefinition()); @@ -67,8 +76,13 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { + if ($input->getOption('tmp') && $input->getOption('storage')) { + throw new \InvalidArgumentException('The --tmp and --storage options cannot be combined.'); + } if ($input->getOption('tmp')) { $input->setOption('columns', $this->tmpReportColumns); + } elseif ($input->getOption('storage')) { + $input->setOption('columns', $this->storageReportColumns); } $this->table->removeDeprecatedColumns(['interval'], '', $input, $output); @@ -76,7 +90,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $bytes = $input->getOption('bytes'); - $rows = $this->buildRows($values, [ + $fields = [ 'used' => new Field( $bytes ? Format::Rounded : Format::Disk, new SourceField(MetricKind::DiskUsed, Aggregation::Avg, '/mnt'), @@ -140,21 +154,68 @@ protected function execute(InputInterface $input, OutputInterface $output): int new SourceField(MetricKind::InodesLimit, Aggregation::Max, '/tmp') ), ), - ], $environment); + ]; + if ($this->storageMetricsEnabled()) { + $fields += $this->storageFields($bytes); + } + $rows = $this->buildRows($values, $fields, $environment); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, 'storage_percent'); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; $this->stdErr->writeln(\sprintf( 'Average %s at %s intervals from %s to %s:', - $input->getOption('tmp') ? 'temporary disk usage' : 'disk usage', + $input->getOption('tmp') ? 'temporary disk usage' : ($input->getOption('storage') ? 'storage usage' : 'disk usage'), (new Duration())->humanize($values['_grain']), $formatter->formatDate($values['_from']), $formatter->formatDate($values['_to']), )); } - $this->table->render($rows, self::TABLE_HEADER, $this->defaultColumns); + $this->table->render($rows, $header, $defaultColumns); return 0; } + + /** + * @return array + */ + private function storageFields(bool $bytes): array + { + $m = self::STORAGE_MOUNTPOINT; + + return [ + 'storage_used' => new Field( + $bytes ? Format::Rounded : Format::Disk, + new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), + ), + 'storage_limit' => new Field( + $bytes ? Format::Rounded : Format::Disk, + new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m), + ), + 'storage_percent' => new Field( + Format::Percent, + new SourceFieldPercentage( + new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), + new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m) + ), + ), + + 'storage_iused' => new Field( + Format::Rounded, + new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), + ), + 'storage_ilimit' => new Field( + Format::Rounded, + new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m), + ), + 'storage_ipercent' => new Field( + Format::Percent, + new SourceFieldPercentage( + new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), + new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m) + ), + ), + ]; + } } diff --git a/legacy/src/Command/Metrics/MetricsCommandBase.php b/legacy/src/Command/Metrics/MetricsCommandBase.php index 28074594c..7d36638f9 100644 --- a/legacy/src/Command/Metrics/MetricsCommandBase.php +++ b/legacy/src/Command/Metrics/MetricsCommandBase.php @@ -42,6 +42,9 @@ abstract class MetricsCommandBase extends CommandBase public const MIN_RANGE = 300; // 5 minutes public const DEFAULT_RANGE = 600; + // The mountpoint key of the network storage volume (used by "storage" mounts). + public const STORAGE_MOUNTPOINT = 'storage'; + /** * @var bool whether services have been identified that use high memory */ @@ -250,6 +253,38 @@ private function getServices(InputInterface $input, Environment $environment): a return $selectedServiceNames; } + protected function storageMetricsEnabled(): bool + { + return $this->config->getBool('api.metrics_storage'); + } + + /** + * Adjusts the table header and default columns for storage metrics. + * + * Storage columns are removed if storage metrics are disabled. Otherwise, + * the $defaultColumn is shown by default if any service reports storage. + * + * @param array $header + * @param string[] $defaultColumns + * @param array $values + * @return array{array, string[]} + */ + protected function storageColumns(array $header, array $defaultColumns, array $values, string $defaultColumn): array + { + if (!$this->storageMetricsEnabled()) { + return [array_filter($header, fn($key): bool => !str_starts_with($key, 'storage_'), ARRAY_FILTER_USE_KEY), $defaultColumns]; + } + foreach ($values['data'] as $point) { + foreach ($point['services'] ?? [] as $service) { + if (isset($service['mountpoints'][self::STORAGE_MOUNTPOINT])) { + return [$header, array_merge($defaultColumns, [$defaultColumn])]; + } + } + } + + return [$header, $defaultColumns]; + } + protected function getChooseEnvFilter(): ?callable { return null; From c19ea13cac12598b9c53a2be44a8eda02b52bc56 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 23 Sep 2026 20:23:22 +0100 Subject: [PATCH 2/7] refactor(metrics): show storage columns automatically - Remove the disk --storage option. Instead, disk shows storage_used, storage_limit and storage_percent by default (metrics:all shows storage_percent). - In tables, storage columns are shown only if a service reports storage. Machine-readable formats always include them, for stable output. - Return empty values instead of failing when the storage mountpoint lacks a metric (e.g. inodes). - Share storageFields() between both commands. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_test.go | 106 ++++++++++++------ .../src/Command/Metrics/AllMetricsCommand.php | 46 +------- .../src/Command/Metrics/DiskUsageCommand.php | 58 +--------- .../Command/Metrics/MetricsCommandBase.php | 61 +++++++++- 4 files changed, 134 insertions(+), 137 deletions(-) diff --git a/integration-tests/metrics_test.go b/integration-tests/metrics_test.go index 4f66a70bd..fa2ebe910 100644 --- a/integration-tests/metrics_test.go +++ b/integration-tests/metrics_test.go @@ -21,7 +21,7 @@ func mountpointMetrics(diskUsed, diskLimit, inodesUsed, inodesLimit float64) map } } -func setupMetricsTest(t *testing.T) (f *cmdFactory, projectID string) { +func setupMetricsTest(t *testing.T, withStorage bool) (f *cmdFactory, projectID string) { authServer := mockapi.NewAuthServer(t) t.Cleanup(authServer.Close) @@ -67,6 +67,22 @@ func setupMetricsTest(t *testing.T) (f *cmdFactory, projectID string) { } return m } + appMounts := map[string]any{ + "/mnt": mountpointMetrics(100, 1000, 10, 1000), + "/tmp": mountpointMetrics(500, 1000, 40, 1000), + } + dbMounts := map[string]any{ + "/mnt": mountpointMetrics(250, 1000, 5, 1000), + "/tmp": mountpointMetrics(10, 1000, 1, 1000), + } + if withStorage { + appMounts["storage"] = mountpointMetrics(920, 1000, 30, 100) + // Storage without inode metrics. + dbMounts["storage"] = map[string]any{ + "disk_used": map[string]any{"avg": 500}, + "disk_limit": map[string]any{"max": 1000}, + } + } _ = json.NewEncoder(w).Encode(map[string]any{ "_grain": 60, "_from": 1790189400, @@ -74,15 +90,8 @@ func setupMetricsTest(t *testing.T) (f *cmdFactory, projectID string) { "data": []any{map[string]any{ "timestamp": 1790190000, "services": map[string]any{ - "app": withMounts(map[string]any{ - "/mnt": mountpointMetrics(100, 1000, 10, 1000), - "/tmp": mountpointMetrics(500, 1000, 40, 1000), - "storage": mountpointMetrics(920, 1000, 30, 100), - }), - "db": withMounts(map[string]any{ - "/mnt": mountpointMetrics(250, 1000, 5, 1000), - "/tmp": mountpointMetrics(10, 1000, 1, 1000), - }), + "app": withMounts(appMounts), + "db": withMounts(dbMounts), }, }}, }) @@ -92,56 +101,81 @@ func setupMetricsTest(t *testing.T) (f *cmdFactory, projectID string) { } func TestMetricsStorage(t *testing.T) { - f, projectID := setupMetricsTest(t) - cases := []struct { - name string - args []string - want string + name string + withStorage bool + env []string + args []string + want string + notWant string }{ { - name: "all storage columns", - args: []string{"metrics:all", "-1", "--no-header", + name: "all storage columns", + withStorage: true, + args: []string{"metrics:all", "-1", "--no-header", "--format", "plain", "-c", "service,disk_percent,storage_percent,storage_inodes_percent"}, - want: "app\t10.0%\t92.0%\t30.0%\ndb\t25.0%\t\t\n", + want: "app\t10.0%\t92.0%\t30.0%\ndb\t25.0%\t50.0%\t\n", + }, + { + name: "all table shows storage when present", + withStorage: true, + args: []string{"metrics:all", "-1"}, + want: "Storage %", }, { - name: "all includes storage by default when present", - args: []string{"metrics:all", "-1"}, - want: "Storage %", + name: "all table hides storage when absent", + args: []string{"metrics:all", "-1"}, + notWant: "Storage", }, { - name: "disk-usage storage columns", - args: []string{"disk", "-1", "--no-header", "-B", + name: "all csv always includes storage", + args: []string{"metrics:all", "-1", "--format", "csv"}, + want: "/tmp inodes %,Storage %\n", + }, + { + name: "disk-usage storage columns", + withStorage: true, + args: []string{"disk", "-1", "--no-header", "-B", "--format", "plain", "-c", "service,storage_used,storage_limit,storage_percent,storage_ipercent"}, - want: "app\t920\t1000\t92.0%\t30.0%\ndb\t\t\t\t\n", + want: "app\t920\t1000\t92.0%\t30.0%\ndb\t500\t1000\t50.0%\t\n", + }, + { + name: "disk-usage table shows storage when present", + withStorage: true, + args: []string{"disk", "-1"}, + want: "Storage used", }, { - name: "disk-usage --storage report", - args: []string{"disk", "-1", "--storage"}, - want: "Storage used", + name: "disabled", + withStorage: true, + env: []string{"TEST_CLI_API_METRICS_STORAGE=0"}, + args: []string{"metrics:all", "-1", "--format", "csv"}, + notWant: "Storage", }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { + f, projectID := setupMetricsTest(t, c.withStorage) + f.extraEnv = c.env args := append([]string{}, c.args...) - args = append(args, "-p", projectID, "-e", "main", "--format", "plain") + args = append(args, "-p", projectID, "-e", "main") stdout, stderr, err := f.RunCombinedOutput(args...) require.NoError(t, err, "stderr: %s", stderr) - assert.Contains(t, stdout, c.want) + if c.want != "" { + assert.Contains(t, stdout, c.want) + } + if c.notWant != "" { + assert.NotContains(t, stdout, c.notWant) + } }) } } -func TestMetricsStorageDisabled(t *testing.T) { - f, projectID := setupMetricsTest(t) +func TestMetricsStorageDisabledColumn(t *testing.T) { + f, projectID := setupMetricsTest(t, true) f.extraEnv = []string{"TEST_CLI_API_METRICS_STORAGE=0"} - stdout, stderr, err := f.RunCombinedOutput("metrics:all", "-1", "-p", projectID, "-e", "main", "--format", "plain") - require.NoError(t, err, "stderr: %s", stderr) - assert.NotContains(t, stdout, "Storage") - - _, stderr, err = f.RunCombinedOutput("disk", "-1", "--storage", "-p", projectID, "-e", "main", "--format", "plain") + _, stderr, err := f.RunCombinedOutput("disk", "-1", "-c", "storage_used", "-p", projectID, "-e", "main") assert.Error(t, err) assert.Contains(t, stderr, "Column not found: storage_used") } diff --git a/legacy/src/Command/Metrics/AllMetricsCommand.php b/legacy/src/Command/Metrics/AllMetricsCommand.php index 091c680b9..0763395de 100644 --- a/legacy/src/Command/Metrics/AllMetricsCommand.php +++ b/legacy/src/Command/Metrics/AllMetricsCommand.php @@ -213,10 +213,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int ), ]; if ($this->storageMetricsEnabled()) { - $fields += $this->storageFields($bytes); + $fields += $this->storageFields($bytes, 'storage_inodes_'); } $rows = $this->buildRows($values, $fields, $environment); - [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, 'storage_percent'); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_percent']); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; @@ -238,46 +238,4 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } - - /** - * @return array - */ - private function storageFields(bool $bytes): array - { - $m = self::STORAGE_MOUNTPOINT; - - return [ - 'storage_used' => new Field( - $bytes ? Format::Rounded : Format::Disk, - new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), - ), - 'storage_limit' => new Field( - $bytes ? Format::Rounded : Format::Disk, - new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m), - ), - 'storage_percent' => new Field( - Format::Percent, - new SourceFieldPercentage( - new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), - new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m) - ), - ), - - 'storage_inodes_used' => new Field( - Format::Rounded, - new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), - ), - 'storage_inodes_limit' => new Field( - Format::Rounded, - new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m), - ), - 'storage_inodes_percent' => new Field( - Format::Percent, - new SourceFieldPercentage( - new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), - new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m) - ), - ), - ]; - } } diff --git a/legacy/src/Command/Metrics/DiskUsageCommand.php b/legacy/src/Command/Metrics/DiskUsageCommand.php index 02419cd5e..bca54a2e0 100644 --- a/legacy/src/Command/Metrics/DiskUsageCommand.php +++ b/legacy/src/Command/Metrics/DiskUsageCommand.php @@ -50,8 +50,6 @@ class DiskUsageCommand extends MetricsCommandBase private array $defaultColumns = ['timestamp', 'service', 'used', 'limit', 'percent', 'ipercent', 'tmp_percent']; /** @var string[] */ private array $tmpReportColumns = ['timestamp', 'service', 'tmp_used', 'tmp_limit', 'tmp_percent', 'tmp_ipercent']; - /** @var string[] */ - private array $storageReportColumns = ['timestamp', 'service', 'storage_used', 'storage_limit', 'storage_percent', 'storage_ipercent']; public function __construct( private readonly PropertyFormatter $propertyFormatter, @@ -64,8 +62,7 @@ public function __construct( protected function configure(): void { $this->addOption('bytes', 'B', InputOption::VALUE_NONE, 'Show sizes in bytes') - ->addOption('tmp', null, InputOption::VALUE_NONE, 'Report temporary disk usage (shows columns: ' . implode(', ', $this->tmpReportColumns) . ')') - ->addOption('storage', null, InputOption::VALUE_NONE, 'Report network storage usage, if available (shows columns: ' . implode(', ', $this->storageReportColumns) . ')'); + ->addOption('tmp', null, InputOption::VALUE_NONE, 'Report temporary disk usage (shows columns: ' . implode(', ', $this->tmpReportColumns) . ')'); $this->addMetricsOptions(); $this->selector->addProjectOption($this->getDefinition()); $this->selector->addEnvironmentOption($this->getDefinition()); @@ -76,13 +73,8 @@ protected function configure(): void protected function execute(InputInterface $input, OutputInterface $output): int { - if ($input->getOption('tmp') && $input->getOption('storage')) { - throw new \InvalidArgumentException('The --tmp and --storage options cannot be combined.'); - } if ($input->getOption('tmp')) { $input->setOption('columns', $this->tmpReportColumns); - } elseif ($input->getOption('storage')) { - $input->setOption('columns', $this->storageReportColumns); } $this->table->removeDeprecatedColumns(['interval'], '', $input, $output); @@ -156,16 +148,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int ), ]; if ($this->storageMetricsEnabled()) { - $fields += $this->storageFields($bytes); + $fields += $this->storageFields($bytes, 'storage_i'); } $rows = $this->buildRows($values, $fields, $environment); - [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, 'storage_percent'); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_used', 'storage_limit', 'storage_percent']); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; $this->stdErr->writeln(\sprintf( 'Average %s at %s intervals from %s to %s:', - $input->getOption('tmp') ? 'temporary disk usage' : ($input->getOption('storage') ? 'storage usage' : 'disk usage'), + $input->getOption('tmp') ? 'temporary disk usage' : 'disk usage', (new Duration())->humanize($values['_grain']), $formatter->formatDate($values['_from']), $formatter->formatDate($values['_to']), @@ -176,46 +168,4 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 0; } - - /** - * @return array - */ - private function storageFields(bool $bytes): array - { - $m = self::STORAGE_MOUNTPOINT; - - return [ - 'storage_used' => new Field( - $bytes ? Format::Rounded : Format::Disk, - new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), - ), - 'storage_limit' => new Field( - $bytes ? Format::Rounded : Format::Disk, - new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m), - ), - 'storage_percent' => new Field( - Format::Percent, - new SourceFieldPercentage( - new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), - new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m) - ), - ), - - 'storage_iused' => new Field( - Format::Rounded, - new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), - ), - 'storage_ilimit' => new Field( - Format::Rounded, - new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m), - ), - 'storage_ipercent' => new Field( - Format::Percent, - new SourceFieldPercentage( - new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), - new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m) - ), - ), - ]; - } } diff --git a/legacy/src/Command/Metrics/MetricsCommandBase.php b/legacy/src/Command/Metrics/MetricsCommandBase.php index 7d36638f9..a4ceb4302 100644 --- a/legacy/src/Command/Metrics/MetricsCommandBase.php +++ b/legacy/src/Command/Metrics/MetricsCommandBase.php @@ -4,7 +4,10 @@ namespace Platformsh\Cli\Command\Metrics; +use Platformsh\Cli\Model\Metrics\Aggregation; use Platformsh\Cli\Model\Metrics\Field; +use Platformsh\Cli\Model\Metrics\Format; +use Platformsh\Cli\Model\Metrics\MetricKind; use Platformsh\Cli\Model\Metrics\SourceField; use Platformsh\Cli\Model\Metrics\SourceFieldPercentage; use Platformsh\Cli\Selector\Selector; @@ -258,26 +261,74 @@ protected function storageMetricsEnabled(): bool return $this->config->getBool('api.metrics_storage'); } + /** + * Returns fields for the storage volume, with inode fields keyed by $inodesPrefix. + * + * @return array + */ + protected function storageFields(bool $bytes, string $inodesPrefix): array + { + $m = self::STORAGE_MOUNTPOINT; + + return [ + 'storage_used' => new Field( + $bytes ? Format::Rounded : Format::Disk, + new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), + ), + 'storage_limit' => new Field( + $bytes ? Format::Rounded : Format::Disk, + new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m), + ), + 'storage_percent' => new Field( + Format::Percent, + new SourceFieldPercentage( + new SourceField(MetricKind::DiskUsed, Aggregation::Avg, $m), + new SourceField(MetricKind::DiskLimit, Aggregation::Max, $m) + ), + ), + $inodesPrefix . 'used' => new Field( + Format::Rounded, + new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), + ), + $inodesPrefix . 'limit' => new Field( + Format::Rounded, + new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m), + ), + $inodesPrefix . 'percent' => new Field( + Format::Percent, + new SourceFieldPercentage( + new SourceField(MetricKind::InodesUsed, Aggregation::Avg, $m), + new SourceField(MetricKind::InodesLimit, Aggregation::Max, $m) + ), + ), + ]; + } + /** * Adjusts the table header and default columns for storage metrics. * * Storage columns are removed if storage metrics are disabled. Otherwise, - * the $defaultColumn is shown by default if any service reports storage. + * the $storageColumns are shown by default in machine-readable formats + * (for stable output), or in tables if any service reports storage. * * @param array $header * @param string[] $defaultColumns * @param array $values + * @param string[] $storageColumns * @return array{array, string[]} */ - protected function storageColumns(array $header, array $defaultColumns, array $values, string $defaultColumn): array + protected function storageColumns(array $header, array $defaultColumns, array $values, array $storageColumns): array { if (!$this->storageMetricsEnabled()) { return [array_filter($header, fn($key): bool => !str_starts_with($key, 'storage_'), ARRAY_FILTER_USE_KEY), $defaultColumns]; } + if ($this->table->formatIsMachineReadable()) { + return [$header, array_merge($defaultColumns, $storageColumns)]; + } foreach ($values['data'] as $point) { foreach ($point['services'] ?? [] as $service) { if (isset($service['mountpoints'][self::STORAGE_MOUNTPOINT])) { - return [$header, array_merge($defaultColumns, [$defaultColumn])]; + return [$header, array_merge($defaultColumns, $storageColumns)]; } } } @@ -465,6 +516,10 @@ private function extractValue(array $point, SourceField $sourceField): ?float if (!isset($point['mountpoints'][$sourceField->mountpoint])) { return null; } + // The storage volume may not report every metric. + if ($sourceField->mountpoint === self::STORAGE_MOUNTPOINT) { + return $point['mountpoints'][$sourceField->mountpoint][$sourceField->source->value][$sourceField->aggregation->value] ?? null; + } if (!isset($point['mountpoints'][$sourceField->mountpoint][$sourceField->source->value])) { throw new \RuntimeException(\sprintf('Source "%s" not found in the mountpoint "%s".', $sourceField->source->value, $sourceField->mountpoint)); } From 04cff8598f32e7f6c110e3f07c6ab1d05a2151e2 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Wed, 23 Sep 2026 20:29:04 +0100 Subject: [PATCH 3/7] fix(metrics): show blank percentages when usage is unknown A limit without a matching used value was shown as 0%. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_test.go | 7 ++++--- legacy/src/Command/Metrics/MetricsCommandBase.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/integration-tests/metrics_test.go b/integration-tests/metrics_test.go index fa2ebe910..312a62f03 100644 --- a/integration-tests/metrics_test.go +++ b/integration-tests/metrics_test.go @@ -77,10 +77,11 @@ func setupMetricsTest(t *testing.T, withStorage bool) (f *cmdFactory, projectID } if withStorage { appMounts["storage"] = mountpointMetrics(920, 1000, 30, 100) - // Storage without inode metrics. + // Storage with a partial set of inode metrics. dbMounts["storage"] = map[string]any{ - "disk_used": map[string]any{"avg": 500}, - "disk_limit": map[string]any{"max": 1000}, + "disk_used": map[string]any{"avg": 500}, + "disk_limit": map[string]any{"max": 1000}, + "inodes_limit": map[string]any{"max": 100}, } } _ = json.NewEncoder(w).Encode(map[string]any{ diff --git a/legacy/src/Command/Metrics/MetricsCommandBase.php b/legacy/src/Command/Metrics/MetricsCommandBase.php index a4ceb4302..5c4e584aa 100644 --- a/legacy/src/Command/Metrics/MetricsCommandBase.php +++ b/legacy/src/Command/Metrics/MetricsCommandBase.php @@ -499,7 +499,7 @@ private function getValueFromSource(array $point, SourceField|SourceFieldPercent $value = $this->extractValue($point, $fieldDefinition->value); $limit = $this->extractValue($point, $fieldDefinition->limit); - return $limit > 0 ? $value / $limit * 100 : null; + return $value !== null && $limit > 0 ? $value / $limit * 100 : null; } return $this->extractValue($point, $fieldDefinition); From b28edb65a7ff42c795d17c0f410b8f531d500fc2 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 11:11:17 +0100 Subject: [PATCH 4/7] feat(metrics): show storage inode usage by default in disk Matches the /mnt defaults, which include inode usage. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_storage_test.go | 5 +++++ legacy/src/Command/Metrics/DiskUsageCommand.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/integration-tests/metrics_storage_test.go b/integration-tests/metrics_storage_test.go index 69f95d87c..2376a8915 100644 --- a/integration-tests/metrics_storage_test.go +++ b/integration-tests/metrics_storage_test.go @@ -150,6 +150,11 @@ func TestMetricsStorage(t *testing.T) { args: []string{"disk", "-1"}, want: "Storage used", }, + { + name: "disk-usage csv always includes storage", + args: []string{"disk", "-1", "--format", "csv"}, + want: "/tmp %,Storage used,Storage limit,Storage %,Storage inodes %\n", + }, { name: "disabled", withStorage: true, diff --git a/legacy/src/Command/Metrics/DiskUsageCommand.php b/legacy/src/Command/Metrics/DiskUsageCommand.php index bca54a2e0..77c93cca9 100644 --- a/legacy/src/Command/Metrics/DiskUsageCommand.php +++ b/legacy/src/Command/Metrics/DiskUsageCommand.php @@ -151,7 +151,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $fields += $this->storageFields($bytes, 'storage_i'); } $rows = $this->buildRows($values, $fields, $environment); - [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_used', 'storage_limit', 'storage_percent']); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_used', 'storage_limit', 'storage_percent', 'storage_ipercent']); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; From 8c7e7dc6e45aa501e7efa0890311cebe7e49add1 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 11:18:59 +0100 Subject: [PATCH 5/7] feat(metrics): show storage columns if the deployment has storage mounts Tables previously showed storage columns only if a data point reported the storage mountpoint. Also check for mounts with "source: storage" in the current deployment, so the columns do not depend on the data. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_storage_test.go | 31 +++++++++++++------ .../src/Command/Metrics/AllMetricsCommand.php | 2 +- .../src/Command/Metrics/DiskUsageCommand.php | 2 +- .../Command/Metrics/MetricsCommandBase.php | 29 ++++++++++++++--- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/integration-tests/metrics_storage_test.go b/integration-tests/metrics_storage_test.go index 2376a8915..854daabf7 100644 --- a/integration-tests/metrics_storage_test.go +++ b/integration-tests/metrics_storage_test.go @@ -21,7 +21,7 @@ func mountpointMetrics(diskUsed, diskLimit, inodesUsed, inodesLimit float64) map } } -func setupMetricsTest(t *testing.T, withStorage bool) (f *cmdFactory, projectID string) { +func setupMetricsTest(t *testing.T, withStorage, withStorageMount bool) (f *cmdFactory, projectID string) { authServer := mockapi.NewAuthServer(t) t.Cleanup(authServer.Close) @@ -39,9 +39,13 @@ func setupMetricsTest(t *testing.T, withStorage bool) (f *cmdFactory, projectID main := makeEnv(projectID, "main", "production", "active", nil) obsPath := "/projects/" + projectID + "/environments/main/observability" + app := mockapi.App{Name: "app", Type: "php:8.4", Size: "AUTO"} + if withStorageMount { + app.Mounts = map[string]mockapi.Mount{"/files": {Source: "storage", SourcePath: "files"}} + } main.SetCurrentDeployment(&mockapi.Deployment{ WebApps: map[string]mockapi.App{ - "app": {Name: "app", Type: "php:8.4", Size: "AUTO"}, + "app": app, }, Services: map[string]mockapi.App{ "db": {Name: "db", Type: "mariadb:11.4", Size: "AUTO"}, @@ -107,12 +111,13 @@ func setupMetricsTest(t *testing.T, withStorage bool) (f *cmdFactory, projectID func TestMetricsStorage(t *testing.T) { cases := []struct { - name string - withStorage bool - env []string - args []string - want string - notWant string + name string + withStorage bool + withStorageMount bool + env []string + args []string + want string + notWant string }{ { name: "all storage columns", @@ -132,6 +137,12 @@ func TestMetricsStorage(t *testing.T) { args: []string{"metrics:all", "-1"}, notWant: "Storage", }, + { + name: "all table shows storage when the deployment has storage mounts", + withStorageMount: true, + args: []string{"metrics:all", "-1"}, + want: "Storage %", + }, { name: "all csv always includes storage", args: []string{"metrics:all", "-1", "--format", "csv"}, @@ -165,7 +176,7 @@ func TestMetricsStorage(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - f, projectID := setupMetricsTest(t, c.withStorage) + f, projectID := setupMetricsTest(t, c.withStorage, c.withStorageMount) f.extraEnv = c.env args := append([]string{}, c.args...) args = append(args, "-p", projectID, "-e", "main") @@ -182,7 +193,7 @@ func TestMetricsStorage(t *testing.T) { } func TestMetricsStorageDisabledColumn(t *testing.T) { - f, projectID := setupMetricsTest(t, true) + f, projectID := setupMetricsTest(t, true, false) f.extraEnv = []string{"TEST_CLI_API_METRICS_STORAGE=0"} _, stderr, err := f.RunCombinedOutput("disk", "-1", "-c", "storage_used", "-p", projectID, "-e", "main") diff --git a/legacy/src/Command/Metrics/AllMetricsCommand.php b/legacy/src/Command/Metrics/AllMetricsCommand.php index 0763395de..8bf8f35db 100644 --- a/legacy/src/Command/Metrics/AllMetricsCommand.php +++ b/legacy/src/Command/Metrics/AllMetricsCommand.php @@ -216,7 +216,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $fields += $this->storageFields($bytes, 'storage_inodes_'); } $rows = $this->buildRows($values, $fields, $environment); - [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_percent']); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_percent'], $environment); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; diff --git a/legacy/src/Command/Metrics/DiskUsageCommand.php b/legacy/src/Command/Metrics/DiskUsageCommand.php index 77c93cca9..bd1433766 100644 --- a/legacy/src/Command/Metrics/DiskUsageCommand.php +++ b/legacy/src/Command/Metrics/DiskUsageCommand.php @@ -151,7 +151,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $fields += $this->storageFields($bytes, 'storage_i'); } $rows = $this->buildRows($values, $fields, $environment); - [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_used', 'storage_limit', 'storage_percent', 'storage_ipercent']); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_used', 'storage_limit', 'storage_percent', 'storage_ipercent'], $environment); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter; diff --git a/legacy/src/Command/Metrics/MetricsCommandBase.php b/legacy/src/Command/Metrics/MetricsCommandBase.php index cd729b3a1..d3caab5c8 100644 --- a/legacy/src/Command/Metrics/MetricsCommandBase.php +++ b/legacy/src/Command/Metrics/MetricsCommandBase.php @@ -341,7 +341,7 @@ protected function storageFields(bool $bytes, string $inodesPrefix): array * * Storage columns are removed if storage metrics are disabled. Otherwise, * the $storageColumns are shown by default in machine-readable formats - * (for stable output), or in tables if any service reports storage. + * (for stable output), or in tables if the environment uses storage. * * @param array $header * @param string[] $defaultColumns @@ -349,23 +349,42 @@ protected function storageFields(bool $bytes, string $inodesPrefix): array * @param string[] $storageColumns * @return array{array, string[]} */ - protected function storageColumns(array $header, array $defaultColumns, array $values, array $storageColumns): array + protected function storageColumns(array $header, array $defaultColumns, array $values, array $storageColumns, Environment $environment): array { if (!$this->storageMetricsEnabled()) { return [array_filter($header, fn($key): bool => !str_starts_with($key, 'storage_'), ARRAY_FILTER_USE_KEY), $defaultColumns]; } - if ($this->table->formatIsMachineReadable()) { + if ($this->table->formatIsMachineReadable() || $this->usesStorage($values, $environment)) { return [$header, array_merge($defaultColumns, $storageColumns)]; } + + return [$header, $defaultColumns]; + } + + /** + * Checks if the deployment has storage mounts, or if any service reports storage. + * + * @param array $values + */ + private function usesStorage(array $values, Environment $environment): bool + { + $deployment = $this->api->getCurrentDeployment($environment); + foreach (array_merge($deployment->webapps, $deployment->workers) as $app) { + foreach ($app->mounts as $mount) { + if (($mount['source'] ?? null) === 'storage') { + return true; + } + } + } foreach ($values['data'] as $point) { foreach ($point['services'] ?? [] as $service) { if (isset($service['mountpoints'][self::STORAGE_MOUNTPOINT])) { - return [$header, array_merge($defaultColumns, $storageColumns)]; + return true; } } } - return [$header, $defaultColumns]; + return false; } protected function getChooseEnvFilter(): ?callable From e92342d185a4bc3440a178a2ceccb547c04059ea Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 11:23:53 +0100 Subject: [PATCH 6/7] fix(metrics): only check storage mounts of the returned services - Ignore apps and workers excluded by --service or --type, so filtered tables do not gain an always-empty storage column. - Tolerate a missing or null "mounts" deployment property. - Filter by services[] in the test mock, as the API does. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_storage_test.go | 34 +++++++++++++++++-- .../Command/Metrics/MetricsCommandBase.php | 21 +++++++----- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/integration-tests/metrics_storage_test.go b/integration-tests/metrics_storage_test.go index 854daabf7..ca07b9af4 100644 --- a/integration-tests/metrics_storage_test.go +++ b/integration-tests/metrics_storage_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -21,6 +22,26 @@ func mountpointMetrics(diskUsed, diskLimit, inodesUsed, inodesLimit float64) map } } +// filterServices applies the services[] query filter, like the API. +func filterServices(req *http.Request, services map[string]any) map[string]any { + var names []string + for k, v := range req.URL.Query() { + if strings.HasPrefix(k, "services[") { + names = append(names, v...) + } + } + if len(names) == 0 { + return services + } + filtered := make(map[string]any) + for _, n := range names { + if svc, ok := services[n]; ok { + filtered[n] = svc + } + } + return filtered +} + func setupMetricsTest(t *testing.T, withStorage, withStorageMount bool) (f *cmdFactory, projectID string) { authServer := mockapi.NewAuthServer(t) t.Cleanup(authServer.Close) @@ -61,7 +82,7 @@ func setupMetricsTest(t *testing.T, withStorage, withStorageMount bool) (f *cmdF "_links": mockapi.MakeHALLinks("resources_overview=" + apiServer.URL + obsPath + "/resources/overview"), }) }) - apiHandler.Get(obsPath+"/resources/overview", func(w http.ResponseWriter, _ *http.Request) { + apiHandler.Get(obsPath+"/resources/overview", func(w http.ResponseWriter, req *http.Request) { limits := map[string]any{ "cpu_used": map[string]any{"avg": 0.1}, "cpu_limit": map[string]any{"max": 1.0}, @@ -98,10 +119,10 @@ func setupMetricsTest(t *testing.T, withStorage, withStorageMount bool) (f *cmdF "_to": 1790190000, "data": []any{map[string]any{ "timestamp": 1790190000, - "services": map[string]any{ + "services": filterServices(req, map[string]any{ "app": withMounts(appMounts), "db": withMounts(dbMounts), - }, + }), }}, }) }) @@ -143,6 +164,12 @@ func TestMetricsStorage(t *testing.T) { args: []string{"metrics:all", "-1"}, want: "Storage %", }, + { + name: "all table ignores storage mounts of filtered-out apps", + withStorageMount: true, + args: []string{"metrics:all", "-1", "-s", "db"}, + notWant: "Storage", + }, { name: "all csv always includes storage", args: []string{"metrics:all", "-1", "--format", "csv"}, @@ -182,6 +209,7 @@ func TestMetricsStorage(t *testing.T) { args = append(args, "-p", projectID, "-e", "main") stdout, stderr, err := f.RunCombinedOutput(args...) require.NoError(t, err, "stderr: %s", stderr) + assert.NotContains(t, stderr, "Warning") if c.want != "" { assert.Contains(t, stdout, c.want) } diff --git a/legacy/src/Command/Metrics/MetricsCommandBase.php b/legacy/src/Command/Metrics/MetricsCommandBase.php index d3caab5c8..9838114e1 100644 --- a/legacy/src/Command/Metrics/MetricsCommandBase.php +++ b/legacy/src/Command/Metrics/MetricsCommandBase.php @@ -362,23 +362,28 @@ protected function storageColumns(array $header, array $defaultColumns, array $v } /** - * Checks if the deployment has storage mounts, or if any service reports storage. + * Checks if any of the returned services reports storage or has storage mounts. * * @param array $values */ private function usesStorage(array $values, Environment $environment): bool { - $deployment = $this->api->getCurrentDeployment($environment); - foreach (array_merge($deployment->webapps, $deployment->workers) as $app) { - foreach ($app->mounts as $mount) { - if (($mount['source'] ?? null) === 'storage') { + $serviceNames = []; + foreach ($values['data'] as $point) { + foreach ($point['services'] ?? [] as $name => $service) { + if (isset($service['mountpoints'][self::STORAGE_MOUNTPOINT])) { return true; } + $serviceNames[$name] = true; } } - foreach ($values['data'] as $point) { - foreach ($point['services'] ?? [] as $service) { - if (isset($service['mountpoints'][self::STORAGE_MOUNTPOINT])) { + $deployment = $this->api->getCurrentDeployment($environment); + foreach (array_merge($deployment->webapps, $deployment->workers) as $name => $app) { + if (!isset($serviceNames[$name])) { + continue; + } + foreach ($app->getProperty('mounts', false) ?: [] as $mount) { + if (($mount['source'] ?? null) === 'storage') { return true; } } From 17f2a29a25f6fde4c14d096c57fff50a8ade6586 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 24 Sep 2026 11:57:15 +0100 Subject: [PATCH 7/7] feat(metrics): show storage inode usage by default in metrics:all Matches the /mnt and /tmp defaults, and the disk command. Co-Authored-By: Claude Opus 5.5 --- integration-tests/metrics_storage_test.go | 2 +- legacy/src/Command/Metrics/AllMetricsCommand.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/metrics_storage_test.go b/integration-tests/metrics_storage_test.go index ca07b9af4..1de18defc 100644 --- a/integration-tests/metrics_storage_test.go +++ b/integration-tests/metrics_storage_test.go @@ -173,7 +173,7 @@ func TestMetricsStorage(t *testing.T) { { name: "all csv always includes storage", args: []string{"metrics:all", "-1", "--format", "csv"}, - want: "/tmp inodes %,Storage %\n", + want: "/tmp inodes %,Storage %,Storage inodes %\n", }, { name: "disk-usage storage columns", diff --git a/legacy/src/Command/Metrics/AllMetricsCommand.php b/legacy/src/Command/Metrics/AllMetricsCommand.php index 8bf8f35db..a8308c60d 100644 --- a/legacy/src/Command/Metrics/AllMetricsCommand.php +++ b/legacy/src/Command/Metrics/AllMetricsCommand.php @@ -216,7 +216,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $fields += $this->storageFields($bytes, 'storage_inodes_'); } $rows = $this->buildRows($values, $fields, $environment); - [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_percent'], $environment); + [$header, $defaultColumns] = $this->storageColumns(self::TABLE_HEADER, $this->defaultColumns, $values, ['storage_percent', 'storage_inodes_percent'], $environment); if (!$this->table->formatIsMachineReadable()) { $formatter = $this->propertyFormatter;