From 35cd3ddf73ddff92639813e25e331f5c191cbc78 Mon Sep 17 00:00:00 2001
From: Vlad <13818348+walldiss@users.noreply.github.com>
Date: Mon, 23 Sep 2024 16:05:34 +0200
Subject: [PATCH] fix(nodebuilder/p2p/metrics): enforce init order for bitswap
 metrics (#3763)

---
 go.mod                     |  2 +-
 nodebuilder/p2p/bitswap.go | 56 ++++++++++++++++++++++++++++++++++----
 nodebuilder/p2p/metrics.go |  6 +++-
 3 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/go.mod b/go.mod
index b594f78379..5023ad6f1d 100644
--- a/go.mod
+++ b/go.mod
@@ -42,6 +42,7 @@ require (
 	github.com/ipfs/go-ipld-cbor v0.1.0
 	github.com/ipfs/go-ipld-format v0.6.0
 	github.com/ipfs/go-log/v2 v2.5.1
+	github.com/ipfs/go-metrics-interface v0.0.1
 	github.com/ipfs/go-metrics-prometheus v0.0.2
 	github.com/ipld/go-car v0.6.2
 	github.com/libp2p/go-libp2p v0.36.2
@@ -213,7 +214,6 @@ require (
 	github.com/ipfs/go-ipld-legacy v0.2.1 // indirect
 	github.com/ipfs/go-log v1.0.5 // indirect
 	github.com/ipfs/go-merkledag v0.11.0 // indirect
-	github.com/ipfs/go-metrics-interface v0.0.1 // indirect
 	github.com/ipfs/go-peertaskqueue v0.8.1 // indirect
 	github.com/ipfs/go-verifcid v0.0.3 // indirect
 	github.com/ipld/go-car/v2 v2.13.1 // indirect
diff --git a/nodebuilder/p2p/bitswap.go b/nodebuilder/p2p/bitswap.go
index 995800376e..d419a53ab4 100644
--- a/nodebuilder/p2p/bitswap.go
+++ b/nodebuilder/p2p/bitswap.go
@@ -9,10 +9,12 @@ import (
 	"github.com/ipfs/boxo/blockstore"
 	"github.com/ipfs/boxo/exchange"
 	"github.com/ipfs/go-datastore"
-	metrics "github.com/ipfs/go-metrics-prometheus"
+	ipfsmetrics "github.com/ipfs/go-metrics-interface"
+	ipfsprom "github.com/ipfs/go-metrics-prometheus"
 	routinghelpers "github.com/libp2p/go-libp2p-routing-helpers"
 	hst "github.com/libp2p/go-libp2p/core/host"
 	"github.com/libp2p/go-libp2p/core/protocol"
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 
 	"github.com/celestiaorg/celestia-node/share/eds"
@@ -43,7 +45,14 @@ func dataExchange(params bitSwapParams) exchange.Interface {
 		bitswap.SetSimulateDontHavesOnTimeout(false),
 		bitswap.WithoutDuplicatedBlockStats(),
 	}
-	bs := bitswap.New(params.Ctx, net, params.Bs, opts...)
+
+	ctx := params.Ctx
+	if params.Metrics != nil {
+		// metrics scope is required for prometheus metrics and will be used as metrics name
+		// prefix
+		ctx = ipfsmetrics.CtxScope(ctx, "bitswap")
+	}
+	bs := bitswap.New(ctx, net, params.Bs, opts...)
 
 	params.Lifecycle.Append(fx.Hook{
 		OnStop: func(_ context.Context) (err error) {
@@ -53,7 +62,16 @@ func dataExchange(params bitSwapParams) exchange.Interface {
 	return bs
 }
 
-func blockstoreFromDatastore(ctx context.Context, ds datastore.Batching) (blockstore.Blockstore, error) {
+func blockstoreFromDatastore(
+	ctx context.Context,
+	ds datastore.Batching,
+	b blockstoreParams,
+) (blockstore.Blockstore, error) {
+	if b.Metrics != nil {
+		// metrics scope is required for prometheus metrics and will be used as metrics name
+		// prefix
+		ctx = ipfsmetrics.CtxScope(ctx, "blockstore")
+	}
 	return blockstore.CachedBlockstore(
 		ctx,
 		blockstore.NewBlockstore(ds),
@@ -65,7 +83,16 @@ func blockstoreFromDatastore(ctx context.Context, ds datastore.Batching) (blocks
 	)
 }
 
-func blockstoreFromEDSStore(ctx context.Context, store *eds.Store) (blockstore.Blockstore, error) {
+func blockstoreFromEDSStore(
+	ctx context.Context,
+	store *eds.Store,
+	b blockstoreParams,
+) (blockstore.Blockstore, error) {
+	if b.Metrics != nil {
+		// metrics scope is required for prometheus metrics and will be used as metrics name
+		// prefix
+		ctx = ipfsmetrics.CtxScope(ctx, "blockstore")
+	}
 	return blockstore.CachedBlockstore(
 		ctx,
 		store.Blockstore(),
@@ -75,6 +102,13 @@ func blockstoreFromEDSStore(ctx context.Context, store *eds.Store) (blockstore.B
 	)
 }
 
+type blockstoreParams struct {
+	fx.In
+	// Metrics is unused, it is in dependency graph to ensure that prometheus metrics are enabled before bitswap
+	// is started.
+	Metrics *bitswapMetrics `optional:"true"`
+}
+
 type bitSwapParams struct {
 	fx.In
 
@@ -83,12 +117,22 @@ type bitSwapParams struct {
 	Net       Network
 	Host      hst.Host
 	Bs        blockstore.Blockstore
+	// Metrics is unused, it is in dependency graph to ensure that prometheus metrics are enabled before bitswap
+	// is started.
+	Metrics *bitswapMetrics `optional:"true"`
 }
 
 func protocolID(network Network) protocol.ID {
 	return protocol.ID(fmt.Sprintf("/celestia/%s", network))
 }
 
-func enableBitswapMetrics() {
-	_ = metrics.Inject()
+type bitswapMetrics struct{}
+
+func enableBitswapMetrics(_ prometheus.Registerer) *bitswapMetrics {
+	err := ipfsprom.Inject()
+	if err != nil {
+		log.Errorf("failed to inject bitswap metrics: %s", err)
+		return nil
+	}
+	return &bitswapMetrics{}
 }
diff --git a/nodebuilder/p2p/metrics.go b/nodebuilder/p2p/metrics.go
index 6c31a5ab52..84916d63ae 100644
--- a/nodebuilder/p2p/metrics.go
+++ b/nodebuilder/p2p/metrics.go
@@ -19,7 +19,7 @@ func WithMetrics() fx.Option {
 	return fx.Options(
 		fx.Provide(resourceManagerOpt(traceReporter)),
 		fx.Provide(prometheusMetrics),
-		fx.Invoke(enableBitswapMetrics),
+		fx.Provide(enableBitswapMetrics),
 	)
 }
 
@@ -45,6 +45,10 @@ func prometheusMetrics(lifecycle fx.Lifecycle,
 		peerIDLabel:   peerID.String(),
 	}
 	wrapped := prometheus.WrapRegistererWith(labels, reg)
+	// Set the default global registerer to the wrapped one with labels. This way all the metrics
+	// registered with the default registerer will be labeled with the provided labels. It is important
+	// because unlike libp2p metrics, bitswap metrics are registered with the default global registerer.
+	prometheus.DefaultRegisterer = wrapped
 
 	mux := http.NewServeMux()
 	handler := promhttp.HandlerFor(reg, promhttp.HandlerOpts{Registry: wrapped})