Skip to content
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

Track beacon processor import result metrics #6541

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from

Conversation

dapplion
Copy link
Collaborator

@dapplion dapplion commented Oct 26, 2024

Issue Addressed

Our current metrics to track the result of block imports are incomplete. A block import can be triggered by any block component, both from gossip and RPC. We should metric the AvailabilityProcessingStatus of block, blob, and custody column import to count all possible import events.

Also, I would like to track all errors that can occur during import. For example BlockError::DuplicateFullyImported, which show the efficiency of sync.

Proposed Changes

Track the return of all process_* functions with the same helper register_process_result_metrics. Track the result FullyImported / MissingComponents / Error labeled by block component and provenance.

Additional Info

This PR has no breaking changes w.r.t. current metrics. However, it fixes the following metric:

  • beacon_processor_rpc_block_imported_total: This metric is incorrect. Blocks with blobs synced by lookup sync are never tracked, as the block is always imported first and blobs second.

@dapplion dapplion added the ready-for-review The code is ready for review label Oct 26, 2024
@dapplion dapplion force-pushed the beacon-processor-error-metric branch from 92e801b to dc81bbd Compare October 26, 2024 13:44
LazyLock::new(|| {
try_create_int_counter_vec(
"beacon_processor_import_errors_total",
"Total number of block components verified",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be Total number of block components that was not verified

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, just a nit on the metric description and some merge conflicts, otherwise looks good!

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Dec 13, 2024
@dapplion dapplion force-pushed the beacon-processor-error-metric branch from dc81bbd to 77e8e8b Compare December 14, 2024 06:40
@dapplion dapplion requested a review from jimmygchen December 14, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants