feat: add API and logs to display monitored validator indices #8702
Conversation
Summary of ChangesHello @dik654, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new API endpoint and improved logging functionality to enhance the visibility for infrastructure operators regarding the validator indices actively monitored by their beacon node. The changes aim to provide an immediate overview of connected validators, utilizing a debug API namespace and logging to avoid potential cardinality issues associated with metrics for this type of data. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new API endpoint to list monitored validator indices and adds info-level logs when validators are registered or removed from the monitor. The changes are well-implemented and the new API endpoint is a useful addition for operators. My main feedback concerns the new logging feature. While logging the full list of indices is a deliberate design choice to provide state visibility, it could lead to performance degradation and excessively large log entries when a large number of validators are being monitored. I've provided suggestions to truncate the list of indices in the log messages to prevent potential operational issues with log ingestion and storage, while still retaining the benefit of seeing a snapshot of the monitored validators.
| logger.info("Validator registered to monitor", { | ||
| index, | ||
| total: validators.size, | ||
| indices: Array.from(validators.keys()).join(","), | ||
| }); |
There was a problem hiding this comment.
Logging all monitored validator indices on every registration can lead to performance issues and very large log entries, especially when monitoring a large number of validators. This can be problematic for log ingestion and analysis systems, and it might consume significant disk space if registrations are frequent.
Given that the total count is already logged and there's a new API endpoint to fetch the full list, consider truncating the list of indices in the log message if it exceeds a certain length. This would provide a balance between providing context in logs and avoiding performance/operational issues.
const keys = Array.from(validators.keys());
logger.info("Validator registered to monitor", {
index,
total: validators.size,
indices: keys.length > 100 ? `${keys.slice(0, 100).join(",")},...` : keys.join(","),
});| logger.info("Validator removed from monitor", { | ||
| index, | ||
| total: validators.size, | ||
| indices: validators.size > 0 ? Array.from(validators.keys()).join(",") : "", | ||
| }); |
There was a problem hiding this comment.
Similar to the registration logic, logging the full list of validator indices upon removal can cause performance and logging issues when many validators are monitored. It's better to truncate the list to keep log messages manageable. This still provides a snapshot of the monitored validators without overwhelming logging systems.
const keys = Array.from(validators.keys());
logger.info("Validator removed from monitor", {
index,
total: validators.size,
indices: keys.length > 100 ? `${keys.slice(0, 100).join(",")},...` : keys.join(","),
});
dik654
force-pushed
the
feat/monitored-validator-indices
branch
from
3eee5ae to
db3f26b
Compare
| validators.getOrDefault(index).lastRegisteredTimeMs = Date.now(); | ||
| if (isNewValidator) { | ||
| const keys = Array.from(validators.keys()); | ||
| logger.info("Validator registered to monitor", { |
There was a problem hiding this comment.
this will be way to verbose for nodes that have a lot of validators connected, if we want to print something like this in the logs it should be done each epoch (or even less often)
There was a problem hiding this comment.
To clarify the conflicting feedback:
∙ @matthewkeil suggested logging on startup or when validator indices change
∙ @nflaig raised a concern about verbosity for nodes with many validators
I’ll go with epoch-based logging as @nflaig suggested — this keeps logs clean while still providing visibility.
Let me know if there are any objections.
There was a problem hiding this comment.
I would refer to @chiemerieezechukwu what's the preferred method to consume this, I think a metric would be nice too but we'd need to think how we can update it more frequently as 1h seems to be still to long as per #8702 (comment)
There was a problem hiding this comment.
Hi, sorry for the late response. I needed some time to fully understand the context and think through the options.
After carefully reviewing the discussion again, it seems the core issue is that validator_monitor_validators metric doesn't update quickly when validators disconnect:
"Doesn't seem that it is updated regularly though. When I move validators away from beacons, this number doesn't decrease it seems" - @chiemerieezechukwu
If I understand correctly, this is due to RETAIN_REGISTERED_VALIDATORS_MS being set to 1 hour.
@nflaig I was wondering if either of these approaches would be acceptable?
-
Reduce
RETAIN_REGISTERED_VALIDATORS_MSfrom 1 hour to 2-3 epochs (~13-20 min) - I saw you reduced it from 12h to 1h in chore: reduce time to retain registered validators in monitor to 1 hour #7668, so I wanted to check if further reduction might cause any issues. This would make the existing metric update faster. -
Add epoch-based logging with active/inactive count based on
lastRegisteredTimeMs- Keep the existing 1-hour prune logic as is, but log the count of validators registered within the last 2-3 epochs each epoch. This provides faster feedback via logs without changing the existing metric behavior. (Though this probably isn't what @chiemerieezechukwu wants since they mentioned Grafana dashboard)
Once we settle on that, I'd be happy to follow up with @chiemerieezechukwu about the "nice to have" feature for showing which validators (indices or pubkey) are connected.
| }, | ||
| }, | ||
| getMonitoredValidatorIndices: { | ||
| url: "/eth/v1/debug/monitored_validators", |
There was a problem hiding this comment.
@chiemerieezechukwu how do you want to consume this, I would think a metric is easier to work with than exposing an api
There was a problem hiding this comment.
@chiemerieezechukwu how do you want to consume this, I would think a metric is easier to work with than exposing an api
Mainly on a Grafana dashboard. Would be nice to see at any point the number of validators connected to a beacon. Getting which validators (the indices or pubkey) would be a "nice to have"
There was a problem hiding this comment.
Would be nice to see at any point the number of validators connected to a beacon.
you can already see that on the summary dashboard
or validator monitor dashboard
There was a problem hiding this comment.
Would be nice to see at any point the number of validators connected to a beacon.
you can already see that on the summary dashboard
or validator monitor dashboard
Doesn't seem that it is updated regularly though. When I move validators away from beacons, this number doesn't decrease it seems
There was a problem hiding this comment.
I think I might have been dealing with a scenario where the response is cached and not updated on the dashboard
There was a problem hiding this comment.
it's because we cache this internally for 1h and only then if we haven't seen a validator we stop tracking them
fun fact, this was previously 12h until I reduced it here #7668
if we need a faster feedback it might make sense to have a metric tracked somewhere else
| * Returns the validator indices that are currently being monitored by the validator monitor. | ||
| * These are validators that have registered with this beacon node via the validator API. | ||
| */ | ||
| getMonitoredValidatorIndices: Endpoint< |
There was a problem hiding this comment.
this would be a custom route, non standardized in the spec, and has to be put into /lodestar namespace
|
Pinging @0xmrree from this PR for visibility |
4 participants
Closes #8698
Motivation
Infrastructure operators want to easily see which validator indices are connected to their beacon node at a glance.
Description
GET /eth/v1/debug/monitored_validatorsAPI endpoint that returns array of validator indices currently being monitoredValidator registered to monitor index=X, total=Y, indices=0,1,2,...Validator removed from monitor index=X, total=Y, indices=0,1,3,...Usage
API endpoint:
curl http://localhost:9596/eth/v1/debug/monitored_validators # Response: {"data":[0,1,2,3,4,5,6,7]}For dashboard integration (e.g., Grafana), you can use the JSON API datasource plugin to poll this endpoint and display the validator indices.
Design decisions
link to issue
Closes #8698
AI Assistance Disclosure
Used Claude Code to assist with implementation and code exploration.