-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore: Add metrics for supervisor RPC error #16111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
chore: Add metrics for supervisor RPC error #16111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emhane do we really need all of these?
yes I think it will be useful for debugging interop + developing recovery mechanism + perf optimisation we need a separate grafana board for op-reth #15724 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! one counter can be skipped, otherwise lgtm
/// Counter for the number of times the chain database was uninitialized | ||
pub(crate) uninitialized_chain_database_count: Counter, | ||
/// Counter for the number of times data was skipped | ||
pub(crate) skipped_data_count: Counter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can create a single metric 'supervisor_api_errors' and have the error_type as tags to the metric. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the benefits? link to docs for the type you are referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smol comment remaining
Resolves #16080