From 7291f00f483f213c316b99383ccbfe53b9d26e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ask=20Bj=C3=B8rn=20Hansen?= Date: Sun, 12 Oct 2025 16:13:19 -0700 Subject: [PATCH] feat(metricsserver): add Gatherer method Add explicit Gatherer() method to improve API discoverability and prevent users from accidentally using prometheus.DefaultGatherer instead of the custom registry. Changes: - Add Gatherer() method returning prometheus.Gatherer interface - Add NewWithDefaultGatherer() constructor for opt-in default usage - Update package docs with usage examples - Add tests for both gatherer modes --- metricsserver/metrics.go | 56 +++++++++++++++++++++++- metricsserver/metrics_test.go | 80 +++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/metricsserver/metrics.go b/metricsserver/metrics.go index 622c1c1..51c0236 100644 --- a/metricsserver/metrics.go +++ b/metricsserver/metrics.go @@ -3,6 +3,24 @@ // This package implements a dedicated metrics server that exposes application metrics // via HTTP. It uses a custom Prometheus registry to avoid conflicts with other metric // collectors and provides graceful shutdown capabilities. +// +// # Usage +// +// Create a new metrics server and register your metrics with its Registry(): +// +// m := metricsserver.New() +// myCounter := prometheus.NewCounter(...) +// m.Registry().MustRegister(myCounter) +// +// When you need a Gatherer (for example, to pass to other libraries), use the Gatherer() method +// instead of prometheus.DefaultGatherer to ensure your custom metrics are collected: +// +// gatherer := m.Gatherer() // Returns the custom registry as a Gatherer +// +// To use the default Prometheus gatherer alongside your custom registry: +// +// m := metricsserver.NewWithDefaultGatherer() +// m.Gatherer() // Returns prometheus.DefaultGatherer package metricsserver import ( @@ -21,15 +39,32 @@ import ( // Metrics provides a custom Prometheus registry and HTTP handlers for metrics exposure. // It isolates application metrics from the default global registry. type Metrics struct { - r *prometheus.Registry + r *prometheus.Registry + useDefaultGatherer bool } // New creates a new Metrics instance with a custom Prometheus registry. +// Use this when you want isolated metrics that don't interfere with the default registry. func New() *Metrics { r := prometheus.NewRegistry() m := &Metrics{ - r: r, + r: r, + useDefaultGatherer: false, + } + + return m +} + +// NewWithDefaultGatherer creates a new Metrics instance that uses the default Prometheus gatherer. +// This is useful when you want to expose metrics from the default registry alongside your custom metrics. +// The custom registry will still be available via Registry() but Gatherer() will return prometheus.DefaultGatherer. +func NewWithDefaultGatherer() *Metrics { + r := prometheus.NewRegistry() + + m := &Metrics{ + r: r, + useDefaultGatherer: true, } return m @@ -41,6 +76,23 @@ func (m *Metrics) Registry() *prometheus.Registry { return m.r } +// Gatherer returns the Prometheus gatherer to use for collecting metrics. +// This returns the custom registry's Gatherer by default, ensuring your registered +// metrics are collected. If the instance was created with NewWithDefaultGatherer(), +// this returns prometheus.DefaultGatherer instead. +// +// Use this method when you need a prometheus.Gatherer interface, for example when +// configuring other libraries that need to collect metrics. +// +// IMPORTANT: Do not use prometheus.DefaultGatherer directly if you want to collect +// metrics registered with this instance's Registry(). Always use this Gatherer() method. +func (m *Metrics) Gatherer() prometheus.Gatherer { + if m.useDefaultGatherer { + return prometheus.DefaultGatherer + } + return m.r +} + // Handler returns an HTTP handler for the /metrics endpoint with OpenMetrics support. func (m *Metrics) Handler() http.Handler { log := logger.NewStdLog("prom http", false, nil) diff --git a/metricsserver/metrics_test.go b/metricsserver/metrics_test.go index a5fad2f..211e112 100644 --- a/metricsserver/metrics_test.go +++ b/metricsserver/metrics_test.go @@ -67,6 +67,47 @@ func TestRegistry(t *testing.T) { } } +func TestGatherer(t *testing.T) { + metrics := New() + + gatherer := metrics.Gatherer() + if gatherer == nil { + t.Fatal("Gatherer() returned nil") + } + + // Register a test metric + counter := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "test_gatherer_counter", + Help: "A test counter for gatherer", + }) + + metrics.Registry().MustRegister(counter) + counter.Inc() + + // Test that the gatherer collects our custom metric + metricFamilies, err := gatherer.Gather() + if err != nil { + t.Errorf("failed to gather metrics: %v", err) + } + + found := false + for _, mf := range metricFamilies { + if mf.GetName() == "test_gatherer_counter" { + found = true + break + } + } + + if !found { + t.Error("registered metric not found via Gatherer()") + } + + // Verify gatherer is the same as registry + if gatherer != metrics.r { + t.Error("Gatherer() should return the same object as the registry for custom registry mode") + } +} + func TestHandler(t *testing.T) { metrics := New() @@ -212,6 +253,45 @@ func TestListenAndServeContextCancellation(t *testing.T) { } } +func TestNewWithDefaultGatherer(t *testing.T) { + metrics := NewWithDefaultGatherer() + + if metrics == nil { + t.Fatal("NewWithDefaultGatherer returned nil") + } + + if !metrics.useDefaultGatherer { + t.Error("useDefaultGatherer should be true") + } + + gatherer := metrics.Gatherer() + if gatherer == nil { + t.Fatal("Gatherer() returned nil") + } + + // Verify it returns the default gatherer + if gatherer != prometheus.DefaultGatherer { + t.Error("Gatherer() should return prometheus.DefaultGatherer when useDefaultGatherer is true") + } + + // Verify the custom registry is still available and separate + if metrics.Registry() == nil { + t.Error("Registry() should still return a custom registry") + } + + // Test that registering in custom registry doesn't affect default gatherer check + counter := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "test_default_gatherer_counter", + Help: "A test counter", + }) + metrics.Registry().MustRegister(counter) + + // The gatherer should still be the default one, not our custom registry + if metrics.Gatherer() != prometheus.DefaultGatherer { + t.Error("Gatherer() should continue to return prometheus.DefaultGatherer") + } +} + // Benchmark the metrics handler response time func BenchmarkMetricsHandler(b *testing.B) { metrics := New()