suggestions for handling re-registration

metrics/prometheus/noop_register.go

@@ -0,0 +1,15 @@
+// (c) 2025 Ava Labs, Inc. All rights reserved.
+// See the file LICENSE for licensing terms.
+package prometheus
+
+import (
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+)
+
+type NoopRegister struct{}
+
+func (n *NoopRegister) Register(prometheus.Collector) error  { return nil }
+func (n *NoopRegister) MustRegister(...prometheus.Collector) {}
+func (n *NoopRegister) Unregister(prometheus.Collector) bool { return true }
+func (n *NoopRegister) Gather() ([]*dto.MetricFamily, error) { return nil, nil }

metrics/prometheus/prometheus.go

@@ -44,6 +44,9 @@ func (g *Gatherer) Gather() (mfs []*dto.MetricFamily, err error) {
 		if errors.Is(err, errMetricSkip) {
 			continue
 		}
+		if err != nil {
+			return nil, err
+		}
 		mfs = append(mfs, mf)
 	}
 

plugin/evm/config/config.go

@@ -126,8 +126,8 @@ type Config struct {
 	PruneWarpDB                     bool    `json:"prune-warp-db-enabled"`              // Determines if the warpDB should be cleared on startup
 
 	// Metric Settings
-	MetricsEnabled          *bool `json:"metrics-enabled,omitempty"`
-	MetricsExpensiveEnabled bool  `json:"metrics-expensive-enabled"` // Debug-level metrics that might impact runtime performance
+	MetricsEnabled          bool `json:"metrics-enabled,omitempty"`
+	MetricsExpensiveEnabled bool `json:"metrics-expensive-enabled"` // Debug-level metrics that might impact runtime performance
 
 	// API Settings
 	LocalTxsEnabled bool `json:"local-txs-enabled"`
@@ -245,7 +245,7 @@ func (c *Config) SetDefaults(txPoolConfig TxPoolConfig) {
 	c.EnabledEthAPIs = defaultEnabledAPIs
 	c.RPCGasCap = defaultRpcGasCap
 	c.RPCTxFeeCap = defaultRpcTxFeeCap
-	c.MetricsEnabled = defaultPointer(c.MetricsEnabled, defaultMetricsEnabled)
+	c.MetricsEnabled = defaultMetricsEnabled
 	c.MetricsExpensiveEnabled = defaultMetricsExpensiveEnabled
 
 	// TxPool settings
@@ -293,13 +293,6 @@ func (c *Config) SetDefaults(txPoolConfig TxPoolConfig) {
 	c.AcceptedCacheSize = defaultAcceptedCacheSize
 }
 
-func defaultPointer[T any](existing *T, defaultValue T) (updated *T) {
-	if existing != nil {
-		return existing
-	}
-	return &defaultValue
-}
-
 func (d *Duration) UnmarshalJSON(data []byte) (err error) {
 	var v interface{}
 	if err := json.Unmarshal(data, &v); err != nil {

plugin/evm/syncervm_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/ava-labs/avalanchego/api/metrics"
 	avalancheatomic "github.com/ava-labs/avalanchego/chains/atomic"
 	avalanchedatabase "github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/database/prefixdb"
@@ -35,7 +36,6 @@ import (
 	"github.com/ava-labs/coreth/core/types"
 	"github.com/ava-labs/coreth/params"
 	"github.com/ava-labs/coreth/plugin/evm/atomic"
-	"github.com/ava-labs/coreth/plugin/evm/config"
 	"github.com/ava-labs/coreth/plugin/evm/database"
 	"github.com/ava-labs/coreth/predicate"
 	statesyncclient "github.com/ava-labs/coreth/sync/client"
@@ -85,8 +85,6 @@ func TestStateSyncFromScratchExceedParent(t *testing.T) {
 	testSyncerVM(t, vmSetup, test)
 }
 
-func pointerTo[T any](x T) *T { return &x }
-
 func TestStateSyncToggleEnabledToDisabled(t *testing.T) {
 	rand.Seed(1)
 
@@ -127,11 +125,7 @@ func TestStateSyncToggleEnabledToDisabled(t *testing.T) {
 	test.responseIntercept = nil
 	test.expectedErr = nil
 
-	syncDisabledVM := &VM{
-		config: config.Config{
-			MetricsEnabled: pointerTo(false),
-		},
-	}
+	syncDisabledVM := &VM{}
 	appSender := &enginetest.Sender{T: t}
 	appSender.SendAppGossipF = func(context.Context, commonEng.SendConfig, []byte) error { return nil }
 	appSender.SendAppRequestF = func(ctx context.Context, nodeSet set.Set[ids.NodeID], requestID uint32, request []byte) error {
@@ -142,7 +136,8 @@ func TestStateSyncToggleEnabledToDisabled(t *testing.T) {
 		go vmSetup.serverVM.AppRequest(ctx, nodeID, requestID, time.Now().Add(1*time.Second), request)
 		return nil
 	}
-	// Disable metrics to prevent duplicate registerer
+	// Reset metrics to allow re-initialization
+	vmSetup.syncerVM.ctx.Metrics = metrics.NewPrefixGatherer()
 	stateSyncDisabledConfigJSON := `{"state-sync-enabled":false}`
 	if err := syncDisabledVM.Initialize(
 		context.Background(),
@@ -201,14 +196,14 @@ func TestStateSyncToggleEnabledToDisabled(t *testing.T) {
 	syncDisabledVM.blockChain.DrainAcceptorQueue()
 
 	// Create a new VM from the same database with state sync enabled.
-	syncReEnabledVM := &VM{
-		config: config.Config{MetricsEnabled: pointerTo(false)},
-	}
+	syncReEnabledVM := &VM{}
 	// Enable state sync in configJSON
 	configJSON := fmt.Sprintf(
 		`{"state-sync-enabled":true, "state-sync-min-blocks":%d}`,
 		test.stateSyncMinBlocks,
 	)
+	// Reset metrics to allow re-initialization
+	vmSetup.syncerVM.ctx.Metrics = metrics.NewPrefixGatherer()
 	if err := syncReEnabledVM.Initialize(
 		context.Background(),
 		vmSetup.syncerVM.ctx,

plugin/evm/vm.go

@@ -24,7 +24,6 @@ import (
 	"github.com/ava-labs/avalanchego/upgrade"
 	avalanchegoConstants "github.com/ava-labs/avalanchego/utils/constants"
 	"github.com/prometheus/client_golang/prometheus"
-	dto "github.com/prometheus/client_model/go"
 
 	"github.com/ava-labs/coreth/consensus/dummy"
 	"github.com/ava-labs/coreth/constants"
@@ -394,13 +393,13 @@ func (vm *VM) Initialize(
 	vm.toEngine = toEngine
 	vm.shutdownChan = make(chan struct{}, 1)
 
-	if *vm.config.MetricsEnabled {
+	if vm.config.MetricsEnabled {
 		if err := vm.initializeMetrics(); err != nil {
 			return fmt.Errorf("failed to initialize metrics: %w", err)
 		}
 	} else {
 		metrics.Enabled = false // reset global variable to false for tests
-		vm.sdkMetrics = &noopPrometheusRegister{}
+		vm.sdkMetrics = &corethprometheus.NoopRegister{}
 	}
 
 	// Initialize the database
@@ -648,13 +647,6 @@ func (vm *VM) initializeMetrics() error {
 	return vm.ctx.Metrics.Register(sdkMetricsPrefix, vm.sdkMetrics)
 }
 
-type noopPrometheusRegister struct{}
-
-func (n *noopPrometheusRegister) Register(prometheus.Collector) error  { return nil }
-func (n *noopPrometheusRegister) MustRegister(...prometheus.Collector) {}
-func (n *noopPrometheusRegister) Unregister(prometheus.Collector) bool { return true }
-func (n *noopPrometheusRegister) Gather() ([]*dto.MetricFamily, error) { return nil, nil }
-
 func (vm *VM) initializeChain(lastAcceptedHash common.Hash) error {
 	nodecfg := &node.Config{
 		CorethVersion:         Version,

plugin/evm/vm_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/ava-labs/avalanchego/api/keystore"
+	"github.com/ava-labs/avalanchego/api/metrics"
 	avalancheatomic "github.com/ava-labs/avalanchego/chains/atomic"
 	"github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/database/memdb"
@@ -305,13 +306,7 @@ func GenesisVMWithClock(
 	*avalancheatomic.Memory,
 	*enginetest.Sender,
 ) {
-	vmConfig := config.Config{
-		MetricsEnabled: pointerTo(false),
-	}
-	vm := &VM{
-		clock:  clock,
-		config: vmConfig,
-	}
+	vm := &VM{clock: clock}
 	ctx, dbManager, genesisBytes, issuer, m := setupGenesis(t, genesisJSON)
 	appSender := &enginetest.Sender{T: t}
 	appSender.CantSendAppGossip = true
@@ -407,7 +402,6 @@ func TestVMConfigDefaults(t *testing.T) {
 	_, vm, _, _, _ := GenesisVM(t, false, "", configJSON, "")
 
 	var vmConfig config.Config
-	vmConfig.MetricsEnabled = pointerTo(false) // GenesisVM sets it to false
 	vmConfig.SetDefaults(defaultTxPoolConfig)
 	vmConfig.RPCTxFeeCap = txFeeCap
 	vmConfig.EnabledEthAPIs = enabledEthAPIs
@@ -420,7 +414,6 @@ func TestVMNilConfig(t *testing.T) {
 
 	// VM Config should match defaults if no config is passed in
 	var vmConfig config.Config
-	vmConfig.MetricsEnabled = pointerTo(false) // GenesisVM sets it to false
 	vmConfig.SetDefaults(defaultTxPoolConfig)
 	require.Equal(t, vmConfig, vm.config, "VM Config should match default config")
 	require.NoError(t, vm.Shutdown(context.Background()))
@@ -3806,12 +3799,7 @@ func TestSkipChainConfigCheckCompatible(t *testing.T) {
 	require.NoError(t, vm.SetPreference(context.Background(), blk.ID()))
 	require.NoError(t, blk.Accept(context.Background()))
 
-	reinitVM := &VM{
-		// Hack: registering metrics uses global variables, so we need to disable metrics so that we can initialize the VM twice.
-		config: config.Config{
-			MetricsEnabled: pointerTo(false),
-		},
-	}
+	reinitVM := &VM{}
 	// use the block's timestamp instead of 0 since rewind to genesis
 	// is hardcoded to be allowed in core/genesis.go.
 	genesisWithUpgrade := &core.Genesis{}
@@ -3820,12 +3808,18 @@ func TestSkipChainConfigCheckCompatible(t *testing.T) {
 	genesisWithUpgradeBytes, err := json.Marshal(genesisWithUpgrade)
 	require.NoError(t, err)
 
+	// Reset metrics to allow re-initialization
+	vm.ctx.Metrics = metrics.NewPrefixGatherer()
+
 	// this will not be allowed
 	err = reinitVM.Initialize(context.Background(), vm.ctx, dbManager, genesisWithUpgradeBytes, []byte{}, []byte{}, issuer, []*commonEng.Fx{}, appSender)
 	require.ErrorContains(t, err, "mismatching ApricotPhase2 fork block timestamp in database")
 
+	// Reset metrics to allow re-initialization
+	vm.ctx.Metrics = metrics.NewPrefixGatherer()
+
 	// try again with skip-upgrade-check
-	config := []byte(`{"skip-upgrade-check": true, "metrics-enabled": false}`)
+	config := []byte(`{"skip-upgrade-check": true}`)
 	err = reinitVM.Initialize(context.Background(), vm.ctx, dbManager, genesisWithUpgradeBytes, []byte{}, config, issuer, []*commonEng.Fx{}, appSender)
 	require.NoError(t, err)
 	require.NoError(t, reinitVM.Shutdown(context.Background()))