From f12d4624c9d2cfee135284c9618fc4561fb844ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Otto=20Kr=C3=B6pke?= Date: Mon, 12 Oct 2026 15:54:25 +0200 Subject: [PATCH] wmi: reuse label pairs between scrapes (#2699) Co-authored-by: Claude Signed-off-by: Claude --- internal/collector/wmi/wmi.go | 21 +++++++-- internal/collector/wmi/wmi_output_test.go | 53 +++++++++++++++-------- internal/collector/wmi/wmi_test.go | 12 +++++ 3 files changed, 65 insertions(+), 21 deletions(-) diff --git a/internal/collector/wmi/wmi.go b/internal/collector/wmi/wmi.go index 0d97069a..82bb6215 100644 --- a/internal/collector/wmi/wmi.go +++ b/internal/collector/wmi/wmi.go @@ -29,6 +29,7 @@ import ( "time" "github.com/alecthomas/kingpin/v2" + "github.com/prometheus-community/windows_exporter/internal/labelcache" "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/types" "github.com/prometheus/client_golang/prometheus" @@ -73,6 +74,10 @@ type Collector struct { queries []Query + // labels keeps the label pairs of the returned instances between scrapes, + // so that a query returning many instances doesn't rebuild them each time. + labels labelcache.Cache + querySuccessDesc *prometheus.Desc queryDurationDesc *prometheus.Desc } @@ -140,6 +145,8 @@ func (c *Collector) GetName() string { } func (c *Collector) Close() error { + c.labels.Reset() + return nil } @@ -452,11 +459,14 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time. deadline = time.Now().Add(maxScrapeDuration) } + scrape := c.labels.Begin() + defer scrape.Commit() + var errs []error for _, query := range c.queries { startTime := time.Now() - err := c.collectQuery(ch, query, deadline) + err := c.collectQuery(ch, scrape, query, deadline) duration := time.Since(startTime) success := 1.0 @@ -478,7 +488,10 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time. slog.Any("err", err), ) } else { - c.logger.Debug(fmt.Sprintf("wmi query %s succeeded after %s", query.Name, duration)) + c.logger.Debug("wmi query succeeded", + slog.String("query", query.Name), + slog.Duration("duration", duration), + ) } ch <- prometheus.MustNewConstMetric( @@ -499,7 +512,7 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time. return errors.Join(errs...) } -func (c *Collector) collectQuery(ch chan<- prometheus.Metric, query Query, deadline time.Time) error { +func (c *Collector) collectQuery(ch chan<- prometheus.Metric, scrape *labelcache.Scrape, query Query, deadline time.Time) error { // Zero means no timeout, which leaves the MI default in place. var timeout time.Duration @@ -569,7 +582,7 @@ func (c *Collector) collectQuery(ch chan<- prometheus.Metric, query Query, deadl continue } - ch <- prometheus.MustNewConstMetric( + ch <- scrape.MustNewConstMetric( property.desc, property.metricType, value, diff --git a/internal/collector/wmi/wmi_output_test.go b/internal/collector/wmi/wmi_output_test.go index 2dfeaaab..3c48dfe3 100644 --- a/internal/collector/wmi/wmi_output_test.go +++ b/internal/collector/wmi/wmi_output_test.go @@ -65,15 +65,27 @@ func scrape(t *testing.T, c *wmi.Collector) string { func scrapeWithTimeout(t *testing.T, c *wmi.Collector, maxScrapeDuration time.Duration) string { t.Helper() + return newScraper(t, c, maxScrapeDuration)() +} + +// newScraper builds c against a live MI session and returns a function that +// scrapes it, so that a test can scrape the same collector more than once. +func newScraper(t *testing.T, c *wmi.Collector, maxScrapeDuration time.Duration) func() string { + t.Helper() + require.NoError(t, c.Build(slog.New(slog.DiscardHandler), newSession(t))) reg := prometheus.NewRegistry() reg.MustRegister(collectorAdapter{c, maxScrapeDuration}) - rw := httptest.NewRecorder() - promhttp.HandlerFor(reg, promhttp.HandlerOpts{ErrorHandling: promhttp.ContinueOnError}).ServeHTTP(rw, &http.Request{}) + handler := promhttp.HandlerFor(reg, promhttp.HandlerOpts{ErrorHandling: promhttp.ContinueOnError}) - return rw.Body.String() + return func() string { + rw := httptest.NewRecorder() + handler.ServeHTTP(rw, &http.Request{}) + + return rw.Body.String() + } } // collect builds c against a live MI session and returns the number of emitted @@ -102,10 +114,13 @@ func collect(t *testing.T, c *wmi.Collector) (int, error) { // constant labels, boolean conversion, a metric name shared by two queries, // and the query_success semantics for an unknown class, a non-numeric // property next to a numeric one, and a label property of unsupported type. +// +// The collector is scraped twice: the second scrape reuses the label pairs +// cached by the first and must produce the same output. func TestCollectorMetrics(t *testing.T) { t.Parallel() - got := scrape(t, wmi.New(&wmi.Config{ + next := newScraper(t, wmi.New(&wmi.Config{ Queries: []wmi.Query{ { Name: "kernel", @@ -172,16 +187,7 @@ func TestCollectorMetrics(t *testing.T) { Properties: []wmi.Property{{Name: "Primary"}}, }, }, - })) - - // Durations vary between runs; their presence is asserted separately. - var lines []string - - for line := range strings.SplitSeq(got, "\n") { - if !strings.Contains(line, "windows_wmi_query_duration_seconds") { - lines = append(lines, line) - } - } + }), 5*time.Second) expected := `# HELP windows_wmi_kernel_processid windows_exporter: custom WMI metric # TYPE windows_wmi_kernel_processid gauge @@ -212,10 +218,23 @@ windows_wmi_test_shared{query="a"} 1 windows_wmi_test_shared{query="b"} 1 ` - require.Equal(t, expected, strings.Join(lines, "\n")) + for range 2 { + got := next() - for _, name := range []string{"kernel", "missing_class", "os", "partial", "shared_a", "shared_b", "unsupported_label"} { - require.Regexp(t, `(?m)^windows_wmi_query_duration_seconds\{name="`+name+`"\} \d`, got) + // Durations vary between runs; their presence is asserted separately. + var lines []string + + for line := range strings.SplitSeq(got, "\n") { + if !strings.Contains(line, "windows_wmi_query_duration_seconds") { + lines = append(lines, line) + } + } + + require.Equal(t, expected, strings.Join(lines, "\n")) + + for _, name := range []string{"kernel", "missing_class", "os", "partial", "shared_a", "shared_b", "unsupported_label"} { + require.Regexp(t, `(?m)^windows_wmi_query_duration_seconds\{name="`+name+`"\} \d`, got) + } } } diff --git a/internal/collector/wmi/wmi_test.go b/internal/collector/wmi/wmi_test.go index 54b6591c..a2ebaf7f 100644 --- a/internal/collector/wmi/wmi_test.go +++ b/internal/collector/wmi/wmi_test.go @@ -38,6 +38,18 @@ func BenchmarkCollector(b *testing.B) { }) } +// BenchmarkCollectorProcess runs a query returning one instance per process, +// so that the cost of many series per scrape shows. +func BenchmarkCollectorProcess(b *testing.B) { + queries := `[{"name":"process","class":"Win32_Process","label_properties":[{"name":"ProcessId"},{"name":"Name"}],"properties":[{"name":"WorkingSetSize"},{"name":"ThreadCount"},{"name":"HandleCount"}]}]` + + testutils.FuncBenchmarkCollector(b, wmi.Name, wmi.NewWithFlags, func(app *kingpin.Application) { + // The queries are only read by the application action, which runs on parse. + _, err := app.Parse([]string{"--collector.wmi.queries", queries}) + require.NoError(b, err) + }) +} + func TestCollector(t *testing.T) { t.Parallel()