From 13d4adbbd98f2ea41ce00023738eb464871168fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 25 Mar 2019 09:45:50 +0200 Subject: [PATCH] cmd, core, metrics: support expensive optional metrics --- cmd/geth/main.go | 3 ++- cmd/geth/usage.go | 12 ++---------- cmd/utils/flags.go | 8 ++++++-- core/state/state_object.go | 21 ++++++++++++++------- core/state/statedb.go | 25 ++++++++++++++++--------- metrics/metrics.go | 31 ++++++++++++++++++++++++------- 6 files changed, 64 insertions(+), 36 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index f966905a92..83847fc533 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -137,7 +137,6 @@ var ( utils.RPCCORSDomainFlag, utils.RPCVirtualHostsFlag, utils.EthStatsURLFlag, - utils.MetricsEnabledFlag, utils.FakePoWFlag, utils.NoCompactionFlag, utils.GpoBlocksFlag, @@ -174,6 +173,8 @@ var ( } metricsFlags = []cli.Flag{ + utils.MetricsEnabledFlag, + utils.MetricsEnabledExpensiveFlag, utils.MetricsEnableInfluxDBFlag, utils.MetricsInfluxDBEndpointFlag, utils.MetricsInfluxDBDatabaseFlag, diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index a4787fff2b..8658a3fdcf 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -225,16 +225,8 @@ var AppHelpFlagGroups = []flagGroup{ }, debug.Flags...), }, { - Name: "METRICS AND STATS", - Flags: []cli.Flag{ - utils.MetricsEnabledFlag, - utils.MetricsEnableInfluxDBFlag, - utils.MetricsInfluxDBEndpointFlag, - utils.MetricsInfluxDBDatabaseFlag, - utils.MetricsInfluxDBUsernameFlag, - utils.MetricsInfluxDBPasswordFlag, - utils.MetricsInfluxDBTagsFlag, - }, + Name: "METRICS AND STATS", + Flags: metricsFlags, }, { Name: "WHISPER (EXPERIMENTAL)", diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index e00f92fa75..bf433242ec 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -226,7 +226,7 @@ var ( } // Dashboard settings DashboardEnabledFlag = cli.BoolFlag{ - Name: metrics.DashboardEnabledFlag, + Name: "dashboard", Usage: "Enable the dashboard", } DashboardAddrFlag = cli.StringFlag{ @@ -644,9 +644,13 @@ var ( // Metrics flags MetricsEnabledFlag = cli.BoolFlag{ - Name: metrics.MetricsEnabledFlag, + Name: "metrics", Usage: "Enable metrics collection and reporting", } + MetricsEnabledExpensiveFlag = cli.BoolFlag{ + Name: "metrics.expensive", + Usage: "Enable expensive metrics collection and reporting", + } MetricsEnableInfluxDBFlag = cli.BoolFlag{ Name: "metrics.influxdb", Usage: "Enable metrics export/push to an external InfluxDB database", diff --git a/core/state/state_object.go b/core/state/state_object.go index 15e76bd01a..7fbb45b3df 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -25,6 +25,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/rlp" ) @@ -179,8 +180,9 @@ func (self *stateObject) GetCommittedState(db Database, key common.Hash) common. return value } // Track the amount of time wasted on reading the storge trie - defer func(start time.Time) { self.db.StorageReads += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { self.db.StorageReads += time.Since(start) }(time.Now()) + } // Otherwise load the value from the database enc, err := self.getTrie(db).TryGet(key[:]) if err != nil { @@ -221,8 +223,10 @@ func (self *stateObject) setState(key, value common.Hash) { // updateTrie writes cached storage modifications into the object's storage trie. func (self *stateObject) updateTrie(db Database) Trie { // Track the amount of time wasted on updating the storge trie - defer func(start time.Time) { self.db.StorageUpdates += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { self.db.StorageUpdates += time.Since(start) }(time.Now()) + } + // Update all the dirty slots in the trie tr := self.getTrie(db) for key, value := range self.dirtyStorage { delete(self.dirtyStorage, key) @@ -249,7 +253,9 @@ func (self *stateObject) updateRoot(db Database) { self.updateTrie(db) // Track the amount of time wasted on hashing the storge trie - defer func(start time.Time) { self.db.StorageHashes += time.Since(start) }(time.Now()) + if metrics.EnabledExpensive { + defer func(start time.Time) { self.db.StorageHashes += time.Since(start) }(time.Now()) + } self.data.Root = self.trie.Hash() } @@ -261,8 +267,9 @@ func (self *stateObject) CommitTrie(db Database) error { return self.dbErr } // Track the amount of time wasted on committing the storge trie - defer func(start time.Time) { self.db.StorageCommits += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { self.db.StorageCommits += time.Since(start) }(time.Now()) + } root, err := self.trie.Commit(nil) if err == nil { self.data.Root = root diff --git a/core/state/statedb.go b/core/state/statedb.go index 20a1f0128e..0673de543f 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -28,6 +28,7 @@ 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/rlp" "github.com/ethereum/go-ethereum/trie" ) @@ -399,8 +400,9 @@ func (self *StateDB) Suicide(addr common.Address) bool { // updateStateObject writes the given object to the trie. func (s *StateDB) updateStateObject(stateObject *stateObject) { // Track the amount of time wasted on updating the account from the trie - defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) + } // Encode the account and update the account trie addr := stateObject.Address() @@ -414,8 +416,9 @@ func (s *StateDB) updateStateObject(stateObject *stateObject) { // deleteStateObject removes the given object from the state trie. func (s *StateDB) deleteStateObject(stateObject *stateObject) { // Track the amount of time wasted on deleting the account from the trie - defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { s.AccountUpdates += time.Since(start) }(time.Now()) + } // Delete the account from the trie stateObject.deleted = true @@ -433,8 +436,9 @@ func (s *StateDB) getStateObject(addr common.Address) (stateObject *stateObject) return obj } // Track the amount of time wasted on loading the object from the database - defer func(start time.Time) { s.AccountReads += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { s.AccountReads += time.Since(start) }(time.Now()) + } // Load the object from the database enc, err := s.trie.TryGet(addr[:]) if len(enc) == 0 { @@ -625,7 +629,9 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { s.Finalise(deleteEmptyObjects) // Track the amount of time wasted on hashing the account trie - defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) + if metrics.EnabledExpensive { + defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) + } return s.trie.Hash() } @@ -674,8 +680,9 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) delete(s.stateObjectsDirty, addr) } // Write the account trie changes, measuing the amount of wasted time - defer func(start time.Time) { s.AccountCommits += time.Since(start) }(time.Now()) - + if metrics.EnabledExpensive { + defer func(start time.Time) { s.AccountCommits += time.Since(start) }(time.Now()) + } root, err = s.trie.Commit(func(leaf []byte, parent common.Hash) error { var account Account if err := rlp.DecodeBytes(leaf, &account); err != nil { diff --git a/metrics/metrics.go b/metrics/metrics.go index 5910fb0734..6d14c8ecf1 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -15,24 +15,41 @@ import ( ) // Enabled is checked by the constructor functions for all of the -// standard metrics. If it is true, the metric returned is a stub. +// standard metrics. If it is true, the metric returned is a stub. // // This global kill-switch helps quantify the observer effect and makes // for less cluttered pprof profiles. var Enabled = false -// MetricsEnabledFlag is the CLI flag name to use to enable metrics collections. -const MetricsEnabledFlag = "metrics" -const DashboardEnabledFlag = "dashboard" +// 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", "dashboard"} + +// expensiveEnablerFlags is the CLI flag names to use to enable metrics collections. +var expensiveEnablerFlags = []string{"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. func init() { for _, arg := range os.Args { - if flag := strings.TrimLeft(arg, "-"); flag == MetricsEnabledFlag || flag == DashboardEnabledFlag { - log.Info("Enabling metrics collection") - Enabled = true + flag := strings.TrimLeft(arg, "-") + + for _, enabler := range enablerFlags { + if !Enabled && flag == enabler { + log.Info("Enabling metrics collection") + Enabled = true + } + } + for _, enabler := range expensiveEnablerFlags { + if !Enabled && flag == enabler { + log.Info("Enabling expensive metrics collection") + EnabledExpensive = true + } } } }