refactor: remove prometheus.NewRegistry()

replace it with prometheus.DefaultRegisterer
so by default boxo users who do not specify registry
are not missing metrics

CHANGELOG.md

@@ -16,7 +16,8 @@ The following emojis are used to highlight certain changes:
 
 ### Added
 
-- `routing/http/server`: added Prometheus instrumentation to http delegated routing endpoints.
+- `gateway`: `NewCacheBlockStore` and `NewCarBackend` will use `prometheus.DefaultRegisterer` when a custom one is not specified via `WithPrometheusRegistry`
+- `routing/http/server`: exposes Prometheus metrics on `prometheus.DefaultRegisterer` and a custom one can be provided via `WithPrometheusRegistry`
 
 ### Changed
 

examples/car-file-fetcher/main.go

@@ -12,6 +12,7 @@ import (
 	"github.com/ipfs/boxo/files"
 	"github.com/ipfs/boxo/gateway"
 	"github.com/ipfs/boxo/path"
+	"github.com/prometheus/client_golang/prometheus"
 )
 
 func main() {
@@ -54,10 +55,15 @@ func fetch(gatewayURL, ipfsPath, outputPath, userAgent string, limit int64) erro
 		},
 	}
 
+	// Most of the time you want to use prometheus.DefaultRegisterer,
+	// but here we create custom one to demonstrate how custom configuration can
+	// passed to the backend (and to allow tests to run in parallel)
+	metrics := prometheus.NewRegistry()
+
 	// Create the remote CAR gateway backend pointing to the given gateway URL and
 	// using our [http.Client]. A custom [http.Client] is not required and the called
 	// function would create a new one instead.
-	backend, err := gateway.NewRemoteCarBackend([]string{gatewayURL}, httpClient)
+	backend, err := gateway.NewRemoteCarBackend([]string{gatewayURL}, httpClient, gateway.WithPrometheusRegistry(metrics))
 	if err != nil {
 		return err
 	}

gateway/backend_car.go

@@ -73,7 +73,7 @@ func NewCarBackend(f CarFetcher, opts ...BackendOption) (*CarBackend, error) {
 		return nil, err
 	}
 
-	var promReg prometheus.Registerer = prometheus.NewRegistry()
+	var promReg prometheus.Registerer = prometheus.DefaultRegisterer
 	if compiledOptions.promRegistry != nil {
 		promReg = compiledOptions.promRegistry
 	}

gateway/backend_car_test.go

