From 205d8b8062d3f17b396ff7741a20e9c9e70f4fb8 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 20 Jun 2018 10:39:19 +0400 Subject: [PATCH] fixes after @xla review - move prometheus metrics into internal packages - *Option structs - misc. format changes --- consensus/metrics.go | 73 ++++++++++++++++++++++ consensus/state.go | 24 +++++--- mempool/mempool.go | 16 +++-- mempool/metrics.go | 14 +++++ metrics/prometheus/prometheus.go | 102 ------------------------------- node/node.go | 22 +++++-- p2p/metrics.go | 14 +++++ p2p/switch.go | 11 ++-- 8 files changed, 151 insertions(+), 125 deletions(-) delete mode 100644 metrics/prometheus/prometheus.go diff --git a/consensus/metrics.go b/consensus/metrics.go index 7e1e2337..253880e8 100644 --- a/consensus/metrics.go +++ b/consensus/metrics.go @@ -3,6 +3,9 @@ package consensus import ( "github.com/go-kit/kit/metrics" "github.com/go-kit/kit/metrics/discard" + + prometheus "github.com/go-kit/kit/metrics/prometheus" + stdprometheus "github.com/prometheus/client_golang/prometheus" ) // Metrics contains metrics exposed by this package. @@ -37,6 +40,76 @@ type Metrics struct { TotalTxs metrics.Gauge } +// PrometheusMetrics returns Metrics build using Prometheus client library. +func PrometheusMetrics() *Metrics { + return &Metrics{ + Height: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "height", + Help: "Height of the chain.", + }, []string{}), + Rounds: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "rounds", + Help: "Number of rounds.", + }, []string{}), + + Validators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "validators", + Help: "Number of validators.", + }, []string{}), + ValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "validators_power", + Help: "Total power of all validators.", + }, []string{}), + MissingValidators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "missing_validators", + Help: "Number of validators who did not sign.", + }, []string{}), + MissingValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "missing_validators_power", + Help: "Total power of the missing validators.", + }, []string{}), + ByzantineValidators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "byzantine_validators", + Help: "Number of validators who tried to double sign.", + }, []string{}), + ByzantineValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "byzantine_validators_power", + Help: "Total power of the byzantine validators.", + }, []string{}), + + BlockIntervalSeconds: prometheus.NewHistogramFrom(stdprometheus.HistogramOpts{ + Subsystem: "consensus", + Name: "block_interval_seconds", + Help: "Time between this and the last block.", + Buckets: []float64{1, 2.5, 5, 10, 60}, + }, []string{}), + + NumTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "num_txs", + Help: "Number of transactions.", + }, []string{}), + BlockSizeBytes: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "block_size_bytes", + Help: "Size of the block.", + }, []string{}), + TotalTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "consensus", + Name: "total_txs", + Help: "Total number of transactions.", + }, []string{}), + } +} + // NopMetrics returns no-op Metrics. func NopMetrics() *Metrics { return &Metrics{ diff --git a/consensus/state.go b/consensus/state.go index 686d2e7d..aab82296 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -120,8 +120,19 @@ type ConsensusState struct { metrics *Metrics } +// CSOption sets an optional parameter on the ConsensusState. +type CSOption func(*ConsensusState) + // NewConsensusState returns a new ConsensusState. -func NewConsensusState(config *cfg.ConsensusConfig, state sm.State, blockExec *sm.BlockExecutor, blockStore sm.BlockStore, mempool sm.Mempool, evpool sm.EvidencePool, options ...func(*ConsensusState)) *ConsensusState { +func NewConsensusState( + config *cfg.ConsensusConfig, + state sm.State, + blockExec *sm.BlockExecutor, + blockStore sm.BlockStore, + mempool sm.Mempool, + evpool sm.EvidencePool, + options ...CSOption, +) *ConsensusState { cs := &ConsensusState{ config: config, blockExec: blockExec, @@ -169,10 +180,8 @@ func (cs *ConsensusState) SetEventBus(b *types.EventBus) { } // WithMetrics sets the metrics. -func WithMetrics(metrics *Metrics) func(*ConsensusState) { - return func(cs *ConsensusState) { - cs.metrics = metrics - } +func WithMetrics(metrics *Metrics) CSOption { + return func(cs *ConsensusState) { cs.metrics = metrics } } // String returns a string. @@ -1338,8 +1347,9 @@ func (cs *ConsensusState) recordMetrics(height int64, block *types.Block) { if height > 1 { lastBlockMeta := cs.blockStore.LoadBlockMeta(height - 1) - cs.metrics.BlockIntervalSeconds. - Observe(block.Time.Sub(lastBlockMeta.Header.Time).Seconds()) + cs.metrics.BlockIntervalSeconds.Observe( + block.Time.Sub(lastBlockMeta.Header.Time).Seconds(), + ) } cs.metrics.NumTxs.Set(float64(block.NumTxs)) diff --git a/mempool/mempool.go b/mempool/mempool.go index 1df1651e..418470a7 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -87,8 +87,16 @@ type Mempool struct { metrics *Metrics } +// MempoolOption sets an optional parameter on the Mempool. +type MempoolOption func(*Mempool) + // NewMempool returns a new Mempool with the given configuration and connection to an application. -func NewMempool(config *cfg.MempoolConfig, proxyAppConn proxy.AppConnMempool, height int64, options ...func(*Mempool)) *Mempool { +func NewMempool( + config *cfg.MempoolConfig, + proxyAppConn proxy.AppConnMempool, + height int64, + options ...MempoolOption, +) *Mempool { mempool := &Mempool{ config: config, proxyAppConn: proxyAppConn, @@ -122,10 +130,8 @@ func (mem *Mempool) SetLogger(l log.Logger) { } // WithMetrics sets the metrics. -func WithMetrics(metrics *Metrics) func(*Mempool) { - return func(mem *Mempool) { - mem.metrics = metrics - } +func WithMetrics(metrics *Metrics) MempoolOption { + return func(mem *Mempool) { mem.metrics = metrics } } // CloseWAL closes and discards the underlying WAL file. diff --git a/mempool/metrics.go b/mempool/metrics.go index 7c3cc967..f381678c 100644 --- a/mempool/metrics.go +++ b/mempool/metrics.go @@ -3,6 +3,9 @@ package mempool import ( "github.com/go-kit/kit/metrics" "github.com/go-kit/kit/metrics/discard" + + prometheus "github.com/go-kit/kit/metrics/prometheus" + stdprometheus "github.com/prometheus/client_golang/prometheus" ) // Metrics contains metrics exposed by this package. @@ -12,6 +15,17 @@ type Metrics struct { Size metrics.Gauge } +// PrometheusMetrics returns Metrics build using Prometheus client library. +func PrometheusMetrics() *Metrics { + return &Metrics{ + Size: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "mempool", + Name: "size", + Help: "Size of the mempool (number of uncommitted transactions).", + }, []string{}), + } +} + // NopMetrics returns no-op Metrics. func NopMetrics() *Metrics { return &Metrics{ diff --git a/metrics/prometheus/prometheus.go b/metrics/prometheus/prometheus.go deleted file mode 100644 index 7691efae..00000000 --- a/metrics/prometheus/prometheus.go +++ /dev/null @@ -1,102 +0,0 @@ -package prometheus - -import ( - prometheus "github.com/go-kit/kit/metrics/prometheus" - stdprometheus "github.com/prometheus/client_golang/prometheus" - - cs "github.com/tendermint/tendermint/consensus" - mempl "github.com/tendermint/tendermint/mempool" - "github.com/tendermint/tendermint/p2p" -) - -// Consensus returns consensus Metrics build using Prometheus client library. -func Consensus() *cs.Metrics { - return &cs.Metrics{ - Height: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "height", - Help: "Height of the chain.", - }, []string{}), - Rounds: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "rounds", - Help: "Number of rounds.", - }, []string{}), - - Validators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "validators", - Help: "Number of validators.", - }, []string{}), - ValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "validators_power", - Help: "Total power of all validators.", - }, []string{}), - MissingValidators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "missing_validators", - Help: "Number of validators who did not sign.", - }, []string{}), - MissingValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "missing_validators_power", - Help: "Total power of the missing validators.", - }, []string{}), - ByzantineValidators: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "byzantine_validators", - Help: "Number of validators who tried to double sign.", - }, []string{}), - ByzantineValidatorsPower: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "byzantine_validators_power", - Help: "Total power of the byzantine validators.", - }, []string{}), - - BlockIntervalSeconds: prometheus.NewHistogramFrom(stdprometheus.HistogramOpts{ - Subsystem: "consensus", - Name: "block_interval_seconds", - Help: "Time between this and the last block.", - Buckets: []float64{1, 2.5, 5, 10, 60}, - }, []string{}), - - NumTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "num_txs", - Help: "Number of transactions.", - }, []string{}), - BlockSizeBytes: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "block_size_bytes", - Help: "Size of the block.", - }, []string{}), - TotalTxs: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "consensus", - Name: "total_txs", - Help: "Total number of transactions.", - }, []string{}), - } -} - -// P2P returns p2p Metrics build using Prometheus client library. -func P2P() *p2p.Metrics { - return &p2p.Metrics{ - Peers: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "p2p", - Name: "peers", - Help: "Number of peers.", - }, []string{}), - } -} - -// Mempool returns mempool Metrics build using Prometheus client library. -func Mempool() *mempl.Metrics { - return &mempl.Metrics{ - Size: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ - Subsystem: "mempool", - Name: "size", - Help: "Size of the mempool (number of uncommitted transactions).", - }, []string{}), - } -} diff --git a/node/node.go b/node/node.go index 0f55683b..dc79cff7 100644 --- a/node/node.go +++ b/node/node.go @@ -22,7 +22,6 @@ import ( cs "github.com/tendermint/tendermint/consensus" "github.com/tendermint/tendermint/evidence" mempl "github.com/tendermint/tendermint/mempool" - prometrics "github.com/tendermint/tendermint/metrics/prometheus" "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/p2p/pex" "github.com/tendermint/tendermint/privval" @@ -96,7 +95,7 @@ type MetricsProvider func() (*cs.Metrics, *p2p.Metrics, *mempl.Metrics) // DefaultMetricsProvider returns consensus, p2p and mempool Metrics build // using Prometheus client library. func DefaultMetricsProvider() (*cs.Metrics, *p2p.Metrics, *mempl.Metrics) { - return prometrics.Consensus(), prometrics.P2P(), prometrics.Mempool() + return cs.PrometheusMetrics(), p2p.PrometheusMetrics(), mempl.PrometheusMetrics() } // NopMetricsProvider returns consensus, p2p and mempool Metrics as no-op. @@ -243,8 +242,12 @@ func NewNode(config *cfg.Config, // Make MempoolReactor mempoolLogger := logger.With("module", "mempool") - mempool := mempl.NewMempool(config.Mempool, proxyApp.Mempool(), state.LastBlockHeight, - mempl.WithMetrics(memplMetrics)) + mempool := mempl.NewMempool( + config.Mempool, + proxyApp.Mempool(), + state.LastBlockHeight, + mempl.WithMetrics(memplMetrics), + ) mempool.SetLogger(mempoolLogger) mempool.InitWAL() // no need to have the mempool wal during tests mempoolReactor := mempl.NewMempoolReactor(config.Mempool, mempool) @@ -275,8 +278,15 @@ func NewNode(config *cfg.Config, bcReactor.SetLogger(logger.With("module", "blockchain")) // Make ConsensusReactor - consensusState := cs.NewConsensusState(config.Consensus, state.Copy(), - blockExec, blockStore, mempool, evidencePool, cs.WithMetrics(csMetrics)) + consensusState := cs.NewConsensusState( + config.Consensus, + state.Copy(), + blockExec, + blockStore, + mempool, + evidencePool, + cs.WithMetrics(csMetrics), + ) consensusState.SetLogger(consensusLogger) if privValidator != nil { consensusState.SetPrivValidator(privValidator) diff --git a/p2p/metrics.go b/p2p/metrics.go index 2e3eff14..ab876ee7 100644 --- a/p2p/metrics.go +++ b/p2p/metrics.go @@ -3,6 +3,9 @@ package p2p import ( "github.com/go-kit/kit/metrics" "github.com/go-kit/kit/metrics/discard" + + prometheus "github.com/go-kit/kit/metrics/prometheus" + stdprometheus "github.com/prometheus/client_golang/prometheus" ) // Metrics contains metrics exposed by this package. @@ -11,6 +14,17 @@ type Metrics struct { Peers metrics.Gauge } +// PrometheusMetrics returns Metrics build using Prometheus client library. +func PrometheusMetrics() *Metrics { + return &Metrics{ + Peers: prometheus.NewGaugeFrom(stdprometheus.GaugeOpts{ + Subsystem: "p2p", + Name: "peers", + Help: "Number of peers.", + }, []string{}), + } +} + // NopMetrics returns no-op Metrics. func NopMetrics() *Metrics { return &Metrics{ diff --git a/p2p/switch.go b/p2p/switch.go index ed7a8098..bf5f9747 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -77,8 +77,11 @@ type Switch struct { metrics *Metrics } +// SwitchOption sets an optional parameter on the Switch. +type SwitchOption func(*Switch) + // NewSwitch creates a new Switch with the given config. -func NewSwitch(cfg *config.P2PConfig, options ...func(*Switch)) *Switch { +func NewSwitch(cfg *config.P2PConfig, options ...SwitchOption) *Switch { sw := &Switch{ config: cfg, reactors: make(map[string]Reactor), @@ -111,10 +114,8 @@ func NewSwitch(cfg *config.P2PConfig, options ...func(*Switch)) *Switch { } // WithMetrics sets the metrics. -func WithMetrics(metrics *Metrics) func(*Switch) { - return func(sw *Switch) { - sw.metrics = metrics - } +func WithMetrics(metrics *Metrics) SwitchOption { + return func(sw *Switch) { sw.metrics = metrics } } //---------------------------------------------------------------------