Integrate nim-metrics and add some metrics from the ETH2 spec #394
Conversation
| @@ -17,19 +17,20 @@ bin = @[ | |||
|
|
|||
| ### Dependencies | |||
| requires "nim >= 0.19.0", | |||
| proc toGaugeValue*(hash: Eth2Digest): int64 = | ||
| # Only the last 8 bytes are taken into consideration in accordance | ||
| # to the ETH2 metrics spec: | ||
| # https://github.com/ethereum/eth2.0-metrics/blob/6a79914cb31f7d54858c7dd57eee75b6162ec737/metrics.md#interop-metrics |
There was a problem hiding this comment.
Do you mean the long URL?
There was a problem hiding this comment.
no, the spec using a signed integer like this
| @@ -23,6 +23,28 @@ const | |||
| genesisFile = "genesis.json" | |||
| testnetsBaseUrl = "https://serenity-testnets.status.im" | |||
|
|
|||
| declareGauge beacon_slot, "Latest slot of the beacon chain state" | |||
There was a problem hiding this comment.
from experience, metrics have bigger chances of staying recent when they're declared close to where they're updated.
as an example, since we have a function that advances slot time, declare the metric in that function, else it's easy to let the metric and the underlying data go out of sync when the code changes, and you end up with a graveyard of metrics representing what the code once did.
In this they are similar to log statements: you would never ever consider making a central repository of all log strings in the application - it is local information that deserves to be kept local.
at the very least, we should think about how to spread them to the relevant module (ie the BlockPool is responsible for keeping track of the head block, so all related metrics should be in there - then it's easier to make sure that the metric tracks the actual "source of truth"
There was a problem hiding this comment.
Have you noticed the comment two lines below which suggests exactly this :)
It was easier to put the metrics in this file as a starting point, because I used some vim editing tricks to copy paste the metric names and descriptions from the spec and to reformat them as Nim code.
|
Some of the metrics are not implemented yet and the declarations have to be moved to their respective modules, but I prefer if we address this in follow-up PRs. Does anyone have objections against merging this as a starting point? |
No description provided.