fix(sei-tendermint): bound P2P and consensus Prometheus label cardinality (PLT-1070, PLT-1071) - #4029
Conversation
…lity (PLT-1070, PLT-1071) Remove peer_id and validator_address labels from metrics fed by untrusted P2P input so time series count stays bounded. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4029 +/- ##
==========================================
- Coverage 61.21% 60.25% -0.97%
==========================================
Files 2153 2054 -99
Lines 188381 176693 -11688
==========================================
- Hits 115324 106469 -8855
+ Misses 62315 60459 -1856
+ Partials 10742 9765 -977
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Keep per-validator SLO dashboard attribution while bounding cardinality by labeling current-set senders by address and rolling everyone else into other. Co-authored-by: Cursor <cursoragent@cursor.com>
PR SummaryLow Risk Overview
Generated Reviewed by Cursor Bugbot for commit 8755d2c. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Focused, correct cardinality fix: peer_id is dropped from two P2P-fed counters and late_votes is gated to the current validator set with an other overflow bucket, which also closes a path where any peer could mint arbitrary validator_address series. No blockers; the residual note is that Prometheus never evicts series, so the cap is the union of validator sets over the process lifetime rather than the set size claimed in the PR description.
Findings: 0 blocking | 2 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] No test covers the new
MarkLateVotebranching. A small unit test insei-tendermint/internal/consensusasserting that an in-set address yields its own series while an out-of-set address (and a nil validator set) folds intootherwould pin the behavior the SLO dashboard panel depends on, and would catch a future refactor silently reverting to per-sender labeling. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
|
|
||
| func (m *Metrics) MarkLateVote(addr types.Address, validators *types.ValidatorSet) { | ||
| label := lateVoteOtherLabel | ||
| if validators != nil && validators.HasAddress(addr) { |
There was a problem hiding this comment.
[suggestion] Active-set gating bounds the rate of new series but not the total: a Prometheus CounterVec never evicts a child, so every address that was in the set at the moment it sent a late vote keeps its series for the life of the process. The cap is therefore |union of validator sets over process lifetime| + 1, not the ~|validator set| + 1 the PR description states.
That is still a large improvement — the unbounded axis (peer churn, and pre-fix any peer minting a series with an arbitrary ValidatorAddress, since this marking happens before the vote's validator is checked against the set) is gone, and validator churn is slow and governance-paced. So this is worth adjusting the claim in the description/help text for rather than redesigning the label. If a hard cap is actually required, it needs an explicit DeleteLabelValues sweep on validator-set change, which is a meaningfully larger change and probably not warranted here.
Summary
peer_idfromtendermint_consensus_block_partsandtendermint_p2p_peer_receive_bytes_totaltendermint_consensus_late_votesto the current validator set: label active-set senders byvalidator_address, roll all others intootherchID,message_typeon receive-bytes)Context
peer_idlabels on P2P-fed counters grow without bound as peers churn over a node's lifetime. Those metrics are aggregated at the node level (or by channel/message type where labels are fixed).late_votespreviously labeled every distinct sender address forever, which also inflated cardinality as the validator set churned. Full label removal would have broken the Top-10 validators by late-votes rate panel on the Sei L1 Block Time SLO dashboard (sei-l1-block-time-slo). Active-set gating caps series at ~|validator set| + 1 per node while preserving per-validator attribution for live validators.Platform impact
tendermint_consensus_late_votes) continues to work for current-set validators; anotheroverflow series may appear during catch-up or after set churnblock_partsandpeer_receive_bytes_totalare not referenced in Platform dashboards or alertsCloses PLT-1070 and PLT-1071.