diff --git a/internal/collector/registry/registry.go b/internal/collector/registry/registry.go index 85cd8e40..2d53eb97 100644 --- a/internal/collector/registry/registry.go +++ b/internal/collector/registry/registry.go @@ -337,7 +337,13 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error func (c *Collector) collectKey(ch chan<- prometheus.Metric, key Key) error { rk, err := winregistry.OpenKey(key.hive, key.subPath, winregistry.QUERY_VALUE) if err != nil { - return fmt.Errorf("failed to open registry key %s: %w", key.label, errors.Join(err, types.ErrNoData)) + // Child failures are reported by key_success and classified as no-data warnings here. + return fmt.Errorf( + "failed to open registry key %s: %s: %w", + key.label, + err.Error(), + types.ErrNoData, + ) } defer func() { @@ -349,7 +355,13 @@ func (c *Collector) collectKey(ch chan<- prometheus.Metric, key Key) error { for _, value := range key.Values { val, _, err := rk.GetIntegerValue(value.Name) if err != nil { - errs = append(errs, fmt.Errorf("failed to read value %q of registry key %s: %w", value.Name, key.label, errors.Join(err, types.ErrNoData))) + errs = append(errs, fmt.Errorf( + "failed to read value %q of registry key %s: %s: %w", + value.Name, + key.label, + err.Error(), + types.ErrNoData, + )) continue } diff --git a/internal/collector/wmi/wmi.go b/internal/collector/wmi/wmi.go index e584a630..373f0adc 100644 --- a/internal/collector/wmi/wmi.go +++ b/internal/collector/wmi/wmi.go @@ -451,7 +451,12 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration time. if err != nil { // A failing query is reported via query_success. Like the registry // collector, it does not fail the whole collector. - errs = append(errs, fmt.Errorf("failed to collect query %s: %w", query.Name, errors.Join(err, types.ErrNoData))) + errs = append(errs, fmt.Errorf( + "failed to collect query %s: %s: %w", + query.Name, + err.Error(), + types.ErrNoData, + )) success = 0.0 c.logger.Warn("wmi query failed", diff --git a/pkg/collector/collect.go b/pkg/collector/collect.go index efa2acf6..39a00481 100644 --- a/pkg/collector/collect.go +++ b/pkg/collector/collect.go @@ -258,7 +258,7 @@ func logCollectorResult(ctx context.Context, logger *slog.Logger, name string, e result := "succeeded" if err != nil { - if !errors.Is(err, pdh.ErrNoData) && !errors.Is(err, types.ErrNoData) && !errors.Is(err, windows.EPT_S_NOT_REGISTERED) { + if !expectedCollectionError(err) { if errors.Is(err, pdh.ErrPerformanceCounterNotInitialized) { err = fmt.Errorf("%w. Check application logs from initialization pharse for more information", err) } @@ -284,3 +284,25 @@ func logCollectorResult(ctx context.Context, logger *slog.Logger, name string, e return success } + +// expectedCollectionError checks every cause so an expected error cannot hide an independent failure. +func expectedCollectionError(err error) bool { + if joined, ok := err.(interface{ Unwrap() []error }); ok { + causes := joined.Unwrap() + for _, cause := range causes { + if !expectedCollectionError(cause) { + return false + } + } + + return len(causes) != 0 + } + + if wrapped, ok := err.(interface{ Unwrap() error }); ok { + return expectedCollectionError(wrapped.Unwrap()) + } + + noData := errors.Is(err, pdh.ErrNoData) || errors.Is(err, types.ErrNoData) + + return noData || errors.Is(err, windows.EPT_S_NOT_REGISTERED) +} diff --git a/pkg/collector/collect_test.go b/pkg/collector/collect_test.go index 2ace88a0..50ad3e60 100644 --- a/pkg/collector/collect_test.go +++ b/pkg/collector/collect_test.go @@ -20,6 +20,7 @@ package collector_test import ( "context" "errors" + "fmt" "log/slog" "runtime" "strings" @@ -31,14 +32,100 @@ import ( "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/pdh" + "github.com/prometheus-community/windows_exporter/internal/types" "github.com/prometheus-community/windows_exporter/pkg/collector" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/expfmt" "github.com/stretchr/testify/require" + "golang.org/x/sys/windows" ) // The tests in this file share the package-level scrape slot, so they don't run in parallel. +func TestCollectErrorStatus(t *testing.T) { + failure := errors.New("collector failed") + + for _, tc := range []struct { + name string + err error + success string + }{ + {name: "success", success: "1"}, + {name: "failure", err: failure, success: "0"}, + {name: "no data", err: types.ErrNoData, success: "1"}, + {name: "PDH no data", err: pdh.ErrNoData, success: "1"}, + {name: "RPC endpoint absent", err: windows.EPT_S_NOT_REGISTERED, success: "1"}, + {name: "unexpected no data", err: types.ErrNoDataUnexpected, success: "0"}, + { + name: "wrapped no data", + err: fmt.Errorf("empty instances: %w", pdh.ErrNoData), + success: "1", + }, + { + name: "joined expected errors", + err: errors.Join(pdh.ErrNoData, types.ErrNoData, windows.EPT_S_NOT_REGISTERED), + success: "1", + }, + { + name: "nested expected errors", + err: fmt.Errorf( + "groups: %w", + errors.Join(pdh.ErrNoData, errors.Join(types.ErrNoData, windows.EPT_S_NOT_REGISTERED)), + ), + success: "1", + }, + { + name: "classified child warning", + err: fmt.Errorf("child failed: %s: %w", failure.Error(), types.ErrNoData), + success: "1", + }, + { + name: "failure joined with no data", + err: errors.Join(types.ErrNoData, failure), + success: "0", + }, + { + name: "failure joined with PDH no data", + err: errors.Join(pdh.ErrNoData, failure), + success: "0", + }, + { + name: "failure joined with absent RPC endpoint", + err: errors.Join(windows.EPT_S_NOT_REGISTERED, failure), + success: "0", + }, + { + name: "nested mixed errors", + err: fmt.Errorf("groups: %w", errors.Join(pdh.ErrNoData, errors.Join(types.ErrNoData, failure))), + success: "0", + }, + { + name: "multiple wrapped errors", + err: fmt.Errorf("first: %w; second: %w", pdh.ErrNoData, failure), + success: "0", + }, + } { + t.Run(tc.name, func(t *testing.T) { + logs := &recordHandler{} + collection := collector.New(collector.Map{"test": &fakeCollector{name: "test", collectErr: tc.err}}) + handler, err := collection.NewHandler(time.Second, slog.New(logs), nil) + require.NoError(t, err) + + out := scrape(t, handler) + require.Contains(t, out, `windows_exporter_collector_success{collector="test"} `+tc.success) + require.Contains(t, out, `windows_exporter_collector_timeout{collector="test"} 0`) + require.Contains(t, out, "windows_test_test 42", "partial metrics must still be exported") + + warnings := 0 + if tc.success == "0" { + warnings = 1 + } + + require.Equal(t, warnings, logs.warnings("failed")) + }) + } +} + func TestCollectTimedOutCollectorDoesNotOverlap(t *testing.T) { // The collector's delay and the scrape deadlines use synthetic time. synctest.Test(t, func(t *testing.T) { @@ -379,6 +466,7 @@ func TestBuildCloseCyclesDoNotLeakGoroutines(t *testing.T) { type fakeCollector struct { name string buildErr error + collectErr error buildPanic bool buildStarted chan struct{} buildBlock chan struct{} @@ -428,7 +516,7 @@ func (c *fakeCollector) Collect(ch chan<- prometheus.Metric, maxScrapeDuration t ch <- prometheus.MustNewConstMetric(prometheus.NewDesc("windows_test_"+c.name, "Test metric", nil, nil), prometheus.GaugeValue, 42) - return nil + return c.collectErr } func (c *fakeCollector) Close() error {