From cb14d1624f743e7e292efaa517cdf55ccff0b4d9 Mon Sep 17 00:00:00 2001
From: Trong Huu Nguyen <trong.huu.nguyen@nav.no>
Date: Tue, 28 Jul 2026 11:21:46 +0200
Subject: [PATCH] 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       | 17 +++++++++++++----
 pkg/metrics/metrics_test.go  | 28 ++++++++++++++++++++++++++++
 pkg/middleware/prometheus.go |  4 ++--
 3 files changed, 43 insertions(+), 6 deletions(-)
 create mode 100644 pkg/metrics/metrics_test.go

diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go
index 952c4ac..56be83d 100644
--- a/pkg/metrics/metrics.go
+++ b/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 {
diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go
new file mode 100644
index 0000000..ea850ff
--- /dev/null
+++ b/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))
+}
diff --git a/pkg/middleware/prometheus.go b/pkg/middleware/prometheus.go
index 1ba2c4f..a812e14 100644
--- a/pkg/middleware/prometheus.go
+++ b/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
 }