httphandler: merge registries without checking every metric twice (#2612)

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

internal/httphandler/gatherer.go

@@ -0,0 +1,167 @@
+// 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 httphandler
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+)
+
+// Interface guard.
+var _ prometheus.Gatherer = mergedGatherer{}
+
+// mergedGatherer combines the results of two registries.
+//
+// prometheus.Gatherers re-hashes and re-checks every metric that the registries
+// already checked in their own Gather. mergedGatherer merges the sorted results
+// without the second check, as long as no metric family name of one registry
+// collides with the other. If a name collides, it hands both results to
+// prometheus.Gatherers, so the output and errors stay the same as before.
+// See https://github.com/prometheus/client_golang/pull/1702.
+type mergedGatherer struct {
+	first  prometheus.Gatherer
+	second prometheus.Gatherer
+}
+
+func (g mergedGatherer) Gather() ([]*dto.MetricFamily, error) {
+	firstMFs, firstErr := g.first.Gather()
+	secondMFs, secondErr := g.second.Gather()
+
+	if namesCollide(firstMFs, secondMFs) {
+		return prometheus.Gatherers{
+			gathered(firstMFs, firstErr),
+			gathered(secondMFs, secondErr),
+		}.Gather()
+	}
+
+	var errs prometheus.MultiError
+
+	errs = appendGathererErrors(errs, 1, firstErr)
+	errs = appendGathererErrors(errs, 2, secondErr)
+
+	return mergeSorted(firstMFs, secondMFs), errs.MaybeUnwrap()
+}
+
+// gathered returns a Gatherer that returns an earlier Gather result.
+func gathered(mfs []*dto.MetricFamily, err error) prometheus.GathererFunc {
+	return prometheus.GathererFunc(func() ([]*dto.MetricFamily, error) {
+		return mfs, err
+	})
+}
+
+// appendGathererErrors adds err to errs like prometheus.Gatherers does.
+func appendGathererErrors(errs prometheus.MultiError, gatherer int, err error) prometheus.MultiError {
+	if err == nil {
+		return errs
+	}
+
+	var multiErr prometheus.MultiError
+	if !errors.As(err, &multiErr) {
+		return append(errs, fmt.Errorf("[from Gatherer #%d] %w", gatherer, err))
+	}
+
+	for _, err := range multiErr {
+		errs = append(errs, fmt.Errorf("[from Gatherer #%d] %w", gatherer, err))
+	}
+
+	return errs
+}
+
+// namesCollide reports whether a metric family name of a can clash with one of b.
+// Besides equal names, it counts a name that equals the other name plus a
+// summary or histogram suffix, regardless of the types. A false positive only
+// means the scrape takes the slower prometheus.Gatherers path.
+func namesCollide(a, b []*dto.MetricFamily) bool {
+	if len(a) == 0 || len(b) == 0 {
+		return false
+	}
+
+	// Index the smaller result. The exporter metrics registry has only a few families.
+	if len(a) > len(b) {
+		a, b = b, a
+	}
+
+	// names holds the names of a, bases the names of a without a summary or histogram suffix.
+	names := make(map[string]struct{}, len(a))
+	bases := make(map[string]struct{})
+
+	for _, mf := range a {
+		names[mf.GetName()] = struct{}{}
+
+		if base, ok := trimHistogramSuffix(mf.GetName()); ok {
+			bases[base] = struct{}{}
+		}
+	}
+
+	for _, mf := range b {
+		name := mf.GetName()
+
+		if _, ok := names[name]; ok {
+			return true
+		}
+
+		// A name of a is the name plus a suffix.
+		if _, ok := bases[name]; ok {
+			return true
+		}
+
+		// The name is a name of a plus a suffix.
+		if base, ok := trimHistogramSuffix(name); ok {
+			if _, ok := names[base]; ok {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
+// trimHistogramSuffix removes the suffixes the text formats add to summaries and histograms.
+// They are the suffixes client_golang checks for collisions.
+func trimHistogramSuffix(name string) (string, bool) {
+	for _, suffix := range [...]string{"_count", "_sum", "_bucket"} {
+		if base, ok := strings.CutSuffix(name, suffix); ok {
+			return base, true
+		}
+	}
+
+	return name, false
+}
+
+// mergeSorted merges two metric family slices that are sorted by name and share no name.
+func mergeSorted(a, b []*dto.MetricFamily) []*dto.MetricFamily {
+	result := make([]*dto.MetricFamily, 0, len(a)+len(b))
+
+	for len(a) > 0 && len(b) > 0 {
+		if a[0].GetName() < b[0].GetName() {
+			result = append(result, a[0])
+			a = a[1:]
+		} else {
+			result = append(result, b[0])
+			b = b[1:]
+		}
+	}
+
+	result = append(result, a...)
+
+	return append(result, b...)
+}

internal/httphandler/gatherer_test.go

@@ -0,0 +1,310 @@
+// 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 httphandler
+
+import (
+	"errors"
+	"fmt"
+	"strconv"
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/collectors"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+)
+
+func TestMergedGatherer(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name    string
+		first   []prometheus.Collector
+		second  []prometheus.Collector
+		names   []string
+		wantErr bool
+	}{
+		{
+			name:   "interleaved names",
+			first:  []prometheus.Collector{gauge("b_metric", "help", 1), gauge("d_metric", "help", 1)},
+			second: []prometheus.Collector{gauge("a_metric", "help", 1), gauge("c_metric", "help", 1), gauge("e_metric", "help", 1)},
+			names:  []string{"a_metric", "b_metric", "c_metric", "d_metric", "e_metric"},
+		},
+		{
+			name:  "empty second",
+			first: []prometheus.Collector{gauge("b_metric", "help", 1), gauge("a_metric", "help", 1)},
+			names: []string{"a_metric", "b_metric"},
+		},
+		{
+			name:   "same series",
+			first:  []prometheus.Collector{gauge("go_goroutines", "help", 1)},
+			second: []prometheus.Collector{gauge("go_goroutines", "help", 1), gauge("windows_metric", "help", 1)},
+			names:  []string{"go_goroutines", "windows_metric"},
+			// The second go_goroutines is a duplicate of the first.
+			wantErr: true,
+		},
+		{
+			name:   "same name, other labels",
+			first:  []prometheus.Collector{gauge("go_goroutines", "help", 1)},
+			second: []prometheus.Collector{gauge("go_goroutines", "help", 2)},
+			names:  []string{"go_goroutines"},
+		},
+		{
+			name:    "same name, other help",
+			first:   []prometheus.Collector{gauge("go_goroutines", "help", 1)},
+			second:  []prometheus.Collector{gauge("go_goroutines", "other help", 2)},
+			names:   []string{"go_goroutines"},
+			wantErr: true,
+		},
+		{
+			name:    "suffix of a summary in the second registry",
+			first:   []prometheus.Collector{gauge("go_gc_duration_seconds_count", "help", 1)},
+			second:  []prometheus.Collector{prometheus.NewSummary(prometheus.SummaryOpts{Name: "go_gc_duration_seconds", Help: "help"})},
+			names:   []string{"go_gc_duration_seconds_count"},
+			wantErr: true,
+		},
+		{
+			name:    "suffix of a histogram in the first registry",
+			first:   []prometheus.Collector{prometheus.NewHistogram(prometheus.HistogramOpts{Name: "request_seconds", Help: "help"})},
+			second:  []prometheus.Collector{gauge("request_seconds_bucket", "help", 1)},
+			names:   []string{"request_seconds"},
+			wantErr: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			first, second := &countingGatherer{Gatherer: registry(t, tc.first...)}, &countingGatherer{Gatherer: registry(t, tc.second...)}
+
+			got, err := mergedGatherer{first: first, second: second}.Gather()
+			want, gatherersErr := prometheus.Gatherers{registry(t, tc.first...), registry(t, tc.second...)}.Gather()
+
+			require.Equal(t, tc.names, familyNames(got))
+			requireEqualFamilies(t, want, got)
+			require.Equal(t, fmt.Sprint(gatherersErr), fmt.Sprint(err))
+
+			if tc.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			// A collision must not gather the registries a second time.
+			require.Equal(t, 1, first.calls)
+			require.Equal(t, 1, second.calls)
+		})
+	}
+}
+
+func TestMergedGathererErrors(t *testing.T) {
+	t.Parallel()
+
+	errFirst := errors.New("first failed")
+	errSecond := errors.New("second failed")
+
+	for _, tc := range []struct {
+		name      string
+		firstErr  error
+		secondErr error
+		errors    []string
+	}{
+		{name: "no error"},
+		{
+			name:     "plain error",
+			firstErr: errFirst,
+			errors:   []string{"[from Gatherer #1] first failed"},
+		},
+		{
+			name:      "multi error",
+			firstErr:  prometheus.MultiError{errFirst, errSecond},
+			secondErr: errSecond,
+			errors:    []string{"[from Gatherer #1] first failed", "[from Gatherer #1] second failed", "[from Gatherer #2] second failed"},
+		},
+		{
+			name:      "wrapped multi error",
+			secondErr: fmt.Errorf("wrapped: %w", prometheus.MultiError{errFirst, errSecond}),
+			errors:    []string{"[from Gatherer #2] first failed", "[from Gatherer #2] second failed"},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			first := gatheredFamilies(t, tc.firstErr, gauge("b_metric", "help", 1))
+			second := gatheredFamilies(t, tc.secondErr, gauge("a_metric", "help", 1))
+
+			got, err := mergedGatherer{first: first, second: second}.Gather()
+			want, gatherersErr := prometheus.Gatherers{first, second}.Gather()
+
+			// Partial results are returned together with the errors.
+			require.Equal(t, []string{"a_metric", "b_metric"}, familyNames(got))
+			requireEqualFamilies(t, want, got)
+			require.Equal(t, fmt.Sprint(gatherersErr), fmt.Sprint(err))
+
+			if len(tc.errors) == 0 {
+				require.NoError(t, err)
+
+				return
+			}
+
+			var multiErr prometheus.MultiError
+
+			if len(tc.errors) == 1 {
+				// A single error is unwrapped like in prometheus.Gatherers.
+				require.NotErrorAs(t, err, &multiErr)
+				require.EqualError(t, err, tc.errors[0])
+
+				return
+			}
+
+			require.ErrorAs(t, err, &multiErr)
+
+			messages := make([]string, 0, len(multiErr))
+			for _, err := range multiErr {
+				messages = append(messages, err.Error())
+			}
+
+			require.Equal(t, tc.errors, messages)
+		})
+	}
+}
+
+// BenchmarkGather compares prometheus.Gatherers with mergedGatherer for a scrape
+// with the exporter metrics and a few thousand constant metrics.
+func BenchmarkGather(b *testing.B) {
+	exporterMetrics := prometheus.NewRegistry()
+	exporterMetrics.MustRegister(
+		collectors.NewBuildInfoCollector(),
+		collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}),
+		collectors.NewGoCollector(),
+	)
+
+	scrape := prometheus.NewRegistry()
+	scrape.MustRegister(constCollector{families: 50, series: 100})
+
+	for _, bc := range []struct {
+		name     string
+		gatherer prometheus.Gatherer
+	}{
+		{name: "gatherer=Gatherers", gatherer: prometheus.Gatherers{exporterMetrics, scrape}},
+		{name: "gatherer=mergedGatherer", gatherer: mergedGatherer{first: exporterMetrics, second: scrape}},
+	} {
+		b.Run(bc.name, func(b *testing.B) {
+			b.ReportAllocs()
+
+			for b.Loop() {
+				if _, err := bc.gatherer.Gather(); err != nil {
+					b.Fatal(err)
+				}
+			}
+		})
+	}
+}
+
+// constCollector emits constant metrics like the collectors of the exporter.
+type constCollector struct {
+	families int
+	series   int
+}
+
+func (c constCollector) Describe(_ chan<- *prometheus.Desc) {}
+
+func (c constCollector) Collect(ch chan<- prometheus.Metric) {
+	for family := range c.families {
+		desc := prometheus.NewDesc(
+			"windows_bench_metric_"+strconv.Itoa(family)+"_total",
+			"Benchmark metric.",
+			[]string{"instance", "core"},
+			nil,
+		)
+
+		for series := range c.series {
+			ch <- prometheus.MustNewConstMetric(desc, prometheus.CounterValue, float64(series), "instance_"+strconv.Itoa(series), strconv.Itoa(series%8))
+		}
+	}
+}
+
+type countingGatherer struct {
+	prometheus.Gatherer
+
+	calls int
+}
+
+func (g *countingGatherer) Gather() ([]*dto.MetricFamily, error) {
+	g.calls++
+
+	return g.Gatherer.Gather()
+}
+
+// gauge returns a gauge with the label value, so equal values produce the same series.
+func gauge(name, help string, labelValue int) staticCollector {
+	desc := prometheus.NewDesc(name, help, nil, prometheus.Labels{"label": strconv.Itoa(labelValue)})
+
+	return staticCollector{metric: prometheus.MustNewConstMetric(desc, prometheus.GaugeValue, float64(labelValue))}
+}
+
+// staticCollector is an unchecked collector of one metric.
+type staticCollector struct {
+	metric prometheus.Metric
+}
+
+func (c staticCollector) Describe(_ chan<- *prometheus.Desc) {}
+
+func (c staticCollector) Collect(ch chan<- prometheus.Metric) {
+	ch <- c.metric
+}
+
+func registry(t *testing.T, cs ...prometheus.Collector) *prometheus.Registry {
+	t.Helper()
+
+	reg := prometheus.NewRegistry()
+	for _, c := range cs {
+		require.NoError(t, reg.Register(c))
+	}
+
+	return reg
+}
+
+// gatheredFamilies returns a Gatherer that returns the metrics of cs together with err.
+func gatheredFamilies(t *testing.T, err error, cs ...prometheus.Collector) prometheus.GathererFunc {
+	t.Helper()
+
+	mfs, gatherErr := registry(t, cs...).Gather()
+	require.NoError(t, gatherErr)
+
+	return gathered(mfs, err)
+}
+
+func familyNames(mfs []*dto.MetricFamily) []string {
+	names := make([]string, 0, len(mfs))
+	for _, mf := range mfs {
+		names = append(names, mf.GetName())
+	}
+
+	return names
+}
+
+func requireEqualFamilies(t *testing.T, want, got []*dto.MetricFamily) {
+	t.Helper()
+
+	require.Len(t, got, len(want))
+
+	for i := range want {
+		require.True(t, proto.Equal(want[i], got[i]), "metric family %d: want %v, got %v", i, want[i], got[i])
+	}
+}

