storage_spaces: move storage pool and virtual disk metrics out of mscluster (#2646)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

internal/collector/lifecycle_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/prometheus-community/windows_exporter/internal/collector/mssql"
 	"github.com/prometheus-community/windows_exporter/internal/collector/net"
 	"github.com/prometheus-community/windows_exporter/internal/collector/netframework"
+	"github.com/prometheus-community/windows_exporter/internal/collector/storage_spaces"
 	"github.com/prometheus-community/windows_exporter/internal/collector/tcp"
 	"github.com/prometheus-community/windows_exporter/internal/collector/update"
 	"github.com/prometheus-community/windows_exporter/internal/mi"
@@ -155,6 +156,7 @@ func TestBuildRejectsUnknownSubCollector(t *testing.T) {
 		mssql.New(&mssql.Config{CollectorsEnabled: unknown}),
 		net.New(&net.Config{CollectorsEnabled: unknown}),
 		netframework.New(&netframework.Config{CollectorsEnabled: unknown}),
+		storage_spaces.New(&storage_spaces.Config{CollectorsEnabled: unknown}),
 		tcp.New(&tcp.Config{CollectorsEnabled: unknown}),
 	} {
 		t.Run(c.GetName(), func(t *testing.T) {

internal/collector/mscluster/mscluster.go

@@ -40,8 +40,6 @@ const (
 	subCollectorResource      = "resource"
 	subCollectorResourceGroup = "resourcegroup"
 	subCollectorSharedVolumes = "shared_volumes"
-	subCollectorVirtualDisk   = "virtualdisk"
-	subCollectorStoragePool   = "storagepool"
 )
 
 type Config struct {
@@ -57,8 +55,6 @@ var ConfigDefaults = Config{
 		subCollectorResource,
 		subCollectorResourceGroup,
 		subCollectorSharedVolumes,
-		subCollectorVirtualDisk,
-		subCollectorStoragePool,
 	},
 }
 
@@ -70,8 +66,6 @@ type Collector struct {
 	collectorResource
 	collectorResourceGroup
 	collectorSharedVolumes
-	collectorVirtualDisk
-	collectorStoragePool
 
 	config    Config
 	miSession *mi.Session
@@ -135,8 +129,6 @@ func (c *Collector) Build(_ *slog.Logger, miSession *mi.Session) error {
 		subCollectorResource,
 		subCollectorResourceGroup,
 		subCollectorSharedVolumes,
-		subCollectorVirtualDisk,
-		subCollectorStoragePool,
 	}
 
 	for _, name := range c.config.CollectorsEnabled {
@@ -191,18 +183,6 @@ func (c *Collector) Build(_ *slog.Logger, miSession *mi.Session) error {
 		}
 	}
 
-	if slices.Contains(c.config.CollectorsEnabled, subCollectorVirtualDisk) {
-		if err := c.buildVirtualDisk(); err != nil {
-			errs = append(errs, fmt.Errorf("failed to build virtualdisk collector: %w", err))
-		}
-	}
-
-	if slices.Contains(c.config.CollectorsEnabled, subCollectorStoragePool) {
-		if err := c.buildStoragePool(); err != nil {
-			errs = append(errs, fmt.Errorf("failed to build storagepool collector: %w", err))
-		}
-	}
-
 	return errors.Join(errs...)
 }
 
@@ -282,25 +262,5 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time.
 		})
 	}
 
-	if slices.Contains(c.config.CollectorsEnabled, subCollectorVirtualDisk) {
-		g.Go(func() error {
-			if err := c.collectVirtualDisk(ch, maxScrapeDuration); err != nil {
-				return fmt.Errorf("failed to collect virtualdisk metrics: %w", err)
-			}
-
-			return nil
-		})
-	}
-
-	if slices.Contains(c.config.CollectorsEnabled, subCollectorStoragePool) {
-		g.Go(func() error {
-			if err := c.collectStoragePool(ch, maxScrapeDuration); err != nil {
-				return fmt.Errorf("failed to collect storagepool metrics: %w", err)
-			}
-
-			return nil
-		})
-	}
-
 	return g.Wait()
 }

