RFC on logging improvements #20576

1. 
   jonatack commented at 11:12 am on December 5, 2020: member

   Perhaps we can consider creating different levels of net logging.

   For instance, we could separate lower-frequency, important peer-level events (netpeers) from very high-frequency message-level passing (netmessages).

   Categories and naming suggestions welcome.

   One further suggestion by @jnewbery was:

   No objections. I’d take it ever further though, and add an (optional) logging severity (DEBUG/INFO/WARNING/ERROR or similar) that can be added to all log messages. The user can then either choose what severity logs they want for each category (eg = -debug=net:warning,tor:debug etc), or have a logging post-processor that can filter by severity/category.

   I like the idea of optional logging levels (debug/info/warning/error) for each category, including the default debug log, but agreement on which events go into which level may be difficult to achieve.

   To begin with, I propose separating the net logging into at least two categories. Thoughts? Implementation suggestions?

2. jonatack added the label Feature on Dec 5, 2020
3. jonatack renamed this:
   Separate `NET` logging into high-level and low-level categories `
   Separate `NET` logging into high-level and low-level categories
   on Dec 5, 2020
4. fanquake added the label Utils/log/libs on Dec 5, 2020
5. 
   practicalswift commented at 4:44 pm on December 5, 2020: contributor

   Concept ACK: this would allow us to be more strict about not doing any unconditional logging for network events that are trivially triggerable by an attacker. (Context: One argument against making such logging conditional instead is that NET is currently too noisy.)
6. jonatack closed this on Dec 7, 2020

   ---

7. jonatack reopened this on Jan 12, 2021

   ---

8. 
   jonatack commented at 4:08 pm on January 12, 2021: member

   I’ve re-opened this in the hope that it may see Concept ACKs or that someone might implement it.
9. 
   ghost commented at 7:56 pm on February 1, 2021: none

   +1 I wanted to log onion connections. Looks like net category does this. But that logs also all transactions etc. really noisy.
10. 
    jonatack commented at 12:38 pm on February 15, 2021: member

    Linking here to some ideas elsewhere on this topic:

    • #21181 (comment)

    Maybe it would help to simply remove the screaming ERROR in all uppercase.

    Yes screaming is generally bad but apart from that I think it makes sense to have multiple severities in logging. E.g. other types of error that happen in DeserializeFileDB (say, specific de-serialization errors) might not be fatal but would ideally trigger someone to take a closer look. Something @gmaxwell suggested once is to replace it with UNEXPECTED. c-lightning does this as well.

    • #21181 (comment)

    I think we should be consistent in the way this [the logging] is treated. Someone will create a pull in x months to fix this for banlist.dat, then for peers.dat in y months. So finally we have 5 different error messages for the same condition.

    • Failed to read fee estimates from {}. Continue anyway.
    • Failed to open mempool file from disk. Continuing anyway.
    • Could not find anchors file {}
    • Could not find asmap file {}
    • … Another message for banlist and peers?

11. jonatack renamed this:
    Separate `NET` logging into high-level and low-level categories
    RFC on logging improvements
    on Feb 15, 2021
12. 
    ghost commented at 1:03 pm on February 15, 2021: none

    See also #21102.
13. laanwj added the label good first issue on Feb 15, 2021
14. 
    sidd4u commented at 2:35 pm on March 8, 2021: none

    hi, as a new contributor to open source, can I get opportunity to work on this request.
15. 
    jonatack commented at 4:04 pm on March 8, 2021: member

    Hi @sidd4u, no need to ask; feel free to propose your solution via a pull request. Thanks!
16. fanquake deleted a comment on Jun 14, 2021
17. 
    AditiThirdEye commented at 4:25 pm on August 18, 2021: none

    Can you please assign this issue to me if PR is not done yet.
18. 
    jonatack commented at 8:44 pm on August 18, 2021: member

    Hi @AditiThirdEye, thanks! No need to ask; feel free to open a pull request after becoming acquainted with the guidelines in CONTRIBUTING.md and doc/developer-notes.md 👍
19. 
    aswin6197 commented at 5:19 am on October 24, 2021: none

    Hi @jonatack did you something like have a new enum with two values that’s passed to the logPrint similar to LogFlags and added to logs, so it’s easier to filter out important messages?
20. 
    klementtan commented at 7:13 am on March 3, 2022: contributor

    @jonatack @jnewbery I took a stab at this in #24464 and I tried to add log serverity levels (ie [net:warning]) in the logs. I plan to extend this further (in a separate PR) to allow users to specify the minimum level of logs. Would love to get your feedback on it 😄
21. laanwj referenced this in commit 90e49c1ece on May 24, 2022
22. jonatack referenced this in commit 73d430d2b9 on May 24, 2022
23. sidhujag referenced this in commit 2f111e0cd8 on May 28, 2022
24. 
    jonatack commented at 7:21 pm on May 31, 2022: member

    Closing this as completed by #24464 and current follow-ups.
25. jonatack closed this on May 31, 2022

    ---

26. DrahtBot locked this on May 31, 2023

Contributors
jonatack practicalswift ghost sidd4u AditiThirdEye aswin6197 klementtan

Labels
Feature Utils/log/libs good first issue