Attestation performance metrics #7709
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7709 +/- ##
==========================================
+ Coverage 62.10% 62.12% +0.01%
==========================================
Files 429 429
Lines 30365 30379 +14
==========================================
+ Hits 18859 18873 +14
Misses 8566 8566
Partials 2940 2940 |
There was a problem hiding this comment.
I'm slightly concerned on how this will increase the cardinality of the metrics page significantly and require tsdb to store much more data than before.
Is a gauge the most appropriate format?
We track overall inclusion distance with a histogram in the beacon node, should we use a histogram?
validator/client/metrics.go
Outdated
| "pubkey", | ||
| }, | ||
| ) | ||
| // ValidatorCorrectlyVotedHeadGaugeVec used to keep track of validator's accuracy on voting target by public key. |
There was a problem hiding this comment.
| // ValidatorCorrectlyVotedHeadGaugeVec used to keep track of validator's accuracy on voting target by public key. | |
| // ValidatorCorrectlyVotedHeadGaugeVec used to keep track of validator's accuracy on voting head by public key. |
There was a problem hiding this comment.
Ahh thanks for catching this. As for the gauge @prestonvanloon is worried about the same. Gauge is needed to get a time series which you need if you want to collect the table above. Getting a histogram is only useful to get averages. This as gauge replicates the table in beaconcha.in. The prometheus dbase can then later be pruned can't it?
validator/client/metrics.go
Outdated
| if (slot+1)%params.BeaconConfig().SlotsPerEpoch != 0 || slot <= params.BeaconConfig().SlotsPerEpoch { | ||
| // Do nothing unless we are at the end of the epoch, and not in the first epoch. |
potuz
force-pushed
the
inclusion-distance-metrics
branch
from
37bc6fa
to
c6091ae
Compare
|
Fixes #7694 |
|
@terencechain PTAL |
rauljordan
added
OK to merge
Ready For Review
What type of PR is this?
Feature
Fixes #7694
What does this PR do? Why is it needed?
This PR exports 5 metrics by pubkey: the attested slot, the inclusion distance, the correctness of the vote on head,target and source. In order to be correct however this is now based on #7708 since otherwise it would suffer from #7699
Other notes for review
The attested slot is always one epoch ahead with this PR. This has to be taken into account when producing tables like this one
I have checked the above for over a day with beaconcha.in now that it is working.