internal/collector/mscluster/mscluster_test.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/prometheus-community/windows_exporter/internal/collector/mscluster"
 	"github.com/prometheus-community/windows_exporter/internal/utils/testutils"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 func BenchmarkCollector(b *testing.B) {
@@ -30,7 +29,5 @@ func BenchmarkCollector(b *testing.B) {
 }
 
 func TestCollector(t *testing.T) {
-	metrics := testutils.TestCollector(t, mscluster.New, nil)
-	testutils.RequireFixtureMetric(t, metrics, mscluster.Name, "windows_mscluster_virtualdisk_size_bytes", prometheus.Labels{"name": "CIVirtualDisk"})
-	testutils.RequireFixtureMetric(t, metrics, mscluster.Name, "windows_mscluster_storagepool_size_bytes", prometheus.Labels{"name": "GitHubActions"})
+	testutils.TestCollector(t, mscluster.New, nil)
 }

internal/collector/storage_spaces/storage_spaces.go

@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: Apache-2.0
+//
+// Copyright The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package storage_spaces
+
+import (
+	"errors"
+	"fmt"
+	"log/slog"
+	"slices"
+	"strings"
+	"time"
+
+	"github.com/alecthomas/kingpin/v2"
+	"github.com/prometheus-community/windows_exporter/internal/mi"
+	"github.com/prometheus-community/windows_exporter/internal/utils/recovery"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+const (
+	Name = "storage_spaces"
+
+	subCollectorPool        = "pool"
+	subCollectorVirtualDisk = "virtual_disk"
+)
+
+type Config struct {
+	CollectorsEnabled []string `yaml:"enabled"`
+}
+
+//nolint:gochecknoglobals
+var ConfigDefaults = Config{
+	CollectorsEnabled: []string{
+		subCollectorPool,
+		subCollectorVirtualDisk,
+	},
+}
+
+// A Collector is a Prometheus Collector for the Storage Spaces WMI classes
+// MSFT_StoragePool and MSFT_VirtualDisk.
+type Collector struct {
+	collectorStoragePool
+	collectorVirtualDisk
+
+	config    Config
+	miSession *mi.Session
+}
+
+func New(config *Config) *Collector {
+	if config == nil {
+		config = &ConfigDefaults
+	}
+
+	if config.CollectorsEnabled == nil {
+		config.CollectorsEnabled = ConfigDefaults.CollectorsEnabled
+	}
+
+	c := &Collector{
+		config: *config,
+	}
+
+	return c
+}
+
+func NewWithFlags(app *kingpin.Application) *Collector {
+	c := &Collector{
+		config: ConfigDefaults,
+	}
+	c.config.CollectorsEnabled = make([]string, 0)
+
+	var collectorsEnabled string
+
+	app.Flag(
+		"collector.storage_spaces.enabled",
+		"Comma-separated list of collectors to use.",
+	).Default(strings.Join(ConfigDefaults.CollectorsEnabled, ",")).StringVar(&collectorsEnabled)
+
+	app.Action(func(*kingpin.ParseContext) error {
+		c.config.CollectorsEnabled = strings.Split(collectorsEnabled, ",")
+
+		return nil
+	})
+
+	return c
+}
+
+func (c *Collector) GetName() string {
+	return Name
+}
+
+func (c *Collector) Close() error {
+	return nil
+}
+
+func (c *Collector) Build(_ *slog.Logger, miSession *mi.Session) error {
+	if len(c.config.CollectorsEnabled) == 0 {
+		return nil
+	}
+
+	subCollectors := []string{
+		subCollectorPool,
+		subCollectorVirtualDisk,
+	}
+
+	for _, name := range c.config.CollectorsEnabled {
+		if !slices.Contains(subCollectors, name) {
+			return fmt.Errorf("unknown sub collector: %s. Possible values: %s", name,
+				strings.Join(subCollectors, ", "),
+			)
+		}
+	}
+
+	if miSession == nil {
+		return errors.New("miSession is nil")
+	}
+
+	c.miSession = miSession
+
+	errs := make([]error, 0)
+
+	if slices.Contains(c.config.CollectorsEnabled, subCollectorPool) {
+		if err := c.buildStoragePool(); err != nil {
+			errs = append(errs, fmt.Errorf("failed to build pool collector: %w", err))
+		}
+	}
+
+	if slices.Contains(c.config.CollectorsEnabled, subCollectorVirtualDisk) {
+		if err := c.buildVirtualDisk(); err != nil {
+			errs = append(errs, fmt.Errorf("failed to build virtual_disk collector: %w", err))
+		}
+	}
+
+	return errors.Join(errs...)
+}
+
+// Collect sends the metric values for each metric
+// to the provided prometheus Metric channel.
+func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time.Duration) error {
+	if len(c.config.CollectorsEnabled) == 0 {
+		return nil
+	}
+
+	var g recovery.Group
+
+	if slices.Contains(c.config.CollectorsEnabled, subCollectorPool) {
+		g.Go(func() error {
+			if err := c.collectStoragePool(ch, maxScrapeDuration); err != nil {
+				return fmt.Errorf("failed to collect pool metrics: %w", err)
+			}
+
+			return nil
+		})
+	}
+
+	if slices.Contains(c.config.CollectorsEnabled, subCollectorVirtualDisk) {
+		g.Go(func() error {
+			if err := c.collectVirtualDisk(ch, maxScrapeDuration); err != nil {
+				return fmt.Errorf("failed to collect virtual_disk metrics: %w", err)
+			}
+
+			return nil
+		})
+	}
+
+	return g.Wait()
+}

internal/collector/storage_spaces/storage_spaces_pool.go

@@ -15,7 +15,7 @@
 
 //go:build windows
 
-package mscluster
+package storage_spaces
 
 import (
 	"fmt"
@@ -27,7 +27,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-const nameStoragePool = Name + "_storagepool"
+const nameStoragePool = Name + "_pool"
 
 type collectorStoragePool struct {
 	storagePoolMIQuery mi.Query
@@ -92,14 +92,14 @@ func (c *Collector) buildStoragePool() error {
 
 	c.storagePoolOperationalStatus = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, nameStoragePool, "operational_status"),
-		"Operational status codes reported for the storage pool (one series per status value).",
+		"Operational status codes reported for the storage pool (one series per status value)",
 		[]string{"name", "unique_id", "status"},
 		nil,
 	)
 
 	c.storagePoolThinProvisioningAlertThresholds = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, nameStoragePool, "thin_provisioning_alert_thresholds"),
-		"Thin provisioning alert thresholds configured for the storage pool, in percent (one series per configured threshold).",
+		"Thin provisioning alert thresholds configured for the storage pool, in percent (one series per configured threshold)",
 		[]string{"name", "unique_id", "threshold"},
 		nil,
 	)

