test: First pass at unregistering metrics so that we can run multiple…

… tests (#12927)

pkg/compactor/deletion/delete_requests_client_test.go

@@ -7,12 +7,11 @@ import (
 	"testing"
 	"time"
 
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/require"
 )
 
 func TestGetCacheGenNumberForUser(t *testing.T) {
-	deleteClientMetrics := NewDeleteRequestClientMetrics(prometheus.DefaultRegisterer)
+	deleteClientMetrics := NewDeleteRequestClientMetrics(nil)
 
 	t.Run("it requests results from the compactor client", func(t *testing.T) {
 		compactorClient := mockCompactorClient{

pkg/compactor/deletion/metrics.go

@@ -12,6 +12,11 @@ type DeleteRequestClientMetrics struct {
 	deleteRequestsLookupsFailedTotal prometheus.Counter
 }
 
+func (m DeleteRequestClientMetrics) Unregister() {
+	prometheus.Unregister(m.deleteRequestsLookupsTotal)
+	prometheus.Unregister(m.deleteRequestsLookupsFailedTotal)
+}
+
 func NewDeleteRequestClientMetrics(r prometheus.Registerer) *DeleteRequestClientMetrics {
 	m := DeleteRequestClientMetrics{}
 

pkg/loki/loki_test.go

@@ -11,6 +11,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/collectors"
+	"github.com/prometheus/client_golang/prometheus/collectors/version"
+
+	"github.com/grafana/loki/v3/pkg/util/constants"
+
 	"github.com/grafana/dskit/flagext"
 	"github.com/grafana/dskit/server"
 	"github.com/stretchr/testify/assert"
@@ -251,4 +257,21 @@ schema_config:
 	require.NoError(t, err)
 	require.Equal(t, string(bBytes), "abc")
 	assert.True(t, customHandlerInvoked)
+	unregisterLokiMetrics(loki)
+}
+
+func unregisterLokiMetrics(loki *Loki) {
+	loki.ClientMetrics.Unregister()
+	loki.deleteClientMetrics.Unregister()
+	prometheus.Unregister(version.NewCollector(constants.Loki))
+	prometheus.Unregister(collectors.NewGoCollector(
+		collectors.WithGoCollectorRuntimeMetrics(collectors.MetricsAll),
+	))
+	//TODO Update DSKit to have a method to unregister these metrics
+	prometheus.Unregister(loki.Metrics.TCPConnections)
+	prometheus.Unregister(loki.Metrics.TCPConnectionsLimit)
+	prometheus.Unregister(loki.Metrics.RequestDuration)
+	prometheus.Unregister(loki.Metrics.ReceivedMessageSize)
+	prometheus.Unregister(loki.Metrics.SentMessageSize)
+	prometheus.Unregister(loki.Metrics.InflightRequests)
 }

pkg/pattern/flush_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/grafana/dskit/ring"
 	"github.com/grafana/dskit/services"
 	"github.com/grafana/dskit/user"
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/stretchr/testify/require"
 
@@ -23,7 +22,7 @@ import (
 )
 
 func TestSweepInstance(t *testing.T) {
-	ing, err := New(defaultIngesterTestConfig(t), "foo", prometheus.DefaultRegisterer, log.NewNopLogger())
+	ing, err := New(defaultIngesterTestConfig(t), "foo", nil, log.NewNopLogger())
 	require.NoError(t, err)
 	defer services.StopAndAwaitTerminated(context.Background(), ing) //nolint:errcheck
 	err = services.StartAndAwaitRunning(context.Background(), ing)

pkg/scheduler/scheduler_test.go

@@ -27,7 +27,7 @@ func TestScheduler_setRunState(t *testing.T) {
 	// we make a Scheduler with the things required to avoid nil pointers
 	s := Scheduler{
 		log: util_log.Logger,
-		schedulerRunning: promauto.With(prometheus.DefaultRegisterer).NewGauge(prometheus.GaugeOpts{
+		schedulerRunning: promauto.With(nil).NewGauge(prometheus.GaugeOpts{
 			Name: "cortex_query_scheduler_running",
 			Help: "Value will be 1 if the scheduler is in the ReplicationSet and actively receiving/processing requests",
 		}),

pkg/storage/chunk/client/congestion/congestion_test.go

@@ -9,11 +9,13 @@ import (
 
 func TestZeroValueConstruction(t *testing.T) {
 	cfg := Config{}
-	ctrl := NewController(cfg, log.NewNopLogger(), NewMetrics(t.Name(), cfg))
+	m := NewMetrics(t.Name(), cfg)
+	ctrl := NewController(cfg, log.NewNopLogger(), m)
 
 	require.IsType(t, &NoopController{}, ctrl)
 	require.IsType(t, &NoopRetrier{}, ctrl.getRetrier())
 	require.IsType(t, &NoopHedger{}, ctrl.getHedger())
+	m.Unregister()
 }
 
 func TestAIMDConstruction(t *testing.T) {
@@ -22,11 +24,13 @@ func TestAIMDConstruction(t *testing.T) {
 			Strategy: "aimd",
 		},
 	}
-	ctrl := NewController(cfg, log.NewNopLogger(), NewMetrics(t.Name(), cfg))
+	m := NewMetrics(t.Name(), cfg)
+	ctrl := NewController(cfg, log.NewNopLogger(), m)
 
 	require.IsType(t, &AIMDController{}, ctrl)
 	require.IsType(t, &NoopRetrier{}, ctrl.getRetrier())
 	require.IsType(t, &NoopHedger{}, ctrl.getHedger())
+	m.Unregister()
 }
 
 func TestRetrierConstruction(t *testing.T) {
@@ -35,11 +39,13 @@ func TestRetrierConstruction(t *testing.T) {
 			Strategy: "limited",
 		},
 	}
-	ctrl := NewController(cfg, log.NewNopLogger(), NewMetrics(t.Name(), cfg))
+	m := NewMetrics(t.Name(), cfg)
+	ctrl := NewController(cfg, log.NewNopLogger(), m)
 
 	require.IsType(t, &NoopController{}, ctrl)
 	require.IsType(t, &LimitedRetrier{}, ctrl.getRetrier())
 	require.IsType(t, &NoopHedger{}, ctrl.getHedger())
+	m.Unregister()
 }
 
 func TestCombinedConstruction(t *testing.T) {
@@ -51,11 +57,13 @@ func TestCombinedConstruction(t *testing.T) {
 			Strategy: "limited",
 		},
 	}
-	ctrl := NewController(cfg, log.NewNopLogger(), NewMetrics(t.Name(), cfg))
+	m := NewMetrics(t.Name(), cfg)
+	ctrl := NewController(cfg, log.NewNopLogger(), m)
 
 	require.IsType(t, &AIMDController{}, ctrl)
 	require.IsType(t, &LimitedRetrier{}, ctrl.getRetrier())
 	require.IsType(t, &NoopHedger{}, ctrl.getHedger())
+	m.Unregister()
 }
 
 func TestHedgerConstruction(t *testing.T) {

pkg/storage/chunk/client/congestion/controller_test.go

@@ -46,6 +46,7 @@ func TestRequestNoopRetry(t *testing.T) {
 
 	require.EqualValues(t, 2, testutil.ToFloat64(metrics.requests))
 	require.EqualValues(t, 0, testutil.ToFloat64(metrics.retries))
+	metrics.Unregister()
 }
 
 func TestRequestZeroLimitedRetry(t *testing.T) {
@@ -74,6 +75,7 @@ func TestRequestZeroLimitedRetry(t *testing.T) {
 
 	require.EqualValues(t, 1, testutil.ToFloat64(metrics.requests))
 	require.EqualValues(t, 0, testutil.ToFloat64(metrics.retries))
+	metrics.Unregister()
 }
 
 func TestRequestLimitedRetry(t *testing.T) {
@@ -109,6 +111,7 @@ func TestRequestLimitedRetry(t *testing.T) {
 	require.EqualValues(t, 1, testutil.ToFloat64(metrics.retriesExceeded))
 	require.EqualValues(t, 2, testutil.ToFloat64(metrics.retries))
 	require.EqualValues(t, 4, testutil.ToFloat64(metrics.requests))
+	metrics.Unregister()
 }
 
 func TestRequestLimitedRetryNonRetryableErr(t *testing.T) {
@@ -139,6 +142,7 @@ func TestRequestLimitedRetryNonRetryableErr(t *testing.T) {
 	require.EqualValues(t, 0, testutil.ToFloat64(metrics.retries))
 	require.EqualValues(t, 1, testutil.ToFloat64(metrics.nonRetryableErrors))
 	require.EqualValues(t, 1, testutil.ToFloat64(metrics.requests))
+	metrics.Unregister()
 }
 
 func TestAIMDReducedThroughput(t *testing.T) {
@@ -212,6 +216,7 @@ func TestAIMDReducedThroughput(t *testing.T) {
 
 	// should have registered some congestion latency in stats
 	require.NotZero(t, statsCtx.Store().CongestionControlLatency)
+	metrics.Unregister()
 }
 
 func runAndMeasureRate(ctx context.Context, ctrl Controller, duration time.Duration) (float64, float64) {

pkg/storage/chunk/client/congestion/metrics.go

@@ -17,6 +17,11 @@ type Metrics struct {
 
 func (m Metrics) Unregister() {
 	prometheus.Unregister(m.currentLimit)
+	prometheus.Unregister(m.backoffSec)
+	prometheus.Unregister(m.requests)
+	prometheus.Unregister(m.retries)
+	prometheus.Unregister(m.nonRetryableErrors)
+	prometheus.Unregister(m.retriesExceeded)
 }
 
 // NewMetrics creates metrics to be used for monitoring congestion control.

tools/tsdb/migrate-versions/main_test.go

@@ -141,4 +141,5 @@ func TestMigrateTables(t *testing.T) {
 			}
 		})
 	}
+	clientMetrics.Unregister()
 }