internal/httphandler/httphandler.go

@@ -174,7 +174,7 @@ func (c *MetricsHTTPHandler) handlerFactory(ctx context.Context, logger *slog.Lo
 	var regHandler http.Handler
 	if c.exporterMetricsRegistry != nil {
 		regHandler = promhttp.HandlerFor(
-			prometheus.Gatherers{c.exporterMetricsRegistry, reg},
+			mergedGatherer{first: c.exporterMetricsRegistry, second: reg},
 			promhttp.HandlerOpts{
 				ErrorLog:          slog.NewLogLogger(logger.Handler(), slog.LevelError),
 				ErrorHandling:     promhttp.ContinueOnError,

internal/httphandler/httphandler_test.go

@@ -115,6 +115,61 @@ func TestMetricsHTTPHandlerCollectorStatus(t *testing.T) {
 	}
 }
 
+// TestMetricsHTTPHandlerNameCollision checks a collector metric that has the name of an exporter metric,
+// like a textfile metric named go_goroutines.
+func TestMetricsHTTPHandlerNameCollision(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name     string
+		options  *httphandler.Options
+		included []string
+		excluded []string
+	}{
+		{
+			name: "exporter metrics",
+			// The collector metric has another help text, so it is dropped and the other metrics are kept.
+			included: []string{"# HELP go_goroutines Number of goroutines that currently exist.", "windows_test_first 42", "windows_exporter_build_info"},
+			excluded: []string{"go_goroutines 7", "Collector goroutines"},
+		},
+		{
+			name:     "disable exporter metrics",
+			options:  &httphandler.Options{DisableExporterMetrics: true, TimeoutMargin: 0.5},
+			included: []string{"# HELP go_goroutines Collector goroutines", "go_goroutines 7", "windows_test_first 42", "windows_exporter_build_info"},
+			excluded: []string{"Number of goroutines that currently exist."},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			collection := collector.New(collector.Map{
+				"first":     &testCollector{name: "first"},
+				"collision": &collisionCollector{},
+			})
+			handler := httphandler.New(slog.New(slog.DiscardHandler), collection, tc.options)
+			response := httptest.NewRecorder()
+			handler.ServeHTTP(response, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/metrics", nil))
+
+			require.Equal(t, http.StatusOK, response.Code)
+
+			for _, value := range tc.included {
+				require.Contains(t, response.Body.String(), value)
+			}
+
+			for _, value := range tc.excluded {
+				require.NotContains(t, response.Body.String(), value)
+			}
+
+			if tc.options == nil {
+				// The gathering error of the first scrape is counted like before.
+				response = httptest.NewRecorder()
+				handler.ServeHTTP(response, httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/metrics", nil))
+				require.Contains(t, response.Body.String(), `promhttp_metric_handler_errors_total{cause="gathering"} 1`)
+			}
+		})
+	}
+}
+
 func TestMetricsHTTPHandlerScrapeTimeout(t *testing.T) {
 	for _, tc := range []struct {
 		name    string
@@ -262,3 +317,15 @@ func (c *testCollector) Collect(ch chan<- prometheus.Metric, _ time.Duration) er
 
 	return nil
 }
+
+// collisionCollector emits a metric with the name of a Go runtime metric.
+type collisionCollector struct{}
+
+func (c *collisionCollector) GetName() string                           { return "collision" }
+func (c *collisionCollector) Build(_ *slog.Logger, _ *mi.Session) error { return nil }
+func (c *collisionCollector) Close() error                              { return nil }
+func (c *collisionCollector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error {
+	ch <- prometheus.MustNewConstMetric(prometheus.NewDesc("go_goroutines", "Collector goroutines", nil, nil), prometheus.GaugeValue, 7)
+
+	return nil
+}