feat: add logs to track tx and block propagation delay #1184
Conversation
WalkthroughThis change set primarily updates logging levels and adds new debug log statements across several Ethereum protocol handler files to improve log granularity and consistency. Additionally, the patch version constant is incremented. There are no changes to the core logic, control flow, or exported interfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant Handler
participant Peer
participant Logger
Handler->>Logger: Debug log (block/tx broadcast info)
Handler->>Peer: Broadcast block/transactions
Peer->>Logger: Trace log (transaction request)
Handler->>Logger: Trace log (broadcast/announce events)
sequenceDiagram
participant ProtocolHandler
participant Logger
participant Peer
ProtocolHandler->>Logger: Trace log (decoding failures, nil txs)
ProtocolHandler->>Logger: Debug log (received block info)
ProtocolHandler->>Logger: Debug log (received pooled tx info)
ProtocolHandler->>Peer: Process message
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
eth/handler.go (2)
524-524: Refine transaction broadcast log phrasing
The message"Transactions being broadcasted to"is a bit awkward. Consider simplifying to:- log.Trace("Transactions being broadcasted to", "peer", peer.String(), "len", len(hashes)) + log.Trace("Transactions broadcast to", "peer", peer.String(), "len", len(hashes))
530-530: Refine transaction announcement log phrasing
Similarly, update:- log.Trace("Transactions being announced to", "peer", peer.String(), "len", len(hashes)) + log.Trace("Transactions announced to", "peer", peer.String(), "len", len(hashes))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
eth/fetcher/tx_fetcher.go(1 hunks)eth/handler.go(4 hunks)eth/protocols/eth/handlers.go(5 hunks)eth/protocols/eth/peer.go(1 hunks)params/version.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: Analyze (go)
🔇 Additional comments (17)
params/version.go (1)
27-27: Version increment for logging enhancementsThe patch version is incremented from 44 to 45, which is appropriate for the non-breaking changes being introduced in this PR to enhance logging for transaction and block propagation delay tracking.
eth/protocols/eth/peer.go (1)
439-439: Reduced log verbosity for transaction requestsThe log level has been appropriately changed from Debug to Trace, aligning with the PR objective to reduce the verbosity of existing debug logs to prevent log flooding when tracing is enabled on sampled nodes.
eth/fetcher/tx_fetcher.go (1)
797-797: Reduced log verbosity for transaction schedulingThe log level has been appropriately changed from Debug to Trace, aligning with the PR objective to reduce the verbosity of existing debug logs to prevent log flooding when tracing is enabled on sampled nodes.
eth/protocols/eth/handlers.go (11)
297-297: Added debug log for tracking block propagationThis new debug log is useful for tracking block propagation delay, directly addressing one of the main objectives of the PR to add logging functionality that tracks network latency metrics.
366-366: Reduced log verbosity for decoding failuresThe log level has been appropriately changed from Debug to Trace for decoding failures, aligning with the PR objective to reduce the verbosity of existing debug logs.
371-371: Reduced log verbosity for transaction handlingThe log level has been appropriately changed from Debug to Trace for transaction handling, aligning with the PR objective to reduce the verbosity of existing debug logs.
383-383: Reduced log verbosity for decoding failuresThe log level has been appropriately changed from Debug to Trace for decoding failures, aligning with the PR objective to reduce the verbosity of existing debug logs.
388-391: Added detailed logging for transaction propagationThe new debug logs for transaction propagation will help track the transaction propagation delay (a key objective of this PR). The detailed logging includes both trace and debug level entries to provide comprehensive tracking while minimizing log flooding.
434-434: Reduced log verbosity for transaction decoding failuresThe log level has been appropriately changed from Debug to Trace for transaction decoding failures, consistent with the PR objective to reduce verbosity.
437-437: Reduced log verbosity for transaction handlingThe log level has been appropriately changed from Debug to Trace for transaction handling, consistent with the PR objective to reduce verbosity.
443-443: Reduced log verbosity for nil transaction handlingThe log level has been appropriately changed from Debug to Trace for nil transaction handling, consistent with the PR objective to reduce verbosity.
460-460: Reduced log verbosity for decoding failuresThe log level has been appropriately changed from Debug to Trace for decoding failures, consistent with the PR objective to reduce verbosity.
463-463: Reduced log verbosity for pooled transaction handlingThe log level has been appropriately changed from Debug to Trace for pooled transaction handling, consistent with the PR objective to reduce verbosity.
469-469: Reduced log verbosity for nil transaction handlingThe log level has been appropriately changed from Debug to Trace for nil transaction handling, consistent with the PR objective to reduce verbosity.
eth/handler.go (3)
458-458: Block broadcast start logging added
The new debug log inBroadcastBlockrecords the block hash, number, and size before propagation—this is a useful instrumentation for measuring propagation latency.
475-475: Verifyblock.ReceivedAtis populated
The trace log calculates delay withtime.Since(block.ReceivedAt). Please confirm thatblock.ReceivedAtis consistently set when the block is received from peers so that the measured propagation delay is accurate.
483-483: Verifyblock.ReceivedAton announcement
Similarly, the announcement trace log relies onblock.ReceivedAt. Ensure thatReceivedAtremains valid through both propagation and announcement phases.
1. Purpose or design rationale of this PR
Notion task: https://www.notion.so/scrollzkp/N1-Benchmark-monitor-network-latency-1eb7792d22af801dacbdc86ea433199d
This PR adds logs to track the following metrics:
This PR also decreases some
debuglogs totracelogs since they are not used anymore, so that when enabling the "tracing logs" to track the above delay metrics in sampled nodes. The logs won't flood.The remaining tasks to build Grafana dashboards: some scripts to aggregate and calculate timestamp diffs based on Loki logs, grouped by transaction hash and block hash.
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
Style
Chores