ChainSafe / ChainSafe/lodestar
Organize and Update Metrics
- Dominant language
- TypeScript
- Stars
- 1.4k
- Forks
- 483
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 150
Description
## Problem description
There are a few goals that this issue is meant to discuss/cover.
Our collection of metrics has grown over time and the seems to be a lot of technical debt that needs to be addressed. The big thing is the organization of the metrics we currently collect. We have two giant files that are not very well organized so it is becoming harder to maintain. I would like to propose cleaning up the organization by shuffling the metrics into a few separate files so that they are easier to maintain.
There are also a number of metrics that are prescribed in `ethereum/beacon-metrics` that we are not collected as part of our metrics program. Ideally these will also get added.
There is also another creeping issue that I think should be addressed. We are starting to collect metrics is folders other than beacon-node and having the `Metrics` type there creates a circular dependency.
## Solution description
### Creating `packages/metrics` Folder
I would like to suggest that we move the metrics out of the `beacon-node` package and into its own package. This will make it easier to import the `Metrics` interface into other packages. I also am thinking we can export a singleton metrics object from the package. The package could export a getter function that obtains the instance of the singleton `metrics` object for addition to the `chain` object as it is today. But this would open the possibility of getting the metrics object in any help function or the other packages without needing to pass the `chain` object around. Not sure if this is strictly necessary but might make our lives easier going forward.
As an example we have metrics in `reqresp` and in `state-transition` that could easily be centralized.
### Nesting Metrics
I am not sure if added a lot of nested domains will make using the `metrics` object easier or more complicated. This is the first thing that should be decided. Is it better to nest similar metrics within domains, under the metrics object or should we keep a flat structure so its easier to find what one is looking for using intellisense
`metrics.forkChoice.findHeadSeconds` vs `metrics.findHeadSeconds`
My gut tells me that some minimal nesting will be nice due to the large number of metrics that we collect but using a deeply nested structure may become a pain to deal with as some metrics tend to cross domain boundaries and it will be challenging to decide "where" to put things.
` metrics.chain.bls.multithread.aggregatedPublicKeysCount()` vs `metrics.bls.aggregatedPublicKeys`
I actually kinda flip-flopped on this while writing this up so its worth discussing further as I'm not really sure which will be best myself.
### File Structure
I think we really need to split up the two giant files into a number of smaller files so its easier to maintain. Seems like an easy win.
### `lodestar` Metric Prefix
This seems a bit overkill to add to basically every single metric. We only collect metrics on "lodestar" which is a "beacon" node so separating the two seems silly. The only reason would be to differentiate "ethereum standard" metrics from our lodestar specifics but I am not sure that is necessary. I do think however that we should be prefixing with the "domain" a metric represents and having that domain match the filename seems like a nice to have so its clear where it is without having to do a global file search.
### Additional context
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.