fix(metrics): record on the collector that is actually registered

When an equal collector was already registered, the caller kept using
its own instance, whose observations are never scraped. RegisterCollector
now returns the registered collector so callers can adopt it.

pkg/metrics/metrics.go

@@ -133,17 +133,26 @@ func Register() {
 	)
 }
 
-func RegisterCollector(collector prometheus.Collector) {
+// RegisterCollector registers a collector and returns the collector to record
+// observations on. If an equal collector is already registered, that one is returned
+// instead; observations on the given collector would otherwise never be scraped.
+// Registration errors are logged, as they would leave the collector silently absent
+// from the metrics endpoint.
+func RegisterCollector[T prometheus.Collector](collector T) T {
 	err := prometheus.DefaultRegisterer.Register(collector)
 	if err == nil {
-		return
+		return collector
 	}
 
-	if _, ok := errors.AsType[prometheus.AlreadyRegisteredError](err); ok {
-		return
+	if alreadyRegistered, ok := errors.AsType[prometheus.AlreadyRegisteredError](err); ok {
+		if existing, ok := alreadyRegistered.ExistingCollector.(T); ok {
+			return existing
+		}
+		return collector
 	}
 
 	log.Warnf("metrics: registering collector: %+v", err)
+	return collector
 }
 
 func ObserveRedisLatency(operation string, fun func() error) error {

pkg/metrics/metrics_test.go

@@ -0,0 +1,28 @@
+package metrics_test
+
+import (
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/nais/wonderwall/pkg/metrics"
+)
+
+func TestRegisterCollector(t *testing.T) {
+	newCounter := func() *prometheus.CounterVec {
+		return prometheus.NewCounterVec(prometheus.CounterOpts{
+			Name: "wonderwall_test_register_collector_total",
+			Help: "counter used to assert registration behaviour",
+		}, []string{"label"})
+	}
+
+	first := newCounter()
+	assert.Same(t, first, metrics.RegisterCollector(first), "the first registration is used as-is")
+
+	// an equal collector is already registered, so observations on the second one would
+	// never be scraped; the registered one must be returned instead
+	second := newCounter()
+	assert.Same(t, first, metrics.RegisterCollector(second))
+	assert.NotSame(t, second, metrics.RegisterCollector(second))
+}

pkg/middleware/prometheus.go

@@ -56,8 +56,8 @@ func Prometheus(provider string, buckets ...float64) *PrometheusMiddleware {
 		[]string{"code", "method", "path", "host"},
 	)
 
-	metrics.RegisterCollector(m.reqs)
-	metrics.RegisterCollector(m.latency)
+	m.reqs = metrics.RegisterCollector(m.reqs)
+	m.latency = metrics.RegisterCollector(m.latency)
 
 	return &m
 }