diff --git a/internal/collector/printer/printer.go b/internal/collector/printer/printer.go index eb295983..9bebfdcb 100644 --- a/internal/collector/printer/printer.go +++ b/internal/collector/printer/printer.go @@ -184,23 +184,38 @@ type wmiPrintJob struct { Status string `mi:"Status"` } +// minQueryTimeout keeps the job query bounded once the scrape budget is spent. +// MI counts timeouts in whole milliseconds, and a zero timeout means no timeout at all. +const minQueryTimeout = time.Millisecond + func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time.Duration) error { var errs []error - if err := c.collectPrinterStatus(ch); err != nil { + deadline := time.Now().Add(maxScrapeDuration) + + // Win32_Printer calls DeviceCapabilities for every printer, whichever properties are selected. + // For an IPP printer that is offline, this blocks until the network requests time out, + // which can take close to a minute. Bound the query by the scrape timeout, so that a hung call + // fails with the native MI timeout instead of blocking the collector for later scrapes. + if err := c.collectPrinterStatus(ch, maxScrapeDuration); err != nil { errs = append(errs, fmt.Errorf("failed to collect printer status metrics: %w", err)) } - if err := c.collectPrinterJobStatus(ch, maxScrapeDuration); err != nil { + jobQueryTimeout := maxScrapeDuration + if maxScrapeDuration > 0 { + jobQueryTimeout = max(time.Until(deadline), minQueryTimeout) + } + + if err := c.collectPrinterJobStatus(ch, jobQueryTimeout); err != nil { errs = append(errs, fmt.Errorf("failed to collect printer job status metrics: %w", err)) } return errors.Join(errs...) } -func (c *Collector) collectPrinterStatus(ch chan<- prometheus.Metric) error { +func (c *Collector) collectPrinterStatus(ch chan<- prometheus.Metric, maxScrapeDuration time.Duration) error { var printers []wmiPrinter - if err := c.miSession.Query(&printers, mi.NamespaceRootCIMv2, c.miQueryPrinter, 0); err != nil { + if err := c.miSession.Query(&printers, mi.NamespaceRootCIMv2, c.miQueryPrinter, maxScrapeDuration); err != nil { return fmt.Errorf("WMI query failed: %w", err) } diff --git a/internal/collector/printer/printer_test.go b/internal/collector/printer/printer_test.go index 85d4ff14..45b0ea20 100644 --- a/internal/collector/printer/printer_test.go +++ b/internal/collector/printer/printer_test.go @@ -18,12 +18,17 @@ package printer_test import ( + "log/slog" "testing" + "time" "github.com/alecthomas/kingpin/v2" "github.com/prometheus-community/windows_exporter/internal/collector/printer" + "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/utils/testutils" "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func BenchmarkCollector(b *testing.B) { @@ -39,3 +44,29 @@ func TestCollector(t *testing.T) { metrics := testutils.TestCollector(t, printer.New, nil) testutils.RequireFixtureMetric(t, metrics, printer.Name, "windows_printer_job_count", prometheus.Labels{"printer": "CIPrinter"}) } + +// TestCollectorScrapeTimeout checks that the Win32_Printer query is bounded by the scrape timeout. +// Win32_Printer can block for close to a minute on an offline IPP printer; without a timeout, +// that call keeps the collector busy across scrapes. +func TestCollectorScrapeTimeout(t *testing.T) { + miApp, err := mi.ApplicationInitialize() + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, miApp.Close()) }) + + miSession, err := miApp.NewSession(nil) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, miSession.Close()) }) + + c := printer.New(nil) + + t.Cleanup(func() { assert.NoError(t, c.Close()) }) + + require.NoError(t, c.Build(slog.New(slog.DiscardHandler), miSession)) + + ch := make(chan prometheus.Metric, 1000) + + // 1ms is the smallest timeout MI enforces, and well below the time Win32_Printer needs. + err = c.Collect(ch, time.Millisecond) + require.ErrorIs(t, err, mi.MI_RESULT_INVALID_OPERATION_TIMEOUT) + require.ErrorContains(t, err, "failed to collect printer status metrics") +}