internal/collector/storage_spaces/storage_spaces_test.go

@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: Apache-2.0
+//
+// Copyright The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package storage_spaces_test
+
+import (
+	"testing"
+
+	"github.com/prometheus-community/windows_exporter/internal/collector/storage_spaces"
+	"github.com/prometheus-community/windows_exporter/internal/utils/testutils"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/require"
+)
+
+func BenchmarkCollector(b *testing.B) {
+	testutils.FuncBenchmarkCollector(b, storage_spaces.Name, storage_spaces.NewWithFlags)
+}
+
+func TestCollector(t *testing.T) {
+	metrics := testutils.TestCollector(t, storage_spaces.New, nil)
+
+	pool := prometheus.Labels{"name": "GitHubActions"}
+	virtualDisk := prometheus.Labels{"name": "CIVirtualDisk"}
+
+	testutils.RequireFixtureMetric(t, metrics, storage_spaces.Name, "windows_storage_spaces_pool_info", pool)
+	testutils.RequireFixtureMetric(t, metrics, storage_spaces.Name, "windows_storage_spaces_virtual_disk_info", virtualDisk)
+
+	for _, tc := range []struct {
+		metric string
+		labels prometheus.Labels
+	}{
+		{"windows_storage_spaces_pool_health_status", pool},
+		{"windows_storage_spaces_virtual_disk_health_status", virtualDisk},
+	} {
+		if metric := testutils.RequireFixtureMetric(t, metrics, storage_spaces.Name, tc.metric, tc.labels); metric != nil {
+			require.InDelta(t, 0, metric.GetGauge().GetValue(), 0, "%s: expected healthy", tc.metric)
+		}
+	}
+
+	// The fixture pool consists of two 10 GiB disks.
+	if metric := testutils.RequireFixtureMetric(t, metrics, storage_spaces.Name, "windows_storage_spaces_pool_size_bytes", pool); metric != nil {
+		require.Greater(t, metric.GetGauge().GetValue(), float64(10<<30))
+	}
+
+	// Storage Spaces rounds the requested 1 GiB up to whole slabs per column.
+	if metric := testutils.RequireFixtureMetric(t, metrics, storage_spaces.Name, "windows_storage_spaces_virtual_disk_size_bytes", virtualDisk); metric != nil {
+		require.GreaterOrEqual(t, metric.GetGauge().GetValue(), float64(1<<30))
+	}
+
+	if metric := testutils.RequireFixtureMetric(t, metrics, storage_spaces.Name, "windows_storage_spaces_virtual_disk_storage_efficiency_percent", virtualDisk); metric != nil {
+		value := metric.GetGauge().GetValue()
+		require.Greater(t, value, 0.0)
+		require.LessOrEqual(t, value, 100.0)
+	}
+}

