fix: peerdas metrics

[vedant-asati]

wip #8850

[gemini-code-assist]

Summary of Changes

Hello @vedant-asati, I'm Gemini Code Assist¹! 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 significantly enhances the observability of the Peer Data Availability Sampling (PeerDAS) system by refining its metrics collection. The changes introduce precise measurements for various stages of data column and blob processing, from reconstruction and computation times to different verification steps. This will allow for a more accurate understanding of performance bottlenecks and the overall efficiency of the PeerDAS implementation, ensuring better monitoring and debugging capabilities.

Highlights

• Metrics Refinement: Introduced new metrics and adjusted existing ones to provide more granular insights into PeerDAS operations, including reconstruction times, verification processes, and data column handling.
• Performance Monitoring: Implemented timers around critical operations such as blob and data column reconstruction, KZG proof verification, and inclusion proof verification to measure execution duration.
• Data Column Tracking: Added counters to track the number of reconstructed blobs and data columns, as well as requests and successes for data column sidecar processing. A new metric dataColumns.alreadyAdded was introduced to count columns that were already present when attempting to add them.
• Metric Renaming and Initialization: Renamed targetCustodyGroupCount to custodyGroupCount and initialized a new custodyGroupsBackfilled metric to zero, reflecting ongoing development in custody group management.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• packages/beacon-node/src/api/impl/beacon/blocks/index.ts
  • Added recoverBlobSidecars.reconstructionTime and blobsReconstructed metrics for blob reconstruction.
  • Introduced peerDas.dataColumnSidecarComputationTime metric for per-column computation during blob reconstruction.
  • Added peerDas.dataColumnsReconstructionTime and peerDas.reconstructedColumns metrics for data column reconstruction.
• packages/beacon-node/src/chain/blocks/importBlock.ts
  • Updated metric path from importBlock.columnsBySource to dataColumns.bySource for tracking imported data columns.
• packages/beacon-node/src/chain/chain.ts
  • Initialized peerDas.custodyGroupsBackfilled metric to 0.
  • Renamed peerDas.targetCustodyGroupCount to peerDas.custodyGroupCount.
• packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts
  • Added peerDas.dataColumnSidecarComputationTime metric around kzg.computeCells.
  • Added peerDas.kzgVerificationDataColumnBatchTime metric around validateCellsAndKzgCommitments.
• packages/beacon-node/src/chain/validation/dataColumnSidecar.ts
  • Added a comment clarifying dataColumnSidecarKzgProofsVerificationTime is for single verification.
  • Wrapped validateBlockDataColumnSidecars in a try...finally block to ensure dataColumnSidecarGossipVerificationTime is always recorded.
  • Added peerDas.dataColumnSidecarProcessingRequests and peerDas.dataColumnSidecarProcessingSuccesses metrics.
  • Added peerDas.dataColumnSidecarInclusionProofVerificationTime metric.
  • Ensured peerDas.kzgVerificationDataColumnBatchTime metric is consistently recorded.
• packages/beacon-node/src/metrics/metrics/beacon.ts
  • Updated help text for dataColumnSidecarKzgProofsVerificationTime to specify 'single' verification.
  • Added a comment for kzgVerificationDataColumnBatchTime indicating 'batch verification'.
  • Renamed targetCustodyGroupCount to custodyGroupCount.
  • Added custodyGroupsBackfilled gauge metric.
• packages/beacon-node/src/metrics/metrics/lodestar.ts
  • Moved and renamed recoverDataColumnSidecars.recoverTime to recoverBlobSidecars.reconstructionTime.
  • Added recoverBlobSidecars.blobsReconstructed counter.
  • Removed recoverDataColumnSidecars.numberOfColumnsRecovered.
  • Added dataColumns.alreadyAdded counter.
  • Removed importBlock.columnsBySource.
• packages/beacon-node/src/sync/utils/downloadByRange.ts
  • Added a Todo comment regarding passing chain to validateBlockDataColumnSidecars for Fulu metrics.
• packages/beacon-node/src/util/dataColumns.ts
  • Changed recoverTime metric to peerDas.dataColumnsReconstructionTime.
  • Added dataColumns.alreadyAdded metric increment when columns are already present during recovery.
  • Added peerDas.reconstructedColumns and dataColumns.bySource metrics after successful recovery.
  • Added a Todo comment for dataColumns.sentPeersPerSubnet metric.
• packages/beacon-node/src/util/execution.ts
  • Added peerDas.dataColumnSidecarComputationTime metric around the computation of dataColumnSidecars.
  • Added a Todo comment for dataColumns.sentPeersPerSubnet metric.
  • Implemented alreadyAddedColumnsCount to track columns already present.
  • Updated dataColumns.bySource metric to subtract alreadyAddedColumnsCount for engine-sourced columns.

Activity

• The pull request is a work-in-progress (wip) addressing issue PeerDAS Metrics Revamp: Fix phantom metrics, add missing dashboard panels, improve observability #8850, focusing on fixing and enhancing PeerDAS metrics.
• Multiple metric definitions and their usage across various modules have been updated to provide more detailed performance insights.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[vedant-asati]

idk why packages/beacon-node/src/chain/validation/dataColumnSidecar.ts diffs aren't rendering properly

[gemini-code-assist]

Code Review

This pull request introduces several new metrics for PeerDAS and refactors some existing ones. The changes are generally good and improve observability. I've identified a few areas where code clarity could be improved by avoiding variable shadowing, and one instance of potentially inconsistent metric usage. My suggestions focus on making the code more readable and maintainable.

[gemini-code-assist]

The metric dataColumnSidecarComputationTime is used here to time blobsBundle.blobs.map((blob) => kzg.computeCells(blob)). The help text for this metric is "Time taken to compute data column sidecars, including cells and inclusion proof". This seems inconsistent as only cells are being computed here. In other parts of the codebase (e.g., util/execution.ts), this metric is used to time the entire sidecar computation process. Consider using a more specific metric for cell computation or clarifying the scope of this metric to ensure consistency.

[vedant-asati]

Cell proofs come from EL, inclusion proofs may be computed afterwards in the near scope.
We are only timing cell computation here.
This behaviour is not fully consistent with what the spec says.