i2p: fix and improve logs #29833
i2p: fix and improve logs #29833
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
brunoerg
force-pushed
the
2024-04-i2p-log
branch
from
64d3296 to
9fa5033
Compare
|
Force-pushed addressing #29833 (comment). Thanks, @laanwj. |
|
Thanks! |
|
CI failure is unrelated to these changes. |
brunoerg
force-pushed
the
2024-04-i2p-log
branch
from
9fa5033 to
469e483
Compare
|
Rebased to re-run CI. |
Better to ask someone in the IRC channel to re-run the CI, as that won't invalidate ACKs 😄 re-ACK 469e4834bad295f6d9fbf8048a8db23aa758bac9 |
vasild
left a comment
vasild
left a comment
There was a problem hiding this comment.
ACK 469e4834bad295f6d9fbf8048a8db23aa758bac9
There was a problem hiding this comment.
Concept/approach ACK, this is the same or very similar to the changes in fc0a50e (#25203) and 43315a0 (#25203) from two years ago, in case you'd like to compare/check against them.
There was a problem hiding this comment.
(reposting using the "review changes" button to appease the bot, it sent me several "request review" notifications in the last ten minutes :)
ACK modulo comments above. If the third commit is re-changing lines that are already changed in the first commit, would suggest combining them.
Not sure why that would happen. The bot should not react on new review comments after someone else's A C K. (c.f. https://github.com/maflcko/DrahtBot/blob/main/webhook_features/src/features/summary_comment.rs#L308) Looking at other pull requests, this seems to be working correctly, so my guess is that this is another GitHub metatdata corruption bug, which are common? |
|
Force-pushed addressing #29833 (comment). |
Co-authored-by: Vasil Dimov <[email protected]>
Before, interruption was printed as an error. Also, it did not log the reason when an interruption happened, e.g. "Error accepting:". Co-authored-by: Vasil Dimov <[email protected]>
brunoerg
force-pushed
the
2024-04-i2p-log
branch
from
0c63848 to
7d3662f
Compare
|
Force-pushed addressing #29833 (comment) |
|
ACK 7d3662f |
, bitcoin#28078, bitcoin#27071, bitcoin#28077, bitcoin#28692, bitcoin#29813, bitcoin#30085, bitcoin#29833, partial bitcoin#24356 (networking backports: part 11) 5699158 merge bitcoin#29833: fix and improve logs (Kittywhiskers Van Gogh) 95a1853 merge bitcoin#30085: detect addnode cjdns peers in GetAddedNodeInfo() (Kittywhiskers Van Gogh) 9d959d7 merge bitcoin#29813: improve `-i2pacceptincoming` mention (Kittywhiskers Van Gogh) 5045fa3 merge bitcoin#28692: Delete i2p fuzz test (Kittywhiskers Van Gogh) 7b01e8b merge bitcoin#28077: also sleep after errors in Accept() & destroy the session if we get an unexpected error (Kittywhiskers Van Gogh) 0c42410 merge bitcoin#27071: Handle CJDNS from LookupSubNet() (Kittywhiskers Van Gogh) 821c11f merge bitcoin#28078: remove unneeded exports, use helpers over low-level code, use optional (Kittywhiskers Van Gogh) d051929 refactor: replace external `::GetLocal()` usage (Kittywhiskers Van Gogh) 481175f merge bitcoin#27719: remove Tor link & generalize onion getnodeaddresses RPC (Kittywhiskers Van Gogh) 378ad01 merge bitcoin#25989: abort if i2p/cjdns are chosen via -onlynet but are unreachable (Kittywhiskers Van Gogh) 8b23bfb partial bitcoin#24356: replace CConnman::SocketEvents() with mockable Sock::WaitMany() (Kittywhiskers Van Gogh) 1a64e8d merge bitcoin#25286: remove duplicate categories from LogPrint output (Kittywhiskers Van Gogh) Pull request description: ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 5699158 PastaPastaPasta: utACK 5699158 Tree-SHA512: b1ffc310574de5ec40d18aa4a453ef328d8f935c42d43a204c14fa264f099a106ddea793a018f56dcf75d9548fb06d1580c6aa1d90f66afa95089435e0b9606d
Summary: Co-authored-by: Vasil Dimov <[email protected]> This is a backport of [[bitcoin/bitcoin#29833 | core#29833]] Bypasses a one line change from [[bitcoin/bitcoin#25355 | core#25355]] that replaces a `LogPrintfCategory` by a `Log`. Removing that call to `LogPrintfCategory` allows to remove the macro after [[bitcoin/bitcoin#28318 | core#28318]]. Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D18268
9 participants
This PR improves and fixes i2p logs (joint work with vasild).
LogPrinttoLogPrintLevelso we can log according to the severity.Accept. Before this PR, when an interruption happens, it just logs "Error accepting:", no reason is logged as it does for other situations. This PR changes it to log "Accept interrupted".