internal/collector/storage_spaces/storage_spaces_virtual_disk.go

@@ -15,7 +15,7 @@
 
 //go:build windows
 
-package mscluster
+package storage_spaces
 
 import (
 	"fmt"
@@ -26,7 +26,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-const nameVirtualDisk = Name + "_virtualdisk"
+const nameVirtualDisk = Name + "_virtual_disk"
 
 type collectorVirtualDisk struct {
 	virtualDiskMIQuery mi.Query
@@ -62,7 +62,7 @@ func (c *Collector) buildVirtualDisk() error {
 
 	c.virtualDiskInfo = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, nameVirtualDisk, "info"),
-		"Virtual Disk information (value is always 1)",
+		"Virtual disk information (value is always 1)",
 		[]string{"name", "unique_id"},
 		nil,
 	)
@@ -83,7 +83,7 @@ func (c *Collector) buildVirtualDisk() error {
 
 	c.virtualDiskAllocatedSize = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, nameVirtualDisk, "allocated_size_bytes"),
-		"Allocated size of the virtual disk in bytes (data actually provisioned, excludes thin-provisioned unused capacity)",
+		"Allocated size of the virtual disk in bytes (capacity actually provisioned, excludes thin-provisioned unused capacity)",
 		[]string{"name", "unique_id"},
 		nil,
 	)
@@ -97,7 +97,7 @@ func (c *Collector) buildVirtualDisk() error {
 
 	c.virtualDiskStorageEfficiency = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, nameVirtualDisk, "storage_efficiency_percent"),
-		"Storage efficiency percentage (AllocatedSize / FootprintOnPool * 100)",
+		"Storage efficiency percentage (AllocatedSize / FootprintOnPool * 100), omitted while FootprintOnPool is 0",
 		[]string{"name", "unique_id"},
 		nil,
 	)
@@ -159,25 +159,21 @@ func (c *Collector) collectVirtualDisk(ch chan<- prometheus.Metric, maxScrapeDur
 			vdisk.UniqueId,
 		)
 
-		// Calculate storage efficiency (avoid division by zero).
 		// AllocatedSize (not Size) is the correct numerator: Size is the
 		// thin-provisioned ceiling, while AllocatedSize reflects the capacity
 		// actually provisioned. Using Size inflates the ratio on thin-provisioned
 		// disks (e.g. S2D FTT2 CSVs).
-		var storageEfficiency float64
+		// Without a footprint the ratio is undefined; skip it instead of
+		// publishing 0.
 		if vdisk.FootprintOnPool > 0 {
-			storageEfficiency = float64(vdisk.AllocatedSize) / float64(vdisk.FootprintOnPool) * 100
-		} else {
-			storageEfficiency = 0
+			ch <- prometheus.MustNewConstMetric(
+				c.virtualDiskStorageEfficiency,
+				prometheus.GaugeValue,
+				float64(vdisk.AllocatedSize)/float64(vdisk.FootprintOnPool)*100,
+				vdisk.FriendlyName,
+				vdisk.UniqueId,
+			)
 		}
-
-		ch <- prometheus.MustNewConstMetric(
-			c.virtualDiskStorageEfficiency,
-			prometheus.GaugeValue,
-			storageEfficiency,
-			vdisk.FriendlyName,
-			vdisk.UniqueId,
-		)
 	}
 
 	return nil