@@ -24,13 +24,20 @@ import (
 	carv2 "github.com/ipld/go-car/v2"
 	carbs "github.com/ipld/go-car/v2/blockstore"
 	"github.com/ipld/go-car/v2/storage"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 //go:embed testdata/directory-with-multilayer-hamt-and-multiblock-files.car
 var dirWithMultiblockHAMTandFiles []byte
 
+func getTestNewCarBackend(f CarFetcher, opts ...BackendOption) (*CarBackend, error) {
+	// unique prometheus registry to allow for tests to run in parallell without registration conflict
+	opts = append(opts, WithPrometheusRegistry(prometheus.NewRegistry()))
+	return NewCarBackend(f, opts...)
+}
+
 func TestCarBackendTar(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -161,7 +168,7 @@ func TestCarBackendTar(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	p := path.FromCid(cid.MustParse("bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi"))
@@ -331,7 +338,7 @@ func TestCarBackendTarAtEndOfPath(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	p, err := path.Join(path.FromCid(cid.MustParse("bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi")), "hamtDir")
@@ -493,7 +500,7 @@ func TestCarBackendGetFile(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	trustedGatewayServer := httptest.NewServer(NewHandler(Config{DeserializedResponses: true}, backend))
@@ -597,7 +604,7 @@ func TestCarBackendGetFileRangeRequest(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	trustedGatewayServer := httptest.NewServer(NewHandler(Config{DeserializedResponses: true}, backend))
@@ -707,7 +714,7 @@ func TestCarBackendGetFileWithBadBlockReturned(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	trustedGatewayServer := httptest.NewServer(NewHandler(Config{DeserializedResponses: true}, backend))
@@ -812,7 +819,7 @@ func TestCarBackendGetHAMTDirectory(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	trustedGatewayServer := httptest.NewServer(NewHandler(Config{DeserializedResponses: true}, backend))
@@ -921,7 +928,7 @@ func TestCarBackendGetCAR(t *testing.T) {
 	fetcher, err := NewRetryCarFetcher(bs, 3)
 	require.NoError(t, err)
 
-	backend, err := NewCarBackend(fetcher)
+	backend, err := getTestNewCarBackend(fetcher)
 	require.NoError(t, err)
 
 	p := path.FromCid(cid.MustParse("bafybeid3fd2xxdcd3dbj7trb433h2aqssn6xovjbwnkargjv7fuog4xjdi"))
@@ -1031,7 +1038,7 @@ func TestCarBackendPassthroughErrors(t *testing.T) {
 			}}, 3)
 			require.NoError(t, err)
 
-			backend, err := NewCarBackend(fetcher)
+			backend, err := getTestNewCarBackend(fetcher)
 			require.NoError(t, err)
 
 			err = traversal(ctx, imPath, backend)

gateway/blockstore.go

@@ -35,16 +35,15 @@ var _ blockstore.Blockstore = (*cacheBlockStore)(nil)
 // in memory using a two queue cache. It can be useful, for example, when paired
 // with a proxy blockstore (see [NewRemoteBlockstore]).
 //
-// If the given [prometheus.Registerer] is nil, a new one will be created using
-// [prometheus.NewRegistry].
+// If the given [prometheus.Registerer] is nil, a [prometheus.DefaultRegisterer] will be used.
 func NewCacheBlockStore(size int, reg prometheus.Registerer) (blockstore.Blockstore, error) {
 	c, err := lru.New2Q[string, []byte](size)
 	if err != nil {
 		return nil, err
 	}
 
 	if reg == nil {
-		reg = prometheus.NewRegistry()
+		reg = prometheus.DefaultRegisterer
 	}
 
 	cacheHitsMetric := prometheus.NewCounter(prometheus.CounterOpts{

routing/http/client/client_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/ipfs/boxo/routing/http/types"
 	"github.com/ipfs/boxo/routing/http/types/iter"
 	"github.com/ipfs/go-cid"
+	"github.com/prometheus/client_golang/prometheus"
 
 	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/peer"
@@ -99,6 +100,7 @@ func makeTestDeps(t *testing.T, clientsOpts []Option, serverOpts []server.Option
 	const testUserAgent = "testUserAgent"
 	peerID, addrs, identity := makeProviderAndIdentity()
 	router := &mockContentRouter{}
+	serverOpts = append(serverOpts, server.WithPrometheusRegistry(prometheus.NewRegistry())) // avoid panic: duplicate metrics collector registration
 	recordingHandler := &recordingHandler{
 		Handler: server.Handler(router, serverOpts...),
 		f: []func(*http.Request){

routing/http/server/server.go

@@ -144,7 +144,7 @@ func Handler(svc ContentRouter, opts ...Option) http.Handler {
 	}
 
 	if server.promRegistry == nil {
-		server.promRegistry = prometheus.NewRegistry()
+		server.promRegistry = prometheus.DefaultRegisterer
 	}
 
 	// Create middleware with prometheus recorder

routing/http/server/server_test.go

@@ -25,13 +25,20 @@ import (
 	"github.com/libp2p/go-libp2p/core/routing"
 	b58 "github.com/mr-tron/base58/base58"
 	"github.com/multiformats/go-multiaddr"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
+func getTestHandler(router ContentRouter, opts ...Option) http.Handler {
+	// unique prometheus registry to allow for tests to run in parallell without registration conflict
+	opts = append(opts, WithPrometheusRegistry(prometheus.NewRegistry()))
+	return Handler(router, opts...)
+}
+
 func TestHeaders(t *testing.T) {
 	router := &mockContentRouter{}
-	server := httptest.NewServer(Handler(router))
+	server := httptest.NewServer(getTestHandler(router))
 	t.Cleanup(server.Close)
 	serverAddr := "http://" + server.Listener.Addr().String()
 
@@ -144,7 +151,7 @@ func TestProviders(t *testing.T) {
 		}
 
 		router := &mockContentRouter{}
-		server := httptest.NewServer(Handler(router))
+		server := httptest.NewServer(getTestHandler(router))
 		t.Cleanup(server.Close)
 		serverAddr := "http://" + server.Listener.Addr().String()
 		limit := DefaultRecordsLimit
@@ -246,7 +253,7 @@ func TestProviders(t *testing.T) {
 	t.Run("404 when router returns routing.ErrNotFound", func(t *testing.T) {
 		t.Parallel()
 		router := &mockContentRouter{}
-		server := httptest.NewServer(Handler(router))
+		server := httptest.NewServer(getTestHandler(router))
 		t.Cleanup(server.Close)
 		serverAddr := "http://" + server.Listener.Addr().String()
 		router.On("FindProviders", mock.Anything, cid, DefaultRecordsLimit).Return(nil, routing.ErrNotFound)
@@ -262,7 +269,7 @@ func TestProviders(t *testing.T) {
 
 func TestPeers(t *testing.T) {
 	makeRequest := func(t *testing.T, router *mockContentRouter, contentType, arg, filterAddrs, filterProtocols string) *http.Response {
-		server := httptest.NewServer(Handler(router))
+		server := httptest.NewServer(getTestHandler(router))
 		t.Cleanup(server.Close)
 
 		urlStr := fmt.Sprintf("http://%s/routing/v1/peers/%s", server.Listener.Addr().String(), arg)
@@ -662,7 +669,7 @@ func TestIPNS(t *testing.T) {
 	require.NoError(t, err)
 
 	makeRequest := func(t *testing.T, router *mockContentRouter, path string, accept string) *http.Response {
-		server := httptest.NewServer(Handler(router))
+		server := httptest.NewServer(getTestHandler(router))
 		t.Cleanup(server.Close)
 		serverAddr := "http://" + server.Listener.Addr().String()
 		urlStr := serverAddr + path
@@ -814,7 +821,7 @@ func TestIPNS(t *testing.T) {
 			router := &mockContentRouter{}
 			router.On("PutIPNS", mock.Anything, name1, record1).Return(nil)
 
-			server := httptest.NewServer(Handler(router))
+			server := httptest.NewServer(getTestHandler(router))
 			t.Cleanup(server.Close)
 			serverAddr := "http://" + server.Listener.Addr().String()
 			urlStr := serverAddr + "/routing/v1/ipns/" + name1.String()
@@ -833,7 +840,7 @@ func TestIPNS(t *testing.T) {
 
 			router := &mockContentRouter{}
 
-			server := httptest.NewServer(Handler(router))
+			server := httptest.NewServer(getTestHandler(router))
 			t.Cleanup(server.Close)
 			serverAddr := "http://" + server.Listener.Addr().String()
 			urlStr := serverAddr + "/routing/v1/ipns/" + name2.String()