time: refactor collector (#1728)

docs/collector.time.md

@@ -5,15 +5,19 @@ If the Windows Time Service is stopped after collection has started, collector m
 
 Please note the Time Service perflib counters are only available on [Windows Server 2016 or newer](https://docs.microsoft.com/en-us/windows-server/networking/windows-time-service/windows-server-2016-improvements).
 
-|                     |         |
+|                     |        |
-|---------------------|---------|
+|---------------------|--------|
-| Metric name prefix  | `time`  |
+| Metric name prefix  | `time` |
-| Data source         | Perflib |
+| Data source         | PDH    |
-| Enabled by default? | No      |
+| Enabled by default? | No     |
 
 ## Flags
 
-None
+### `--collectors.time.enabled`
+Comma-separated list of collectors to use, for example: `--collectors.time.enabled=ntp,system_time`.
+Matching is case-sensitive.
+
+
 
 ## Metrics
 

internal/collector/service/service_test.go

@@ -10,3 +10,7 @@ import (
 func BenchmarkCollector(b *testing.B) {
 	testutils.FuncBenchmarkCollector(b, service.Name, service.NewWithFlags)
 }
+
+func TestCollector(t *testing.T) {
+	testutils.TestCollector(t, service.New, nil)
+}

internal/collector/time/const.go

@@ -0,0 +1,10 @@
+package time
+
+const (
+	ClockFrequencyAdjustmentPPBTotal = "Clock Frequency Adjustment (ppb)"
+	ComputedTimeOffset               = "Computed Time Offset"
+	NTPClientTimeSourceCount         = "NTP Client Time Source Count"
+	NTPRoundTripDelay                = "NTP Roundtrip Delay"
+	NTPServerIncomingRequestsTotal   = "NTP Server Incoming Requests"
+	NTPServerOutgoingResponsesTotal  = "NTP Server Outgoing Responses"
+)

internal/collector/time/time.go

@@ -4,28 +4,46 @@ package time
 
 import (
 	"errors"
+	"fmt"
 	"log/slog"
+	"slices"
+	"strings"
 	"time"
 
 	"github.com/alecthomas/kingpin/v2"
 	"github.com/prometheus-community/windows_exporter/internal/headers/kernel32"
 	"github.com/prometheus-community/windows_exporter/internal/mi"
-	v1 "github.com/prometheus-community/windows_exporter/internal/perfdata/v1"
+	"github.com/prometheus-community/windows_exporter/internal/perfdata"
+	"github.com/prometheus-community/windows_exporter/internal/perfdata/perftypes"
 	"github.com/prometheus-community/windows_exporter/internal/types"
 	"github.com/prometheus/client_golang/prometheus"
 	"golang.org/x/sys/windows"
 )
 
-const Name = "time"
+const (
+	Name = "time"
 
-type Config struct{}
+	collectorSystemTime = "system_time"
+	collectorNTP        = "ntp"
+)
 
-var ConfigDefaults = Config{}
+type Config struct {
+	CollectorsEnabled []string `yaml:"collectors_enabled"`
+}
+
+var ConfigDefaults = Config{
+	CollectorsEnabled: []string{
+		collectorSystemTime,
+		collectorNTP,
+	},
+}
 
 // Collector is a Prometheus Collector for Perflib counter metrics.
 type Collector struct {
 	config Config
 
+	perfDataCollector perfdata.Collector
+
 	currentTime                      *prometheus.Desc
 	timezone                         *prometheus.Desc
 	clockFrequencyAdjustmentPPBTotal *prometheus.Desc
@@ -41,6 +59,10 @@ func New(config *Config) *Collector {
 		config = &ConfigDefaults
 	}
 
+	if config.CollectorsEnabled == nil {
+		config.CollectorsEnabled = ConfigDefaults.CollectorsEnabled
+	}
+
 	c := &Collector{
 		config: *config,
 	}
@@ -48,8 +70,26 @@ func New(config *Config) *Collector {
 	return c
 }
 
-func NewWithFlags(_ *kingpin.Application) *Collector {
+func NewWithFlags(app *kingpin.Application) *Collector {
-	return &Collector{}
+	c := &Collector{
+		config: ConfigDefaults,
+	}
+	c.config.CollectorsEnabled = make([]string, 0)
+
+	var collectorsEnabled string
+
+	app.Flag(
+		"collector.time.enabled",
+		"Comma-separated list of collectors to use. Defaults to all, if not specified. ntp may not available on all systems.",
+	).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 {
@@ -57,14 +97,40 @@ func (c *Collector) GetName() string {
 }
 
 func (c *Collector) GetPerfCounter(_ *slog.Logger) ([]string, error) {
-	return []string{"Windows Time Service"}, nil
+	return []string{}, nil
 }
 
 func (c *Collector) Close(_ *slog.Logger) error {
+	if slices.Contains(c.config.CollectorsEnabled, collectorNTP) {
+		c.perfDataCollector.Close()
+	}
+
 	return nil
 }
 
 func (c *Collector) Build(_ *slog.Logger, _ *mi.Session) error {
+	for _, collector := range c.config.CollectorsEnabled {
+		if !slices.Contains([]string{collectorSystemTime, collectorNTP}, collector) {
+			return fmt.Errorf("unknown collector: %s", collector)
+		}
+	}
+
+	counters := []string{
+		ClockFrequencyAdjustmentPPBTotal,
+		ComputedTimeOffset,
+		NTPClientTimeSourceCount,
+		NTPRoundTripDelay,
+		NTPServerIncomingRequestsTotal,
+		NTPServerOutgoingResponsesTotal,
+	}
+
+	var err error
+
+	c.perfDataCollector, err = perfdata.NewCollector(perfdata.V2, "Windows Time Service", nil, counters)
+	if err != nil {
+		return fmt.Errorf("failed to create Windows Time Service collector: %w", err)
+	}
+
 	c.currentTime = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, Name, "current_timestamp_seconds"),
 		"OperatingSystem.LocalDateTime",
@@ -119,40 +185,24 @@ func (c *Collector) Build(_ *slog.Logger, _ *mi.Session) error {
 
 // Collect sends the metric values for each metric
 // to the provided prometheus Metric channel.
-func (c *Collector) Collect(ctx *types.ScrapeContext, logger *slog.Logger, ch chan<- prometheus.Metric) error {
+func (c *Collector) Collect(_ *types.ScrapeContext, _ *slog.Logger, ch chan<- prometheus.Metric) error {
-	logger = logger.With(slog.String("collector", Name))
-
 	errs := make([]error, 0, 2)
 
-	if err := c.collectTime(ch); err != nil {
+	if slices.Contains(c.config.CollectorsEnabled, collectorSystemTime) {
-		logger.Error("failed collecting time metrics",
+		if err := c.collectTime(ch); err != nil {
-			slog.Any("err", err),
+			errs = append(errs, fmt.Errorf("failed collecting time metrics: %w", err))
-		)
+		}
-
-		errs = append(errs, err)
 	}
 
-	if err := c.collectNTP(ctx, logger, ch); err != nil {
+	if slices.Contains(c.config.CollectorsEnabled, collectorNTP) {
-		logger.Error("failed collecting time ntp metrics",
+		if err := c.collectNTP(ch); err != nil {
-			slog.Any("err", err),
+			errs = append(errs, fmt.Errorf("failed collecting time ntp metrics: %w", err))
-		)
+		}
-
-		errs = append(errs, err)
 	}
 
 	return errors.Join(errs...)
 }
 
-// Perflib "Windows Time Service".
-type windowsTime struct {
-	ClockFrequencyAdjustmentPPBTotal float64 `perflib:"Clock Frequency Adjustment (PPB)"`
-	ComputedTimeOffset               float64 `perflib:"Computed Time Offset"`
-	NTPClientTimeSourceCount         float64 `perflib:"NTP Client Time Source Count"`
-	NTPRoundTripDelay                float64 `perflib:"NTP Roundtrip Delay"`
-	NTPServerIncomingRequestsTotal   float64 `perflib:"NTP Server Incoming Requests"`
-	NTPServerOutgoingResponsesTotal  float64 `perflib:"NTP Server Outgoing Responses"`
-}
-
 func (c *Collector) collectTime(ch chan<- prometheus.Metric) error {
 	ch <- prometheus.MustNewConstMetric(
 		c.currentTime,
@@ -178,48 +228,46 @@ func (c *Collector) collectTime(ch chan<- prometheus.Metric) error {
 	return nil
 }
 
-func (c *Collector) collectNTP(ctx *types.ScrapeContext, logger *slog.Logger, ch chan<- prometheus.Metric) error {
+func (c *Collector) collectNTP(ch chan<- prometheus.Metric) error {
-	logger = logger.With(slog.String("collector", Name))
+	perfData, err := c.perfDataCollector.Collect()
-
+	if err != nil {
-	var dst []windowsTime // Single-instance class, array is required but will have single entry.
+		return fmt.Errorf("failed to collect VM Memory metrics: %w", err)
-
-	if err := v1.UnmarshalObject(ctx.PerfObjects["Windows Time Service"], &dst, logger); err != nil {
-		return err
 	}
 
-	if len(dst) == 0 {
+	data, ok := perfData[perftypes.EmptyInstance]
-		return errors.New("no data returned for Windows Time Service")
+	if !ok {
+		return errors.New("query for Windows Time Service returned empty result set")
 	}
 
 	ch <- prometheus.MustNewConstMetric(
 		c.clockFrequencyAdjustmentPPBTotal,
 		prometheus.CounterValue,
-		dst[0].ClockFrequencyAdjustmentPPBTotal,
+		data[ClockFrequencyAdjustmentPPBTotal].FirstValue,
 	)
 	ch <- prometheus.MustNewConstMetric(
 		c.computedTimeOffset,
 		prometheus.GaugeValue,
-		dst[0].ComputedTimeOffset/1000000, // microseconds -> seconds
+		data[ComputedTimeOffset].FirstValue/1000000, // microseconds -> seconds
 	)
 	ch <- prometheus.MustNewConstMetric(
 		c.ntpClientTimeSourceCount,
 		prometheus.GaugeValue,
-		dst[0].NTPClientTimeSourceCount,
+		data[NTPClientTimeSourceCount].FirstValue,
 	)
 	ch <- prometheus.MustNewConstMetric(
 		c.ntpRoundTripDelay,
 		prometheus.GaugeValue,
-		dst[0].NTPRoundTripDelay/1000000, // microseconds -> seconds
+		data[NTPRoundTripDelay].FirstValue/1000000, // microseconds -> seconds
 	)
 	ch <- prometheus.MustNewConstMetric(
 		c.ntpServerIncomingRequestsTotal,
 		prometheus.CounterValue,
-		dst[0].NTPServerIncomingRequestsTotal,
+		data[NTPServerIncomingRequestsTotal].FirstValue,
 	)
 	ch <- prometheus.MustNewConstMetric(
 		c.ntpServerOutgoingResponsesTotal,
 		prometheus.CounterValue,
-		dst[0].NTPServerOutgoingResponsesTotal,
+		data[NTPServerOutgoingResponsesTotal].FirstValue,
 	)
 
 	return nil

internal/collector/time/time_test.go

@@ -10,3 +10,7 @@ import (
 func BenchmarkCollector(b *testing.B) {
 	testutils.FuncBenchmarkCollector(b, time.Name, time.NewWithFlags)
 }
+
+func TestCollector(t *testing.T) {
+	testutils.TestCollector(t, time.New, nil)
+}

internal/collector/vmware/const.go

@@ -9,16 +9,16 @@ const (
 	cpuStolenMs              = "CPU stolen time"             // \VM Processor(*)\CPU stolen time
 	cpuTimePercents          = "% Processor Time"            // \VM Processor(*)\% Processor Time
 
-	MemActiveMB      = "MemActiveMB"      // \VM Memory\Memory Active in MB
+	memActiveMB      = "MemActiveMB"      // \VM Memory\Memory Active in MB
-	MemBalloonedMB   = "MemBalloonedMB"   // \VM Memory\Memory Ballooned in MB
+	memBalloonedMB   = "MemBalloonedMB"   // \VM Memory\Memory Ballooned in MB
-	MemLimitMB       = "MemLimitMB"       // \VM Memory\Memory Limit in MB
+	memLimitMB       = "MemLimitMB"       // \VM Memory\Memory Limit in MB
-	MemMappedMB      = "MemMappedMB"      // \VM Memory\Memory Mapped in MB
+	memMappedMB      = "MemMappedMB"      // \VM Memory\Memory Mapped in MB
-	MemOverheadMB    = "MemOverheadMB"    // \VM Memory\Memory Overhead in MB
+	memOverheadMB    = "MemOverheadMB"    // \VM Memory\Memory Overhead in MB
-	MemReservationMB = "MemReservationMB" // \VM Memory\Memory Reservation in MB
+	memReservationMB = "MemReservationMB" // \VM Memory\Memory Reservation in MB
-	MemSharedMB      = "MemSharedMB"      // \VM Memory\Memory Shared in MB
+	memSharedMB      = "MemSharedMB"      // \VM Memory\Memory Shared in MB
-	MemSharedSavedMB = "MemSharedSavedMB" // \VM Memory\Memory Shared Saved in MB
+	memSharedSavedMB = "MemSharedSavedMB" // \VM Memory\Memory Shared Saved in MB
-	MemShares        = "MemShares"        // \VM Memory\Memory Shares
+	memShares        = "MemShares"        // \VM Memory\Memory Shares
-	MemSwappedMB     = "MemSwappedMB"     // \VM Memory\Memory Swapped in MB
+	memSwappedMB     = "MemSwappedMB"     // \VM Memory\Memory Swapped in MB
-	MemTargetSizeMB  = "MemTargetSizeMB"  // \VM Memory\Memory Target Size
+	memTargetSizeMB  = "MemTargetSizeMB"  // \VM Memory\Memory Target Size
-	MemUsedMB        = "MemUsedMB"        // \VM Memory\Memory Used in MB
+	memUsedMB        = "MemUsedMB"        // \VM Memory\Memory Used in MB
 )

internal/collector/vmware/vmware.go

@@ -143,18 +143,18 @@ func (c *Collector) Build(_ *slog.Logger, _ *mi.Session) error {
 	)
 
 	counters = []string{
-		MemActiveMB,
+		memActiveMB,
-		MemBalloonedMB,
+		memBalloonedMB,
-		MemLimitMB,
+		memLimitMB,
-		MemMappedMB,
+		memMappedMB,
-		MemOverheadMB,
+		memOverheadMB,
-		MemReservationMB,
+		memReservationMB,
-		MemSharedMB,
+		memSharedMB,
-		MemSharedSavedMB,
+		memSharedSavedMB,
-		MemShares,
+		memShares,
-		MemSwappedMB,
+		memSwappedMB,
-		MemTargetSizeMB,
+		memTargetSizeMB,
-		MemUsedMB,
+		memUsedMB,
 	}
 
 	c.perfDataCollectorMemory, err = perfdata.NewCollector(perfdata.V2, "VM Memory", nil, counters)
@@ -268,73 +268,73 @@ func (c *Collector) collectMem(ch chan<- prometheus.Metric) error {
 	ch <- prometheus.MustNewConstMetric(
 		c.memActive,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemActiveMB].FirstValue),
+		utils.MBToBytes(data[memActiveMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memBallooned,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemBalloonedMB].FirstValue),
+		utils.MBToBytes(data[memBalloonedMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memLimit,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemLimitMB].FirstValue),
+		utils.MBToBytes(data[memLimitMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memMapped,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemMappedMB].FirstValue),
+		utils.MBToBytes(data[memMappedMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memOverhead,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemOverheadMB].FirstValue),
+		utils.MBToBytes(data[memOverheadMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memReservation,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemReservationMB].FirstValue),
+		utils.MBToBytes(data[memReservationMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memShared,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemSharedMB].FirstValue),
+		utils.MBToBytes(data[memSharedMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memSharedSaved,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemSharedSavedMB].FirstValue),
+		utils.MBToBytes(data[memSharedSavedMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memShares,
 		prometheus.GaugeValue,
-		data[MemShares].FirstValue,
+		data[memShares].FirstValue,
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memSwapped,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemSwappedMB].FirstValue),
+		utils.MBToBytes(data[memSwappedMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memTargetSize,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemTargetSizeMB].FirstValue),
+		utils.MBToBytes(data[memTargetSizeMB].FirstValue),
 	)
 
 	ch <- prometheus.MustNewConstMetric(
 		c.memUsed,
 		prometheus.GaugeValue,
-		utils.MBToBytes(data[MemUsedMB].FirstValue),
+		utils.MBToBytes(data[memUsedMB].FirstValue),
 	)
 
 	return nil