This repository was archived by the owner on Jul 1, 2021. It is now read-only.
Allow DEBUG2-logging in sub-protocols + nitpick debug messages + various
#113
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
veox
force-pushed
the
nitpick-debug-messages
branch
from
2c928de to
05b7a43
Compare
veox
changed the title
--help
veox
force-pushed
the
nitpick-debug-messages
branch
3 times, most recently
from
77f2bf5 to
e14dfa2
Compare
veox
changed the title
--help--help + update LICENSE date
pipermerriam
approved these changes
Jan 5, 2019
veox
changed the title
--help + update LICENSE date--help + update LICENSE date
veox
changed the title
--help + update LICENSE date--help + update LICENSE date
veox
force-pushed
the
nitpick-debug-messages
branch
2 times, most recently
from
d1ef4eb to
4460871
Compare
|
@jannikluhn @hwwhww @ChihChengLiang I've taken the liberty of touching BCC (not just ETH) sub-protocol in this PR, in commit 69fc919. (For ETH, it was 9890146.) The Do ping if this change is undesired, it's (still) easily-revertable. |
veox
changed the title
--help + update LICENSE dateDEBUG2-logging in sub-protocols + update --help + update LICENSE date
|
@pipermerriam I think this change - allowing EDIT: I've squashed the "small" nit-picking commits into one. BTW, I wonder why github doesn't clear your review approval. The code has changed, with force-pushes... |
veox
changed the title
DEBUG2-logging in sub-protocols + update --help + update LICENSE dateDEBUG2-logging in sub-protocols + nitpick debug messages + various
veox
force-pushed
the
nitpick-debug-messages
branch
2 times, most recently
from
3ee74be to
f6ebea1
Compare
4 commits squashed:
* logging/protocol: display bytes as hex in debug log.
Three messages showed trie nodes and block hashes as-is:
b'\xc2m!\xfe\xc5\xf4zNA/\x0b}eD2r\x851R~5\x8aA0D/\xfe}\xd8\x12>\xde'
They are now converted to hex strings before printing to log.
* p2p/discovery: display Kademlia target in hex, not int.
The target is often (only?..) chosen randomly, as a 256-bit number.
Although it's passed in as an int (for simplicity?..), displaying it
like so looks confusing.
* logging/p2p: Do print peer when disconnecting.
Although there's often a follow-up debug message displaying both
the peer and the reason verbosely, we should print a short-hand
name of the peer here; otherwise, the message is near-useless in
a multi-peer setup:
Disconnecting from remote peer; reason: too_many_peers
This is only useful when debugging with a single peer; compare to:
Disconnecting from remote peer <Node([email protected])>; reason: too_many_peers
This can already be used in fail2ban-like setups, to limit peer
churn; the generic protocol-level reason is useful it this
context, since `too_many_peers` and `useless_peer` can be
differentiated into separate filters, but there's no need to
parse complex user-level log messages like
... network (901025) does not match ours (1) ...
* trinity/sync/common/{chain,headers}: add missing %s to DEBUG2 logging.
…el DEBUG2 + do so in eth/proto.
ETH/Status message to be logged at DEBUG2 level, not DEBUG.
This is the only logging statement in the file, and it contains
best/genesis block hashes that are displayed as-is (b'\xd4..\xa3').
Displaying them nicely would require making a copy of the logged
data structure, or modifying it on the fly, slowing things down
needlessly.
The message only needs be logged when debugging handshake failures,
so instead of trying to make it printable, push it "one level down".
Note that the "log level" should theoretically be selectable like
trinity --log-level trinity.protocol.eth.proto.ETHProtocol:DEBUG2
instead of the previous
trinity --log-level p2p.eth.ETHProtocol:DEBUG2
according to help, but that actually doesn't work.
In fact, this seems not a regression, but simply broken already. :(
…2 log-level. ... And, similarly, push sub-protocol-level messages one level down.
veox
force-pushed
the
nitpick-debug-messages
branch
from
f6ebea1 to
3e4f1b2
Compare
There are two places where the options are displayed: * `trinity --help`; * `trinity --log-level SOME-INVALID-LOG-LEVEL-AKA-FOOBAR`. These are formatted differently: the latter takes heed of newlines, the former gets forcibly reformatted. Therefore, the list categories are not delineated with dash/asterisk markers, and only spaces are used.
veox
force-pushed
the
nitpick-debug-messages
branch
from
3e4f1b2 to
d609fc0
Compare
veox
force-pushed
the
nitpick-debug-messages
branch
from
d609fc0 to
290f539
Compare
pipermerriam
approved these changes
Jan 7, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was wrong?
DEBUG2logging level;DEBUG-level logging was a bit too noisy, and displayingint/byteswhere hex-strings make more sense;How was it fixed?
Major:
HasExtendedDebugLogger;ETH/StatusandBCC/Statussub-protocol-level logging was moved to levelDEBUG2.Minor:
to_hex();trinity --helpoutput for log levels now generated on-the-fly;Extra:
LICENSEfile.Detailed reasoning is in commit messages.
Cute Animal Picture