Skip to content

Commit

Permalink
Only set metrics.Enabled in VM.Initalize
Browse files Browse the repository at this point in the history
  • Loading branch information
qdm12 committed Jan 14, 2025
1 parent 928d299 commit 3e3d44c
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 29 deletions.
4 changes: 0 additions & 4 deletions metrics/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ import (
dto "github.com/prometheus/client_model/go"
)

func init() {
metrics.Enabled = true
}

type Gatherer struct {
registry Registry
}
Expand Down
12 changes: 11 additions & 1 deletion plugin/evm/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"`
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 11 additions & 8 deletions plugin/evm/syncervm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}`,
Expand Down
28 changes: 20 additions & 8 deletions plugin/evm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -630,19 +639,22 @@ 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
}
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,
Expand Down
24 changes: 16 additions & 8 deletions plugin/evm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()))
Expand Down Expand Up @@ -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,
Expand All @@ -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{}
Expand All @@ -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()))
Expand Down

0 comments on commit 3e3d44c

Please sign in to comment.