Skip to content

Conversation

@fedacking
Copy link
Contributor

Motivation

The logs in P2P use 4 utility function to log what the peer did. (log_peer_debug, log_peer_error, log_peer_trace, log_peer_warn) These functions obscure which file triggered the log in the tracing macros. Changing those functions to macros, changes this behaviour to show the proper file. Example:
2025-10-20T21:27:08.447426Z ERROR ethrex_p2p::rlpx::utils: reth/[0xddd9…df1e(194.233.75.31:30303)]: Broken pipe with peer, disconnected
2025-10-20T21:34:24.178984Z ERROR ethrex_p2p::rlpx::connection::server: reth/[0x6101…3c5f(54.87.135.143:30303)]: Broken pipe with peer, disconnected

Description

@github-actions github-actions bot added L1 Ethereum client L2 Rollup client labels Oct 20, 2025
@fedacking
Copy link
Contributor Author

Waiting until base branch start_on_thread is merged

@github-actions
Copy link

github-actions bot commented Oct 20, 2025

Lines of code report

Total lines added: 10
Total lines removed: 2
Total lines changed: 12

Detailed view
+-----------------------------------------------------------+-------+------+
| File                                                      | Lines | Diff |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection/handshake.rs | 526   | -2   |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/utils.rs                | 110   | +10  |
+-----------------------------------------------------------+-------+------+

@ElFantasma ElFantasma marked this pull request as ready for review October 21, 2025 02:06
@ElFantasma ElFantasma requested a review from a team as a code owner October 21, 2025 02:06
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 21, 2025
@ElFantasma
Copy link
Contributor

Updated the branch and marked ready for review as start_on_thread has been merged

@MegaRedHand
Copy link
Collaborator

Did you try using #[track_caller]? I understand it's meant for something similar, but for panics. Maybe it works for this as well?

@fedacking
Copy link
Contributor Author

Did you try using #[track_caller]? I understand it's meant for something similar, but for panics. Maybe it works for this as well?

It doesn't seem to work on my testing with info!(). I think from the discussion it works with panic by making it a special case.

@fedacking fedacking enabled auto-merge October 21, 2025 21:11
@fedacking fedacking added this pull request to the merge queue Oct 21, 2025
Merged via the queue into main with commit ec56d72 Oct 21, 2025
29 checks passed
@fedacking fedacking deleted the p2p_log_macros branch October 21, 2025 21:47
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client L2 Rollup client

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants