diff --git a/metrics/prometheus/prometheus.go b/metrics/prometheus/prometheus.go index 772260a095..226fd78e7f 100644 --- a/metrics/prometheus/prometheus.go +++ b/metrics/prometheus/prometheus.go @@ -16,10 +16,6 @@ import ( dto "github.com/prometheus/client_model/go" ) -func init() { - metrics.Enabled = true -} - type Gatherer struct { registry Registry } diff --git a/plugin/evm/config/config.go b/plugin/evm/config/config.go index f5bf644657..306562570d 100644 --- a/plugin/evm/config/config.go +++ b/plugin/evm/config/config.go @@ -27,6 +27,7 @@ const ( defaultSnapshotWait = false defaultRpcGasCap = 50_000_000 // Default to 50M Gas Limit defaultRpcTxFeeCap = 100 // 100 AVAX + defaultMetricsEnabled = true defaultMetricsExpensiveEnabled = true defaultApiMaxDuration = 0 // Default to no maximum API call duration defaultWsCpuRefillRate = 0 // Default to no maximum WS CPU usage @@ -125,7 +126,8 @@ type Config struct { PruneWarpDB bool `json:"prune-warp-db-enabled"` // Determines if the warpDB should be cleared on startup // Metric Settings - 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"` @@ -243,6 +245,7 @@ func (c *Config) SetDefaults(txPoolConfig TxPoolConfig) { c.EnabledEthAPIs = defaultEnabledAPIs c.RPCGasCap = defaultRpcGasCap c.RPCTxFeeCap = defaultRpcTxFeeCap + c.MetricsEnabled = defaultPointer(c.MetricsEnabled, defaultMetricsEnabled) c.MetricsExpensiveEnabled = defaultMetricsExpensiveEnabled // TxPool settings @@ -290,6 +293,13 @@ 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 ba0d05ac38..befc6a7ec2 100644 --- a/plugin/evm/syncervm_test.go +++ b/plugin/evm/syncervm_test.go @@ -35,6 +35,7 @@ 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" @@ -44,7 +45,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/rlp" ) @@ -85,13 +85,10 @@ 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) - // Hack: registering metrics uses global variables, so we need to disable metrics here so that we can initialize the VM twice. - metrics.Enabled = false - defer func() { - metrics.Enabled = true - }() var lock sync.Mutex reqCount := 0 @@ -130,7 +127,11 @@ func TestStateSyncToggleEnabledToDisabled(t *testing.T) { test.responseIntercept = nil test.expectedErr = nil - syncDisabledVM := &VM{} + syncDisabledVM := &VM{ + config: config.Config{ + MetricsEnabled: pointerTo(false), + }, + } 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 { @@ -200,7 +201,9 @@ func TestStateSyncToggleEnabledToDisabled(t *testing.T) { syncDisabledVM.blockChain.DrainAcceptorQueue() // Create a new VM from the same database with state sync enabled. - syncReEnabledVM := &VM{} + syncReEnabledVM := &VM{ + config: config.Config{MetricsEnabled: pointerTo(false)}, + } // Enable state sync in configJSON configJSON := fmt.Sprintf( `{"state-sync-enabled":true, "state-sync-min-blocks":%d}`, diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index e7770c1e5a..6becfc57b4 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -24,6 +24,7 @@ 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" @@ -279,7 +280,10 @@ type VM struct { p2pValidators *p2p.Validators // Metrics - sdkMetrics *prometheus.Registry + sdkMetrics interface { + prometheus.Registerer + prometheus.Gatherer + } bootstrapped avalancheUtils.Atomic[bool] IsPlugin bool @@ -390,8 +394,13 @@ func (vm *VM) Initialize( vm.toEngine = toEngine vm.shutdownChan = make(chan struct{}, 1) - if err := vm.initializeMetrics(); err != nil { - return fmt.Errorf("failed to initialize metrics: %w", err) + 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{} } // Initialize the database @@ -630,12 +639,8 @@ func (vm *VM) Initialize( } func (vm *VM) initializeMetrics() error { + metrics.Enabled = true vm.sdkMetrics = prometheus.NewRegistry() - // If metrics are enabled, register the default metrics registry - if !metrics.Enabled { - return nil - } - gatherer := corethprometheus.NewGatherer(metrics.DefaultRegistry) if err := vm.ctx.Metrics.Register(ethMetricsPrefix, gatherer); err != nil { return err @@ -643,6 +648,13 @@ 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 28796ab8cc..d5eeea611f 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -27,7 +27,6 @@ import ( "github.com/ava-labs/coreth/plugin/evm/config" "github.com/ava-labs/coreth/trie" "github.com/ava-labs/coreth/utils" - "github.com/ethereum/go-ethereum/metrics" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -306,7 +305,13 @@ func GenesisVMWithClock( *avalancheatomic.Memory, *enginetest.Sender, ) { - vm := &VM{clock: clock} + vmConfig := config.Config{ + MetricsEnabled: pointerTo(false), + } + vm := &VM{ + clock: clock, + config: vmConfig, + } ctx, dbManager, genesisBytes, issuer, m := setupGenesis(t, genesisJSON) appSender := &enginetest.Sender{T: t} appSender.CantSendAppGossip = true @@ -402,6 +407,7 @@ 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 @@ -414,6 +420,7 @@ 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())) @@ -3780,10 +3787,6 @@ func TestExtraStateChangeAtomicGasLimitExceeded(t *testing.T) { } func TestSkipChainConfigCheckCompatible(t *testing.T) { - // Hack: registering metrics uses global variables, so we need to disable metrics here so that we can initialize the VM twice. - metrics.Enabled = false - defer func() { metrics.Enabled = true }() - importAmount := uint64(50000000) issuer, vm, dbManager, _, appSender := GenesisVMWithUTXOs(t, true, genesisJSONApricotPhase1, "", "", map[ids.ShortID]uint64{ testShortIDAddrs[0]: importAmount, @@ -3803,7 +3806,12 @@ func TestSkipChainConfigCheckCompatible(t *testing.T) { require.NoError(t, vm.SetPreference(context.Background(), blk.ID())) require.NoError(t, blk.Accept(context.Background())) - reinitVM := &VM{} + 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), + }, + } // use the block's timestamp instead of 0 since rewind to genesis // is hardcoded to be allowed in core/genesis.go. genesisWithUpgrade := &core.Genesis{} @@ -3817,7 +3825,7 @@ func TestSkipChainConfigCheckCompatible(t *testing.T) { require.ErrorContains(t, err, "mismatching ApricotPhase2 fork block timestamp in database") // try again with skip-upgrade-check - config := []byte(`{"skip-upgrade-check": true}`) + config := []byte(`{"skip-upgrade-check": true, "metrics-enabled": false}`) 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()))