fixes after @xla review

- move prometheus metrics into internal packages
- *Option structs
- misc. format changes

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{

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))

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.

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{

metrics/prometheus/prometheus.go

@@ -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{}),
-	}
-}

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)

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{

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 }
 }
 
 //---------------------------------------------------------------------