Target metrics #1811
Target metrics #1811
Conversation
| selectTxsToGossipSum prometheus.Gauge | ||
| selectedTxsToGossip prometheus.Counter | ||
| targetTxsSum prometheus.Gauge |
There was a problem hiding this comment.
Is there a specific reason to use a Gauge? I feel like only Inc/Add method are used to represent the metric.
There was a problem hiding this comment.
Hmm was just copying how we had handled durations elsewhere. This will never decrease, so we could use a counter instead.
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| numTxsGossipped := 0 |
| }), | ||
| selectedTxsToGossip: prometheus.NewCounter(prometheus.CounterOpts{ | ||
| Namespace: namespace, | ||
| Name: "selected_txs_to_gossip", |
There was a problem hiding this comment.
should we call this select_ to be consistent w/ the other metrics?
| }), | ||
| selectTxsToGossipSum: prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| Namespace: namespace, | ||
| Name: "select_txs_to_gossip_sum", |
There was a problem hiding this comment.
it's not obvious this + target_txs_sum are latency metrics from the naming - can we put time or something in the name to make it more clear?
| return nil | ||
| } | ||
| g.log.Debug("gossiping transactions", zap.Int("txs", len(txs)), zap.Duration("t", time.Since(start))) | ||
| g.metrics.txsGossiped.Add(float64(len(txs))) |
There was a problem hiding this comment.
Why did we remove this metric?
| return err | ||
| } | ||
| } | ||
| t.metrics.txsGossiped.Add(float64(numTxsGossipped)) |
There was a problem hiding this comment.
Is this right? This looks like it will re-accumulate previously counted txs into the metric since we're using numTxsGossiped. Shouldn't we be using something like t.metrics.txsGossiped.Add(float64(len(gossipContainer.Txs)) here?
This PR add target specific metrics into the gossiper and renames
gtotas the pointer receiver for*gossiper.Target.