diff --git a/metrics/prometheus/noop_register.go b/metrics/prometheus/noop_register.go new file mode 100644 index 0000000000..873abcbb64 --- /dev/null +++ b/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 } diff --git a/metrics/prometheus/prometheus.go b/metrics/prometheus/prometheus.go index 226fd78e7f..8120348e8d 100644 --- a/metrics/prometheus/prometheus.go +++ b/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) } diff --git a/plugin/evm/config/config.go b/plugin/evm/config/config.go index 306562570d..f7c97ddee1 100644 --- a/plugin/evm/config/config.go +++ b/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 { diff --git a/plugin/evm/syncervm_test.go b/plugin/evm/syncervm_test.go index befc6a7ec2..4e42bfac7e 100644 --- a/plugin/evm/syncervm_test.go +++ b/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, diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 6becfc57b4..0c78520b56 100644 --- a/plugin/evm/vm.go +++ b/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, diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index d5eeea611f..18dc89f482 100644 --- a/plugin/evm/vm_test.go +++ b/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()))