kernel: make logging callback global #34775

1. 
   stickies-v commented at 8:35 am on March 9, 2026: contributor

   The underlying LogInstance() is a process-wide singleton, and all logging configuration (categories, levels, options) are already global. The per-connection btck_LoggingConnection handle gives the false impression of independent logger instances.

   Fix this by explicitly making the entire kernel logging interface explicitly global, simplifying its usage. Replaces btck_LoggingConnection and its {create,destroy} functions with a single btck_logging_set_callback function. The KernelLogger class is introduced to hold global kernel logging state, which will be used in future work such as #34374.

   To an extent, this approach is an alternative to #30342 which contextualizes logging. While I do believe contextualized logging would be a natural fit for the kernel interface, it seems there is currently (in my view) too little demand for the scope of changes necessary. That’s why I’ve opened this PR as a much more straightforward way to fix kernel’s current logging weirdness. If you believe kernel logging should be contextualized right away, please go review #30342 instead.

2. kernel: make logging callback global

   The underlying LogInstance() is a process-wide singleton, and all
   logging configuration (categories, levels, options) are already
   global. The per-connection btck_LoggingConnection handle gives the
   false impression of independent logger instances.
   
   Replace btck_logging_connection_create/destroy with a single
   btck_logging_set_callback that sets or clears the global callback.
   Internally, a KernelLogger singleton manages the LoggingConnection
   lifetime, intentionally leaked on exit to avoid static destruction
   order issues (matching LogInstance).

   dafe5eec14
3. DrahtBot added the label Validation on Mar 9, 2026
4. 
   DrahtBot commented at 8:36 am on March 9, 2026: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	sedited

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #33847 (kernel: Improve logging API by ryanofsky)
   • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)

   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.

   LLM Linter (✨ experimental)

   Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

   • g_kernel_logger().SetCallback(nullptr, nullptr, nullptr) in src/kernel/bitcoinkernel.cpp
   • btck_logging_set_callback(nullptr, nullptr, nullptr) in src/kernel/bitcoinkernel_wrapper.h

   2026-03-09 08:36:13

5. 
   sedited commented at 11:12 am on March 9, 2026: contributor

   While I do believe contextualized logging would be a natural fit for the kernel interface, it seems there is currently (in my view) too little demand for the scope of changes necessary.

   It really isn’t a feature/issue that is sorely missed for the foreseeable future in the kernel library. There are a few applications where it would be a nice to have. Logs coming from different sources, for example in the units tests, can be a little confusing sometimes, but even that has been manageable, and I don’t think I’ve heard many complain about that.

   Not having separate connections seems simpler. Clients of the library can still multiplex the single callback on their side in whatever fashion they wish to. There are many other (and more interesting problems imo) to be working on, so this seems like the pragmatic thing to do for now.

   Concept ACK

6. sedited requested review from alexanderwiederin on Mar 9, 2026
7. 
   purpleKarrot commented at 1:12 pm on March 9, 2026: contributor

   I would go a step further. Since the log function is global, there is no need for local user data. It can be a simple function pointer, even in the C++ wrapper.
8. 
   stickies-v commented at 2:35 pm on March 9, 2026: contributor

   Since the log function is global, there is no need for local user data.

   Forcing the user to use globals if they need state seems unnecessary, when using user_data is I think a well-established pattern in C APIs? Unless I’m misunderstanding your suggested approach?

9. 
   ryanofsky commented at 3:09 pm on March 9, 2026: contributor

   I think it would be a mistake to limit functionality of the logging/debuging/tracing API prematurely, in a way that seems difficult to reverse and does not seem to provide a noticeable simplification (possible I missed it, I have not reviewed the code much yet!). Anyway I have many more thoughts on this topic and can post them later.

   One thing I did want to say is I don’t think #30342 is the only alternative to this and other approaches should be possible. Also I am actively working on #30342, and will push a significant update to it today or tomorrow. So if there are problems with that approach, I would like to at least try to pin them down and address them.

   My preference in terms of review & development effort would be to undraft your other PR #34374. I rebased that PR recently #34374 (comment) and think is ready for review, and that it makes improvements that should have many more benefits to applications than this PR. Personally, I would like to review it and see it merged ASAP.

10. ?
    added_to_project_v2 sedited
11. ?
    project_v2_item_status_changed github-project-automation[bot]
12. ?
    project_v2_item_status_changed sedited

Contributors
stickies-v DrahtBot sedited purpleKarrot ryanofsky

Review Requested
alexanderwiederin

Labels
Validation