Handle duplicate registration of deletion metrics (#6446)

* Handle duplicate registration of deletion metrics

Signed-off-by: Michel Hollands <[email protected]>

* Create metrics struct at Loki level

Signed-off-by: Michel Hollands <[email protected]>

* Fix test

Signed-off-by: Michel Hollands <[email protected]>

Author: MichelHollands

pkg/loki/loki.go

@@ -20,6 +20,7 @@ import (
 	"github.com/grafana/dskit/runtimeconfig"
 	"github.com/grafana/dskit/services"
 	"github.com/pkg/errors"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/weaveworks/common/middleware"
 	"github.com/weaveworks/common/server"
 	"github.com/weaveworks/common/signals"
@@ -43,6 +44,7 @@ import (
 	"github.com/grafana/loki/pkg/storage/config"
 	"github.com/grafana/loki/pkg/storage/stores/series/index"
 	"github.com/grafana/loki/pkg/storage/stores/shipper/compactor"
+	"github.com/grafana/loki/pkg/storage/stores/shipper/compactor/deletion"
 	"github.com/grafana/loki/pkg/storage/stores/shipper/indexgateway"
 	"github.com/grafana/loki/pkg/tracing"
 	"github.com/grafana/loki/pkg/usagestats"
@@ -250,16 +252,18 @@ type Loki struct {
 	usageReport              *usagestats.Reporter
 	indexGatewayRingManager  *indexgateway.RingManager
 
-	clientMetrics storage.ClientMetrics
+	clientMetrics       storage.ClientMetrics
+	deleteClientMetrics *deletion.DeleteRequestClientMetrics
 
 	HTTPAuthMiddleware middleware.Interface
 }
 
 // New makes a new Loki.
 func New(cfg Config) (*Loki, error) {
 	loki := &Loki{
-		Cfg:           cfg,
-		clientMetrics: storage.NewClientMetrics(),
+		Cfg:                 cfg,
+		clientMetrics:       storage.NewClientMetrics(),
+		deleteClientMetrics: deletion.NewDeleteRequestClientMetrics(prometheus.DefaultRegisterer),
 	}
 	usagestats.Edition("oss")
 	loki.setupAuthMiddleware()

pkg/loki/modules.go

@@ -1002,7 +1002,7 @@ func (t *Loki) deleteRequestsClient() (deletion.DeleteRequestsClient, error) {
 		return nil, err
 	}
 
-	return deletion.NewDeleteRequestsClient(compactorAddress, &http.Client{Timeout: 5 * time.Second}, prometheus.DefaultRegisterer)
+	return deletion.NewDeleteRequestsClient(compactorAddress, &http.Client{Timeout: 5 * time.Second}, t.deleteClientMetrics)
 }
 
 func calculateMaxLookBack(pc config.PeriodConfig, maxLookBackConfig, minDuration time.Duration) (time.Duration, error) {

pkg/storage/stores/shipper/compactor/deletion/delete_requests_client.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"github.com/go-kit/log/level"
-	"github.com/prometheus/client_golang/prometheus"
 
 	"github.com/grafana/loki/pkg/util/log"
 )
@@ -33,7 +32,7 @@ type deleteRequestsClient struct {
 	cache         map[string][]DeleteRequest
 	cacheDuration time.Duration
 
-	metrics *deleteRequestClientMetrics
+	metrics *DeleteRequestClientMetrics
 
 	stopChan chan struct{}
 }
@@ -50,7 +49,7 @@ func WithRequestClientCacheDuration(d time.Duration) DeleteRequestsStoreOption {
 	}
 }
 
-func NewDeleteRequestsClient(addr string, c httpClient, r prometheus.Registerer, opts ...DeleteRequestsStoreOption) (DeleteRequestsClient, error) {
+func NewDeleteRequestsClient(addr string, c httpClient, deleteClientMetrics *DeleteRequestClientMetrics, opts ...DeleteRequestsStoreOption) (DeleteRequestsClient, error) {
 	u, err := url.Parse(addr)
 	if err != nil {
 		level.Error(log.Logger).Log("msg", "error parsing url", "err", err)
@@ -63,7 +62,7 @@ func NewDeleteRequestsClient(addr string, c httpClient, r prometheus.Registerer,
 		httpClient:    c,
 		cacheDuration: 5 * time.Minute,
 		cache:         make(map[string][]DeleteRequest),
-		metrics:       newDeleteRequestClientMetrics(r),
+		metrics:       deleteClientMetrics,
 		stopChan:      make(chan struct{}),
 	}
 

pkg/storage/stores/shipper/compactor/deletion/delete_requests_client_test.go

@@ -8,13 +8,16 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/require"
 )
 
 func TestGetCacheGenNumberForUser(t *testing.T) {
+	deleteClientMetrics := NewDeleteRequestClientMetrics(prometheus.DefaultRegisterer)
+
 	t.Run("it requests results from the api", func(t *testing.T) {
 		httpClient := &mockHTTPClient{ret: `[{"request_id":"test-request"}]`}
-		client, err := NewDeleteRequestsClient("http://test-server", httpClient, nil)
+		client, err := NewDeleteRequestsClient("http://test-server", httpClient, deleteClientMetrics)
 		require.Nil(t, err)
 
 		deleteRequests, err := client.GetAllDeleteRequestsForUser(context.Background(), "userID")
@@ -30,7 +33,7 @@ func TestGetCacheGenNumberForUser(t *testing.T) {
 
 	t.Run("it caches the results", func(t *testing.T) {
 		httpClient := &mockHTTPClient{ret: `[{"request_id":"test-request"}]`}
-		client, err := NewDeleteRequestsClient("http://test-server", httpClient, nil, WithRequestClientCacheDuration(100*time.Millisecond))
+		client, err := NewDeleteRequestsClient("http://test-server", httpClient, deleteClientMetrics, WithRequestClientCacheDuration(100*time.Millisecond))
 		require.Nil(t, err)
 
 		deleteRequests, err := client.GetAllDeleteRequestsForUser(context.Background(), "userID")

pkg/storage/stores/shipper/compactor/deletion/metrics.go

@@ -5,13 +5,13 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promauto"
 )
 
-type deleteRequestClientMetrics struct {
+type DeleteRequestClientMetrics struct {
 	deleteRequestsLookupsTotal       prometheus.Counter
 	deleteRequestsLookupsFailedTotal prometheus.Counter
 }
 
-func newDeleteRequestClientMetrics(r prometheus.Registerer) *deleteRequestClientMetrics {
-	m := deleteRequestClientMetrics{}
+func NewDeleteRequestClientMetrics(r prometheus.Registerer) *DeleteRequestClientMetrics {
+	m := DeleteRequestClientMetrics{}
 
 	m.deleteRequestsLookupsTotal = promauto.With(r).NewCounter(prometheus.CounterOpts{
 		Namespace: "loki",