cmd, core, metrics: always report expensive metrics

cmd/geth/config.go

@@ -267,7 +267,7 @@ func applyMetricConfig(ctx *cli.Context, cfg *gethConfig) {
 		cfg.Metrics.Enabled = ctx.Bool(utils.MetricsEnabledFlag.Name)
 	}
 	if ctx.IsSet(utils.MetricsEnabledExpensiveFlag.Name) {
-		cfg.Metrics.EnabledExpensive = ctx.Bool(utils.MetricsEnabledExpensiveFlag.Name)
+		log.Warn("Expensive metrics are collected by default, please remove this flag", "flag", utils.MetricsEnabledExpensiveFlag.Name)
 	}
 	if ctx.IsSet(utils.MetricsHTTPFlag.Name) {
 		cfg.Metrics.HTTP = ctx.String(utils.MetricsHTTPFlag.Name)

cmd/utils/flags.go

@@ -862,12 +862,6 @@ var (
 		Usage:    "Enable metrics collection and reporting",
 		Category: flags.MetricsCategory,
 	}
-	MetricsEnabledExpensiveFlag = &cli.BoolFlag{
-		Name:     "metrics.expensive",
-		Usage:    "Enable expensive metrics collection and reporting",
-		Category: flags.MetricsCategory,
-	}
-
 	// MetricsHTTPFlag defines the endpoint for a stand-alone metrics HTTP endpoint.
 	// Since the pprof service enables sensitive/vulnerable behavior, this allows a user
 	// to enable a public-OK metrics endpoint without having to worry about ALSO exposing

cmd/utils/flags_legacy.go

@@ -93,35 +93,35 @@ var (
 		Name:     "light.serve",
 		Usage:    "Maximum percentage of time allowed for serving LES requests (deprecated)",
 		Value:    ethconfig.Defaults.LightServ,
-		Category: flags.LightCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	LightIngressFlag = &cli.IntFlag{
 		Name:     "light.ingress",
 		Usage:    "Incoming bandwidth limit for serving light clients (deprecated)",
 		Value:    ethconfig.Defaults.LightIngress,
-		Category: flags.LightCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	LightEgressFlag = &cli.IntFlag{
 		Name:     "light.egress",
 		Usage:    "Outgoing bandwidth limit for serving light clients (deprecated)",
 		Value:    ethconfig.Defaults.LightEgress,
-		Category: flags.LightCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	LightMaxPeersFlag = &cli.IntFlag{
 		Name:     "light.maxpeers",
 		Usage:    "Maximum number of light clients to serve, or light servers to attach to (deprecated)",
 		Value:    ethconfig.Defaults.LightPeers,
-		Category: flags.LightCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	LightNoPruneFlag = &cli.BoolFlag{
 		Name:     "light.nopruning",
 		Usage:    "Disable ancient light chain data pruning (deprecated)",
-		Category: flags.LightCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	LightNoSyncServeFlag = &cli.BoolFlag{
 		Name:     "light.nosyncserve",
 		Usage:    "Enables serving light clients before syncing (deprecated)",
-		Category: flags.LightCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	// Deprecated November 2023
 	LogBacktraceAtFlag = &cli.StringFlag{
@@ -138,19 +138,24 @@ var (
 	// Deprecated February 2024
 	MinerNewPayloadTimeoutFlag = &cli.DurationFlag{
 		Name:     "miner.newpayload-timeout",
-		Usage:    "Specify the maximum time allowance for creating a new payload",
+		Usage:    "Specify the maximum time allowance for creating a new payload (deprecated)",
 		Value:    ethconfig.Defaults.Miner.Recommit,
-		Category: flags.MinerCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	MinerEtherbaseFlag = &cli.StringFlag{
 		Name:     "miner.etherbase",
-		Usage:    "0x prefixed public address for block mining rewards",
+		Usage:    "0x prefixed public address for block mining rewards (deprecated)",
-		Category: flags.MinerCategory,
+		Category: flags.DeprecatedCategory,
 	}
 	MiningEnabledFlag = &cli.BoolFlag{
 		Name:     "mine",
-		Usage:    "Enable mining",
+		Usage:    "Enable mining (deprecated)",
-		Category: flags.MinerCategory,
+		Category: flags.DeprecatedCategory,
+	}
+	MetricsEnabledExpensiveFlag = &cli.BoolFlag{
+		Name:     "metrics.expensive",
+		Usage:    "Enable expensive metrics collection and reporting (deprecated)",
+		Category: flags.DeprecatedCategory,
 	}
 )
 

core/state/state_object.go

@@ -25,7 +25,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
-	"github.com/ethereum/go-ethereum/metrics"
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/ethereum/go-ethereum/trie/trienode"
 	"github.com/holiman/uint256"
@@ -197,9 +196,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
 	if s.db.snap != nil {
 		start := time.Now()
 		enc, err = s.db.snap.Storage(s.addrHash, crypto.Keccak256Hash(key.Bytes()))
-		if metrics.EnabledExpensive {
+		s.db.SnapshotStorageReads += time.Since(start)
-			s.db.SnapshotStorageReads += time.Since(start)
+
-		}
 		if len(enc) > 0 {
 			_, content, _, err := rlp.Split(enc)
 			if err != nil {
@@ -217,9 +215,8 @@ func (s *stateObject) GetCommittedState(key common.Hash) common.Hash {
 			return common.Hash{}
 		}
 		val, err := tr.GetStorage(s.address, key.Bytes())
-		if metrics.EnabledExpensive {
+		s.db.StorageReads += time.Since(start)
-			s.db.StorageReads += time.Since(start)
+
-		}
 		if err != nil {
 			s.db.setError(err)
 			return common.Hash{}
@@ -283,9 +280,8 @@ func (s *stateObject) updateTrie() (Trie, error) {
 		return s.trie, nil
 	}
 	// Track the amount of time wasted on updating the storage trie
-	if metrics.EnabledExpensive {
+	defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())
-		defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())
+
-	}
 	// The snapshot storage map for the object
 	var (
 		storage map[common.Hash][]byte
@@ -370,9 +366,8 @@ func (s *stateObject) updateRoot() {
 		return
 	}
 	// Track the amount of time wasted on hashing the storage trie
-	if metrics.EnabledExpensive {
+	defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
-		defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
+
-	}
 	s.data.Root = tr.Hash()
 }
 
@@ -386,9 +381,8 @@ func (s *stateObject) commit() (*trienode.NodeSet, error) {
 		return nil, nil
 	}
 	// Track the amount of time wasted on committing the storage trie
-	if metrics.EnabledExpensive {
+	defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
-		defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
+
-	}
 	// The trie is currently in an open state and could potentially contain
 	// cached mutations. Call commit to acquire a set of nodes that have been
 	// modified, the set can be nil if nothing to commit.

core/state/statedb.go

@@ -28,7 +28,6 @@ import (
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/log"
-	"github.com/ethereum/go-ethereum/metrics"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/trie"
 	"github.com/ethereum/go-ethereum/trie/trienode"
@@ -495,9 +494,8 @@ func (s *StateDB) GetTransientState(addr common.Address, key common.Hash) common
 // updateStateObject writes the given object to the trie.
 func (s *StateDB) updateStateObject(obj *stateObject) {
 	// Track the amount of time wasted on updating the account from the trie
-	if metrics.EnabledExpensive {
+	defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
-		defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
+
-	}
 	// Encode the account and update the account trie
 	addr := obj.Address()
 	if err := s.trie.UpdateAccount(addr, &obj.data); err != nil {
@@ -527,9 +525,8 @@ func (s *StateDB) updateStateObject(obj *stateObject) {
 // deleteStateObject removes the given object from the state trie.
 func (s *StateDB) deleteStateObject(obj *stateObject) {
 	// Track the amount of time wasted on deleting the account from the trie
-	if metrics.EnabledExpensive {
+	defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
-		defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now())
+
-	}
 	// Delete the account from the trie
 	addr := obj.Address()
 	if err := s.trie.DeleteAccount(addr); err != nil {
@@ -561,9 +558,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
 	if s.snap != nil {
 		start := time.Now()
 		acc, err := s.snap.Account(crypto.HashData(s.hasher, addr.Bytes()))
-		if metrics.EnabledExpensive {
+		s.SnapshotAccountReads += time.Since(start)
-			s.SnapshotAccountReads += time.Since(start)
+
-		}
 		if err == nil {
 			if acc == nil {
 				return nil
@@ -587,9 +583,8 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject {
 		start := time.Now()
 		var err error
 		data, err = s.trie.GetAccount(addr)
-		if metrics.EnabledExpensive {
+		s.AccountReads += time.Since(start)
-			s.AccountReads += time.Since(start)
+
-		}
 		if err != nil {
 			s.setError(fmt.Errorf("getDeleteStateObject (%x) error: %w", addr.Bytes(), err))
 			return nil
@@ -917,9 +912,8 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
 		s.stateObjectsPending = make(map[common.Address]struct{})
 	}
 	// Track the amount of time wasted on hashing the account trie
-	if metrics.EnabledExpensive {
+	defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now())
-		defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now())
+
-	}
 	return s.trie.Hash()
 }
 
@@ -1042,16 +1036,16 @@ func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root
 	if err != nil {
 		return nil, nil, err
 	}
-	if metrics.EnabledExpensive {
+	// Report the metrics
-		n := int64(len(slots))
+	n := int64(len(slots))
 
-		slotDeletionMaxCount.UpdateIfGt(int64(len(slots)))
+	slotDeletionMaxCount.UpdateIfGt(int64(len(slots)))
-		slotDeletionMaxSize.UpdateIfGt(int64(size))
+	slotDeletionMaxSize.UpdateIfGt(int64(size))
+
+	slotDeletionTimer.UpdateSince(start)
+	slotDeletionCount.Mark(n)
+	slotDeletionSize.Mark(int64(size))
 
-		slotDeletionTimer.UpdateSince(start)
-		slotDeletionCount.Mark(n)
-		slotDeletionSize.Mark(int64(size))
-	}
 	return slots, nodes, nil
 }
 
@@ -1190,10 +1184,8 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
 		}
 	}
 	// Write the account trie changes, measuring the amount of wasted time
-	var start time.Time
+	start := time.Now()
-	if metrics.EnabledExpensive {
+
-		start = time.Now()
-	}
 	root, set, err := s.trie.Commit(true)
 	if err != nil {
 		return common.Hash{}, err
@@ -1205,23 +1197,23 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
 		}
 		accountTrieNodesUpdated, accountTrieNodesDeleted = set.Size()
 	}
-	if metrics.EnabledExpensive {
+	// Report the commit metrics
-		s.AccountCommits += time.Since(start)
+	s.AccountCommits += time.Since(start)
+
+	accountUpdatedMeter.Mark(int64(s.AccountUpdated))
+	storageUpdatedMeter.Mark(int64(s.StorageUpdated))
+	accountDeletedMeter.Mark(int64(s.AccountDeleted))
+	storageDeletedMeter.Mark(int64(s.StorageDeleted))
+	accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
+	accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
+	storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
+	storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
+	s.AccountUpdated, s.AccountDeleted = 0, 0
+	s.StorageUpdated, s.StorageDeleted = 0, 0
 
-		accountUpdatedMeter.Mark(int64(s.AccountUpdated))
-		storageUpdatedMeter.Mark(int64(s.StorageUpdated))
-		accountDeletedMeter.Mark(int64(s.AccountDeleted))
-		storageDeletedMeter.Mark(int64(s.StorageDeleted))
-		accountTrieUpdatedMeter.Mark(int64(accountTrieNodesUpdated))
-		accountTrieDeletedMeter.Mark(int64(accountTrieNodesDeleted))
-		storageTriesUpdatedMeter.Mark(int64(storageTrieNodesUpdated))
-		storageTriesDeletedMeter.Mark(int64(storageTrieNodesDeleted))
-		s.AccountUpdated, s.AccountDeleted = 0, 0
-		s.StorageUpdated, s.StorageDeleted = 0, 0
-	}
 	// If snapshotting is enabled, update the snapshot tree with this new version
 	if s.snap != nil {
-		start := time.Now()
+		start = time.Now()
 		// Only update if there's a state transition (skip empty Clique blocks)
 		if parent := s.snap.Root(); parent != root {
 			if err := s.snaps.Update(root, parent, s.convertAccountSet(s.stateObjectsDestruct), s.accounts, s.storages); err != nil {
@@ -1235,9 +1227,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
 				log.Warn("Failed to cap snapshot tree", "root", root, "layers", 128, "err", err)
 			}
 		}
-		if metrics.EnabledExpensive {
+		s.SnapshotCommits += time.Since(start)
-			s.SnapshotCommits += time.Since(start)
-		}
 		s.snap = nil
 	}
 	if root == (common.Hash{}) {
@@ -1248,15 +1238,14 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
 		origin = types.EmptyRootHash
 	}
 	if root != origin {
-		start := time.Now()
+		start = time.Now()
 		set := triestate.New(s.accountsOrigin, s.storagesOrigin)
 		if err := s.db.TrieDB().Update(root, origin, block, nodes, set); err != nil {
 			return common.Hash{}, err
 		}
 		s.originalRoot = root
-		if metrics.EnabledExpensive {
+		s.TrieDBCommits += time.Since(start)
-			s.TrieDBCommits += time.Since(start)
+
-		}
 		if s.onCommit != nil {
 			s.onCommit(set)
 		}

internal/flags/categories.go

@@ -21,7 +21,6 @@ import "github.com/urfave/cli/v2"
 const (
 	EthCategory        = "ETHEREUM"
 	BeaconCategory     = "BEACON CHAIN"
-	LightCategory      = "LIGHT CLIENT"
 	DevCategory        = "DEVELOPER CHAIN"
 	StateCategory      = "STATE HISTORY MANAGEMENT"
 	TxPoolCategory     = "TRANSACTION POOL (EVM)"

metrics/config.go

@@ -19,7 +19,7 @@ package metrics
 // Config contains the configuration for the metric collection.
 type Config struct {
 	Enabled          bool   `toml:",omitempty"`
-	EnabledExpensive bool   `toml:",omitempty"`
+	EnabledExpensive bool   `toml:"-"`
 	HTTP             string `toml:",omitempty"`
 	Port             int    `toml:",omitempty"`
 	EnableInfluxDB   bool   `toml:",omitempty"`

metrics/metrics.go

@@ -24,23 +24,12 @@ import (
 // for less cluttered pprof profiles.
 var Enabled = false
 
-// EnabledExpensive is a soft-flag meant for external packages to check if costly
-// metrics gathering is allowed or not. The goal is to separate standard metrics
-// for health monitoring and debug metrics that might impact runtime performance.
-var EnabledExpensive = false
-
 // enablerFlags is the CLI flag names to use to enable metrics collections.
 var enablerFlags = []string{"metrics"}
 
 // enablerEnvVars is the env var names to use to enable metrics collections.
 var enablerEnvVars = []string{"GETH_METRICS"}
 
-// expensiveEnablerFlags is the CLI flag names to use to enable metrics collections.
-var expensiveEnablerFlags = []string{"metrics.expensive"}
-
-// expensiveEnablerEnvVars is the env var names to use to enable metrics collections.
-var expensiveEnablerEnvVars = []string{"GETH_METRICS_EXPENSIVE"}
-
 // Init enables or disables the metrics system. Since we need this to run before
 // any other code gets to create meters and timers, we'll actually do an ugly hack
 // and peek into the command line args for the metrics flag.
@@ -53,14 +42,6 @@ func init() {
 			}
 		}
 	}
-	for _, enabler := range expensiveEnablerEnvVars {
-		if val, found := syscall.Getenv(enabler); found && !EnabledExpensive {
-			if enable, _ := strconv.ParseBool(val); enable { // ignore error, flag parser will choke on it later
-				log.Info("Enabling expensive metrics collection")
-				EnabledExpensive = true
-			}
-		}
-	}
 	for _, arg := range os.Args {
 		flag := strings.TrimLeft(arg, "-")
 
@@ -70,12 +51,6 @@ func init() {
 				Enabled = true
 			}
 		}
-		for _, enabler := range expensiveEnablerFlags {
-			if !EnabledExpensive && flag == enabler {
-				log.Info("Enabling expensive metrics collection")
-				EnabledExpensive = true
-			}
-		}
 	}
 }