-
Notifications
You must be signed in to change notification settings - Fork 788
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
Remove unused metrics #6817
base: unstable
Are you sure you want to change the base?
Remove unused metrics #6817
Conversation
pub static DATA_COLUMNS_SIDECAR_PROCESSING_SUCCESSES: LazyLock<Result<IntCounter>> = | ||
LazyLock::new(|| { | ||
try_create_int_counter( | ||
"beacon_data_column_sidecar_processing_successes_total", | ||
"Number of data column sidecars verified for gossip", | ||
) | ||
}); |
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.
Note: another metric with the same name beacon_data_column_sidecar_processing_successes_total
exists:
lighthouse/beacon_node/beacon_chain/src/metrics.rs
Lines 1647 to 1653 in 86c65d6
pub static DATA_COLUMN_SIDECAR_PROCESSING_SUCCESSES: LazyLock<Result<IntCounter>> = | |
LazyLock::new(|| { | |
try_create_int_counter( | |
"beacon_data_column_sidecar_processing_successes_total", | |
"Number of data column sidecars verified for gossip", | |
) | |
}); |
*/ | ||
pub static BLOCK_AVAILABILITY_DELAY: LazyLock<Result<IntGauge>> = LazyLock::new(|| { | ||
try_create_int_gauge( | ||
"block_availability_delay", |
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.
this sounds like a useful metric that we should use
pub static DATA_AVAILABILITY_OVERFLOW_STORE_CACHE_SIZE: LazyLock<Result<IntGauge>> = | ||
LazyLock::new(|| { | ||
try_create_int_gauge( | ||
"data_availability_overflow_store_cache_size", |
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.
We have DATA_AVAILABILITY_OVERFLOW_MEMORY_BLOCK_CACHE_SIZE
and DATA_AVAILABILITY_OVERFLOW_MEMORY_STATE_CACHE_SIZE
, so ok to remove
"beacon_sync_contribution_processing_signature_seconds", | ||
"Time spent on the signature verification of sync contribution processing", | ||
) | ||
}); |
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.
Ok to delete for me
"beacon_update_head_seconds", | ||
"Time taken to update the canonical head", | ||
) | ||
}); |
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.
We have FORK_CHOICE_TIMES
which cover what we may be interested in, ok to remove
"beacon_fork_choice_set_head_lag_times", | ||
"Time taken between finding the head and setting the canonical head value", | ||
) | ||
}); |
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.
Doesn't sound too useful, ok to remove
"beacon_block_processing_signature_seconds", | ||
"Time spent doing signature verification for a block.", | ||
) | ||
}); |
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 don't think we are interested in this, ok to remove
Issue Addressed
N/A
Proposed Changes
Removed metrics that were defined but not used anywhere.