feat(dot/peerset): Instrument peerset with peer reputation and number of bans #2241
Conversation
timwu20
requested review from
EclesioMeloJunior,
edwardmack,
jimjbrettj,
kishansagathiya,
noot and
qdm12
as code owners
timwu20
changed the title
Codecov Report
@@ Coverage Diff @@
## development #2241 +/- ##
===============================================
+ Coverage 59.52% 59.82% +0.29%
===============================================
Files 210 212 +2
Lines 27540 27666 +126
===============================================
+ Hits 16394 16552 +158
+ Misses 9447 9417 -30
+ Partials 1699 1697 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
timwu20
force-pushed
the
tim/peerset-metrics
branch
from
1fd5e6d
to
73a98ae
Compare
| @@ -282,6 +291,7 @@ func (ps *PeerSet) updateTime() error { | |||
| after := reputationTick(before) | |||
| n.setReputation(after) | |||
| ps.peerState.nodes[peerID] = n | |||
| reputationGauge.WithLabelValues(peerID.Pretty()).Set(float64(after)) | |||
There was a problem hiding this comment.
That will potentially blow up memory usage of the Prometheus server since peerID is rather random. Maybe leave this as a log or find another way to report the metric with a lower cardinality / bounded label.
See the caution note in yellow at https://prometheus.io/docs/practices/naming/
There was a problem hiding this comment.
Ideally I only want to label this on the devnets where the peers are bound to only like 3-4 nodes. I'll refactor this PR to make this configurable.
There was a problem hiding this comment.
Maybe just have it as a periodic log? 🤔 Like our sync benchmarker?
Or you could have a mapping from [1 to 50] to peerIDs, use the [1-50] set for the metrics labels and log out the mapping at program start. Not totally ideal either, but that would work.
| @@ -334,6 +344,7 @@ func (ps *PeerSet) reportPeer(change ReputationChange, peers ...peer.ID) error { | |||
| for i := 0; i < setLen; i++ { | |||
| if ps.peerState.peerStatus(i, pid) == connectedPeer { | |||
| // disconnect peer | |||
| bannedCounter.WithLabelValues(pid.Pretty()).Inc() | |||
Changes
Tests
Issues
Primary Reviewer