waku-org / nwaku

Waku node and protocol.
Other
199 stars 51 forks source link

chore: log the full peerIds #2264

Open richard-ramos opened 9 months ago

richard-ramos commented 9 months ago

Background

The peerIDs are currently being logged in an abbreviated form like peerId=16U*TCnsXV instead of 16Uiu2HAmSve7tR5YZugpskMv2dmJAsMUKmfWYEKRXNUxRaTCnsXV. This is annoying at the time of debugging because you need to modify the peerId you're looking for to match the format used in nim-libp2p. Also, in some places we're logging the full peerId by prefixing it with $, while in others we just use the default.

To fix this situation, I tried to override https://github.com/status-im/nim-libp2p/blob/ce0685c272e41614e97583f0a8dec2b6506913fd/libp2p/peerid.nim#L48 as suggested by @alrevuelta but i was not successful, so ended up quickly doing a branch In https://github.com/waku-org/nwaku/compare/chore/log-peerid in which I try to do this by using $ but it kinda sucks because the compiler complains about =$ not existing, so I had to end up adding a space right before the $ sign, and honestly it looks bad.

I'd prefer to do this via overriding the function since then no one would have to remember to add a $ when logging the peerIds, but i'm not sure how to do this. Any idea on how this can be achieved?

cc: @arnetheduck, @Ivansete-status

arnetheduck commented 9 months ago

This sounds mostly like a matter of taste and habit - I have no strong opinion one way or the other except to note that nim-libp2p has logged the peerid this way for years, and I don't remember anyone being dissatisfied (ie nimbus logs this way).

The strategy nimbus employs is to log it once in full form (with $) and then use the short form to reference it - it works well for grep etc and keeps all logs that reference some peer short enough that we can focus / prioritise on other, more useful information.

=$ in Nim is just another function - perhaps you were thinking that these would be parsed as two operators, but they are not - unlike go, nim allows you to customize operators simiar to c++ (to allow creating natural custom data types) - it's a single function call and you use to to declare your own operators:

proc `=$`(v: int): string = $(v + 5)
echo(=$42)

Regarding space around =, this ends up being common practice in Nim, and is again, I believe, a matter of habit more than aesthetics - we usually add space around operators like = and when it's done consistently, it's hard to fault (ie consistency is usually more important than choosing one way or the other, for achieving flow when reading code)