-
Notifications
You must be signed in to change notification settings - Fork 886
Remove unused metrics #6817
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
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
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 do have this:
lighthouse/beacon_node/beacon_chain/src/metrics.rs
Lines 1390 to 1396 in a4e3f36
pub static BEACON_BLOCK_DELAY_AVAILABLE_SLOT_START: LazyLock<Result<IntGauge>> = | |
LazyLock::new(|| { | |
try_create_int_gauge( | |
"beacon_block_delay_available_slot_start", | |
"Duration between the time that block became available and the start of the slot.", | |
) | |
}); |
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.
oh nvm just read the comments below!
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
"block_availability_delay", | ||
"Duration between start of the slot and the time at which all components of the block are available.", | ||
) | ||
}); |
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.
Actually reviewed current unstable and we have BEACON_BLOCK_DELAY_AVAILABLE_SLOT_START
which tracks exactly this but with more precision. Ok to delete BLOCK_AVAILABILITY_DELAY
This reverts commit 2ffb739.
Issue Addressed
N/A
Proposed Changes
Removed metrics that were defined but not used anywhere.