kernel: Attach logging connections to contexts, per-connection log levels #33847

pull ryanofsky wants to merge 7 commits into bitcoin:master from ryanofsky:pr/klog changing 8 files +612 −273
  1. ryanofsky commented at 11:30 PM on November 10, 2025: contributor

    This PR is a draft because it is based on #34374. All new changes are in last commit.


    Problem: Every logging connection receives all kernel log output, regardless of which context produced it. An application that opens two chainstates (say mainnet and signet, or a test creating several contexts) gets one interleaved log stream and can't tell which context a line came from. #30342 can separate them, but only if the application says which logging connection belongs to which context, and the current API has no way to do that: connections aren't passed to any other kernel function, and the only log setting, btck_logging_set_min_level, is process-wide.

    Solution: Add btck_context_options_set_logger, and make btck_logging_set_min_level apply to a single connection:

    btck_LoggingConnection* log = btck_logging_connection_create(callback, user_data, destroy_user_data);
    btck_logging_set_min_level(log, btck_LogLevel_DEBUG);
    btck_context_options_set_logger(options, log);
    btck_Context* context = btck_context_create(options);
    

    A connection receives nothing until a context uses it. Per-connection levels are a small feature, but the main purpose is to change the API now, before more code depends on it: #30342 builds on this to give each context's connection only that context's entries, so code like the above gets separate output per context unchanged. Until then, a connection may also receive entries from other kernel operations, as the header documents. Small change to bitcoinkernel.{h,cpp}, the C++ wrapper, bitcoin-chainstate and test_kernel, based on #34374 (only the last commit is new).

    Alternatives: Keep the current API and change it when #30342 is ready, since the kernel API has no compatibility guarantees. But that change would be expensive for callers: code written today sets up logging once, often far from where contexts are created (in main, a logging module, or when a language binding is imported), because nothing needs the connection afterwards. Per-context output would then mean restructuring every application and binding to get a connection to each place that creates a context. With a connection in the API from the start it costs one extra call, and bindings can map connections to their language's usual per-object loggers (e.g. Python's logging.getLogger(name)). #34775 takes the opposite approach, replacing connections with a single global callback.

  2. maflcko added the label Insufficient Review on Nov 11, 2025
  3. maflcko removed the label Insufficient Review on Nov 11, 2025
  4. DrahtBot added the label Validation on Nov 11, 2025
  5. sedited commented at 2:05 PM on November 11, 2025: contributor

    Concept ACK

    Thank you for contributing this @ryanofsky! We've discussed doing something similar a few months ago. This is close to how I hoped to implement the logging capabilities, but reviewers highlighted some inconsistencies with this approach: A secondary logger object cannot receive its own non-global options. The docstrings do at least still explain this even with this change, I am not sure if it is really less surprising for users. Maybe it would be preferable to push forward #30342 before this is done? Tagging @stringintech and @stickies-v here, since they both left comments on the logging capabilities during review of #30595.

  6. DrahtBot commented at 2:05 PM on November 11, 2025: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    External sites

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    Concept ACK sedited
    Approach NACK stickies-v

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36353 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36353.svg"></sub> (kernel: don't reuse cached CheckBlock results across params by FlashWayne)
    • #36326 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36326.svg"></sub> (kernel: Use typed errors for fatal and flush error notifications by arejula27)
    • #36000 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36000.svg"></sub> (validation: prefetch blocks while connecting by l0rinc)
    • #35641 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35641.svg"></sub> (kernel: Add script evaluation tracer by sedited)
    • #35322 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35322.svg"></sub> (logging: streamline Logger state and drop redundant methods by ryanofsky)
    • #35187 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35187.svg"></sub> (kernel: Block validation without a complete UTXO set by sedited)
    • #34775 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34775.svg"></sub> (kernel: allow only one global logging connection by stickies-v)
    • #34374 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/34374.svg"></sub> (kernel: use struct-based logging and simplify logging interface by stickies-v)
    • #28690 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/28690.svg"></sub> (build: Introduce internal kernel library by sedited)

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  7. ryanofsky force-pushed on Nov 11, 2025
  8. ryanofsky commented at 2:28 PM on November 11, 2025: contributor

    <!-- begin push-2 -->

    Updated d0bd114e974500ad11c8ecac26d6606166ab7438 -> 0926479924f453c38b5e0ad069435fb3241b2389 (pr/klog.1 -> pr/klog.2, compare)<!-- end --> updating btck_logging_connection_create documentation

    <!-- begin push-3 -->

    Rebased 0926479924f453c38b5e0ad069435fb3241b2389 -> 168128bb5e69b10a29732d5af0719adabbb776e1 (pr/klog.2 -> pr/klog.3, compare)<!-- end -->

    <!-- begin push-4 -->

    Rebased 168128bb5e69b10a29732d5af0719adabbb776e1 -> c8f6a474f9bcc72b08e67101aba0962ba66af1aa (pr/klog.3 -> pr/klog.4, compare)<!-- end --> due to conflict with #33728

    <!-- begin push-5 -->

    Rebased c8f6a474f9bcc72b08e67101aba0962ba66af1aa -> 6d370c720a4c106968e73cba67888efb2d7fbb88 (pr/klog.4 -> pr/klog.5, compare)<!-- end --> due to conflict with #31650

  9. ryanofsky commented at 3:29 PM on November 11, 2025: contributor

    re: #33847 (comment)

    Thanks @TheCharlatan. I think the approach you took in #30595 of making the kernel API mirror the current logging.h API was the best one to take as a starting point. #30595 covers a large surface area, and it would have been asking too much of reviewers to evaluate all the different ways the kernel API and existing APIs could or should diverge in one PR. It should be more manageable to make kernel API improvements in targeted followups like this PR, and hopefully the improvements here are welcome.

    On the question of whether to push forward with this PR or #30342 first: I think this PR should have much higher priority than #30342. The reason is that this PR is not backwards compatible and will require downstream changes to language bindings and probably to most external programs using those bindings. By contrast, #30342 is backwards compatible. If #30342 were merged later, it would just improve how requested log options are applied and how log messages are forwarded, without requiring external changes. #30342 is also a bigger PR that will take more effort to review.

    It is true that a potential drawback of merging this PR without #30342 is that the design of the API exposed here could make it seem like it provides more functionality than it currently does. Users would not know about the limitations without reading the documentation, and could be surprised or confused. I just would not expect this to be a problem in practice, because the surprising behavior should only occur if users create multiple log streams or multiple kernel context objects at the same time, and that should be uncommon. And even if it did happen, the consequences would not seem very bad: log options might be applied in a different way than expected, or some log messages might be received unexpectedly. It seems like it would place a bigger burden on users to first roll out a logging API that does not support multiple options and streams and then later change it incompatibly, than to roll out an API that could support them and then implement that support internally later.

  10. stringintech commented at 10:07 PM on November 13, 2025: contributor

    Thanks @ryanofsky for explaining the trade-offs!

    I think I also prefer we push this change after we support separate logger instances per connection though, and at this point keep the API consistent with what it's currently capable of. I'm not sure how big of an impact delaying this would have on bindings/users, especially if we're assuming their use cases are mostly limited to a single logging connection.

  11. ryanofsky commented at 1:29 AM on November 14, 2025: contributor

    Thanks for the feedback @stringintech. Can you explain what specifically you may be looking for which this PR does not provide? The API defined here is consistent and logical. It just happens to be compatible with new features like per-connection options and separate log streams (already implemented in #30342) instead of being pointlessly incompatible with them. It also eliminates the btck_logging_disable() function which is unsafe and confusing. And it gets rid of the kernel's inefficient default behavior of storing up log messages in a 1MB buffer in case a logger is attached at later time, instead taking a simpler and more rational approach: logging when a logger is attached and not logging otherwise. I think if you look at the API and documentation defined here you will see that it makes more sense than the current API and should be less confusing. If there are any specific problems I'd be happy to address them.

    If the logging API is not going to be rationalized soon (it's only been around a week since the API was introduced in #30595), I am concerned it will be more difficult to rationalize later. Logging is a fundamental part of the kernel API so if backwards incompatible changes are made, they are likely to break most language bindings and potentially many applications written in different languages. It should make sense to provide a logging API that's forward compatible and exposes fewer quirks and implementation details sooner instead of later, if we can do that.

  12. stringintech commented at 12:58 PM on November 14, 2025: contributor

    By inconsistency, I mean that refactoring set_options/set_level_category/enable_category/etc. to receive a btck_LoggingConnection* parameter suggests that settings are supported per connection, while calling them on a newly instantiated logging connection would actually override settings for all previously instantiated connections. While the actual behavior is well-documented, the function signatures themselves may still create expectations of per-connection isolation.

    I understand you already acknowledged this concern in your last paragraph here, and believe the benefits of pushing this change forward outweigh this downside. My preference was to wait for #30342, since the API refactoring and implementation naturally seem to be parts of a single patch, though I may be too optimistic about how much time/effort it takes to push that PR forward, or the effort it takes to adapt the bindings/other projects to the breaking changes.

    Regarding the StartLogging() call being moved from the logging connection constructor to context creation: This delay could mean logs won't be produced by context-free operations (like btck_script_pubkey_verify()). If i am not wrong, it appears we don't currently have such operations producing logs, but if we add them in the future while still using a global logger, would we need to move StartLogging() back to the logging connection constructor?

  13. stickies-v commented at 1:59 PM on November 14, 2025: contributor

    Concept ACK for making the logging interface local instead of global, but approach ~0 leaning towards nack. I think the approach that merging the backwards-incompatible interface early is not unreasonable but not my preferred one, because:

    • I don't think we should at all care about breaking the kernel interface at the moment, the kernel API is experimental and upstream development flexibility and pace should absolutely be prioritized until kernel is in a more stable state
    • having the interface behave in unexpected ways is just not desirable, even if this gap is not catastrophic as I think you correctly described
    • internal logging refactorings / changes (e.g. #30342, but also others) may surface roadblocks or ideas that inform us about a better interface design, so waiting a bit more seems helpful to me

    I would prefer to convert this to draft, and have this PR serve as the WIP discussion grounds for polishing up the kernel logging API while we make changes such as #30342. I don't have a proposal ready, but I would also like to make breaking changes to the category/level/enable/disable logic (at least for kernel API) which I think currently is extremely unergonomic and unintuitive, as well as support richer callbacks that do not require users (language bindings, especially) to parse already formatted log strings.

  14. ryanofsky commented at 2:15 PM on November 14, 2025: contributor

    This delay could mean logs won't be produced by context-free operations

    Thanks, I'll look into this. I am a little surprised there would be any context-free operations except very basic ones that shouldn't log. It'd expect there to be some context object, even a minimal one, associated with all non-trivial operations to control things like logging, random number generation, caching, etc.

    Of course I do understand your abstract concern that adding a connection parameter "suggests that settings are supported per connection." This is similar to how in many filesystem API's, flushing one file can cause other files to be flushed to disk. Or how in the python database API, cursor objects have commit() and rollback() methods even though in some implementations calling these can affect other cursors. It's not unusual for APIs to provide a little more generality than is guaranteed in every case, if this provides other benefits and avoids other types of pain.

    So I would like to understand any ways your abstract concern could translate to confusion or problems in practice. When I consider all use-cases I can think of, the new logging API here is less confusing and more coherent than the current one. I already mentioned the current nonsensical default behavior of storing all log messages into a 1MB buffer even when no logging is requested. But lets say you do want to enable logging. You might see the current btck_logging_set_* functions and think they actually do something. Well, they don't, because in order to have logging, you need to create a connection object. Adding a connection parameter to these functions makes it obvious that a connection object is actually necessary. Or, in the current API you might see the btck_logging_connection_create function and notice that takes a callback and returns a btck_LoggingConnection handle. But where is that handle used? Only by the btck_logging_connection_destroy function and nowhere else. In the current API, kernel functions which actually log and set logging options are less discoverable because the API is not coherent and the functions using the connection do not reference it.

    More generally, I'd expect the majority of scripts and applications using the kernel library to only set one logging callback, and in that use-case, the new API should be a strict improvement over the old one. It is true that in the alternate case where an application does create multiple log callbacks, if the application calls btck_logging_set_* functions, and the author did not read the documentation for those functions, they might mistakenly think those options apply to a single callback instead of multiple. But those applications will still benefit from this PR because of the follow-up in #30342 which can apply log options singly. So I am trying to think of any application or use-case which would not benefit from this PR and I don't see one. Would be interested to know if I am looking at this from the wrong perspective or am missing something.

  15. ryanofsky marked this as a draft on Nov 14, 2025
  16. ryanofsky commented at 2:33 PM on November 14, 2025: contributor

    prefer to convert this to draft

    Sure happy to do that while there is conceptual discussion.

    having the interface behave in unexpected ways is just not desirable

    I think I've pointed out practical ways the current interface behaves in unexpected ways and is counterintuitive, and practical ways this PR is an improvement. The concerns you and stringintech raised do seem plausible, but also vague so if there are any concrete scenarios you can think of where this PR might create a problem, they would be helpful to know about.

    Thanks for the thoughtful reviews!

  17. in src/kernel/bitcoinkernel.h:807 in 0926479924 outdated
     802 | + * @brief Sets log instance for the kernel context to use.
     803 | + *
     804 | + * @param[in] context_options  Non-null, previously created by @ref btck_context_options_create.
     805 | + * @param[in] logger           Is set to the context options.
     806 | + */
     807 | +BITCOINKERNEL_API void btck_context_options_set_logger(
    


    stickies-v commented at 1:20 PM on December 10, 2025:

    How do we deal with ownership of the logger here? Is it owned by the context? If yes:

    • should we still expose the logging_connection_destroy function?
    • how do users update the logging_connection options? Their handle should be invalid after ownership is passed to the context? A btck_context_get_logger() could be an alternative?
    • users may need to create multiple logging connections, e.g. to pass to context-free functions that need logging

    If no:

    • managing lifetimes becomes more brittle, i.e. users must ensure logger is kept alive for the duration of context. This feels like a terrible idea, but one approach could be to let the static kernel::Context own the loggers, with the implication that connections can be created, but not destroyed (until the application aborts)?

    ryanofsky commented at 2:27 PM on December 10, 2025:

    Thanks for following up! If I understanding your concerns correctly, I don't think there is any reason to think this PR can cause problems.

    In the kernel API, log messages are only written to logging connections, not to files or other possible log sinks. So with #30342 a "logger" is just a logging connection. If you create a logging connection, you create a BCLog::Logger instance and if you destroy the logging connection you destroy it.

    Before #30342, there is only a single BCLog::Logger instance and all logging connections share it. This PR does not change that. It keeps behavior the same and just adds function parameters so the association between log connections and log sources is explicit, rather than that implicit, and so the more flexible logging enabled by #30342 can be introduced without API changes, binding changes, script and application changes.

    How do we deal with ownership of the logger here?

    It is owned by the logging connection.

    Is it owned by the context?

    No.

    • should we still expose the logging_connection_destroy function?

    Yes.

    • how do users update the logging_connection options? Their handle should be invalid after ownership is passed to the context?

    No, a logging connection is just a handle referring to a BCLog::Logger instance. The handle can be used until the connection is closed.

    • users may need to create multiple logging connections, e.g. to pass to context-free functions that need logging

    No, a logging connection can be passed to different log sources, the log sources do not own the connection.

    • managing lifetimes becomes more brittle, i.e. users must ensure logger is kept alive for the duration of context. This feels like a terrible idea

    No, they only need to keep the logging connection open as long as they want to receive logging callbacks. If they don't want to receive logging callbacks, there is no need for a connection or logger.

    but one approach could be to let the static kernel::Context own the loggers, with the implication that connections can be created, but not destroyed (until the application aborts)?

    This sounds very kludgy and there should be no need for it. (Having a static context in general seems like a kludge to me and I hope there are plans to get rid of it. There should definitely be no need to use it here.)


    stickies-v commented at 2:49 PM on December 10, 2025:

    I agree that with the current implementation (where btck_LoggingConnection just represents a callback) there are no meaningful lifetime concerns, except perhaps conceptually. However, with something like #30342, it does look problematic, since Context would be keeping a reference to the logger instance, which may have already been invalidated by the user (who owns the logger)?

    Assume the following pseudo-code:

    opts = btck_context_options_create();
    logger = btck_logging_connection_create();
    btck_context_options_set_logger(opts, logger);
    ctx = btck_context_create(opts);
    btck_logging_connection_destroy(loger);
    

    I think ctx is now in an invalid state? We had a similar concern in earlier versions of this PR, where context had to be kept alive for the duration of chainman, so eventually the approach was changed to let chainman own the context it was created with. Arguably, the same could be done here, as per the bullet points under the "yes" section? It's currently my preferred approach over passing just a view (which, I think should be passed by const reference and documented as such, if you decide to keep it).

    In my previous comment, in the context-owns-logger approach, I suggested we might want to remove logging_connection_destroy. Of course, if/when we have context-free functions that need logging, it would be perfectly fine to pass a stand-alone btck_LoggingConnection, at which point exposing logging_connection_destroy would make sense again.


    ryanofsky commented at 3:50 PM on December 10, 2025:

    I see comment has been edited, but just to respond to the example, since it is illustrative:

    opts = btck_context_options_create();
    logger = btck_logging_connection_create();
    btck_context_options_set_logger(opts, logger);
    ctx = btck_context_create(opts);
    btck_logging_connection_destroy(loger);
    

    I'd want to resolve this by adding a int use_count field to the LoggingConnection struct and making the btck_logging_connection_destroy call fail if the use_count is not 0.

    I would imagine using a similar pattern in other parts of the kernel when references to handles need to be stored.

    I don't think it would be smart in general to work around the need to share handles by resorting to using global variables or large context objects, or by limiting flexibility of the API.


    stickies-v commented at 4:40 PM on December 10, 2025:

    I was actually wrong about my earlier example about chainman owning context: it is passed as a non-owning (const ptr) view, and lifetime then ensured via std::shared_ptr. Whether we prevent destroying or automatically extend lifetimes is not really my concern here, so I think we can wrap this up on the conclusion that you prefer letting the user manage btck_LoggingConnection's lifetime (with safeguards in place), which I think works well.

    Thank you for the in-depth response. Essentially, the misunderstanding I had is that if the context doesn't own the logging connection it can't ensure its lifetime. I now understand that's not actually true, and we already use this pattern in existing kernel code, e.g. with context associating with chainman. I agree then that there is no need for context to own the logging connection.


    ryanofsky commented at 6:59 PM on December 10, 2025:

    re: #33847 (review)

    my earlier example about chainman owning context: it is passed as a non-owning (const ptr) view, and lifetime then ensured via std::shared_ptr.

    That's pretty interesting. I can see how the lifetime extension idea also works, although my first reaction is that it seems a little fragile, and it seems safer in general to return an error to applications if they try to delete objects that are currently in use, instead of internally giving the objects longer lifetimes.

    In the case of btck_LoggingConnection, I agree either approach could work. The only nuance is that if a shared_ptr is used, regardless of whether the logger object is freed, btck_logging_connection_destroy needs to unregister the logging callback function so the kernel does not try to call the application after it is no longer expecting calls.

  18. ryanofsky force-pushed on Dec 12, 2025
  19. DrahtBot added the label Needs rebase on Jan 14, 2026
  20. ryanofsky force-pushed on Jan 18, 2026
  21. DrahtBot removed the label Needs rebase on Jan 18, 2026
  22. DrahtBot added the label Needs rebase on Jan 19, 2026
  23. ryanofsky force-pushed on Jan 22, 2026
  24. DrahtBot removed the label Needs rebase on Jan 23, 2026
  25. DrahtBot added the label Needs rebase on Sep 15, 2026
  26. kernel: add missing log levels and categories
    There are log levels and categories emitted by kernel code that
    are not yet part of the C API. Once entries are delivered as
    structs in a future commit, every emitted value must map to a
    constant, so add the missing ones first in this preparatory commit.
    
    The LOCK constant is unconditional so the C API is the same in every
    build.
    
    No behaviour change. Part of a series of commits to make kernel logging
    struct-based instead of string-based.
    3a72f3040c
  27. kernel: expose btck_LogEntry
    Introduce the btck_LogEntry struct the logging callback will use in
    a future commit (instead of the current string argument). The struct
    is not yet used.
    
    The level and category definitions move up so the callback typedef can
    reference btck_LogEntry; review with --color-moved.
    
    No behaviour change. Part of a series of commits to make kernel logging
    struct-based instead of string-based.
    94cf10c4bf
  28. kernel: add log level and category name helpers to wrapper
    Convenience functions for consumers that want to format levels and
    categories.
    
    No behaviour change. Part of a series of commits to make kernel logging
    struct-based instead of string-based.
    4469d1f57e
  29. kernel: remove API to format and disable logging
    Both functions lose their purpose once the kernel logger is no longer
    coupled to the node logger: formatting becomes the consumer's job, and
    nothing is buffered before a connection exists, so there is nothing to
    disable. Remove them early to simplify the next commits.
    
    Behaviour change until the backend switch later in this series:
    - strings are delivered in BCLog::Logger's default format
    - the 1MB pre-connection buffer cannot be turned off.
    
    Part of a series of commits to make kernel logging struct-based instead
    of string-based.
    b4edc98745
  30. kernel: add KernelLogger
    Add the kernel's own logging backend, unused until the next commit
    switches the C API over to it.
    
    Callbacks run while the logger's mutex is held: invocations are
    serialized, and unregistering waits for an in-flight callback.
    
    Callbacks are typed with a local LogCallback alias carrying the
    struct-based signature, because btck_LogCallback still delivers a
    string at this point. The next commit changes the typedef and replaces
    the alias.
    
    No behaviour change. Part of a series of commits to make kernel logging
    struct-based instead of string-based.
    95e63e954a
  31. kernel: make logging struct-based and simplify configuration
    Update btck_LogCallback to use struct-based logging and drop logging.cpp
    from the kernel build.
    
    Since consumers can now easily filter by category, simplify the logging
    configuration to just btck_logging_set_min_level.
    
    Behaviour changes for consumers:
    - Filtering is now purely levels-based, and consumers can choose any
      level they like. Debug and Trace logging applies to all categories,
      further filtering should be done client-side.
      Note: evaluation remains unchanged: Debug and Trace statements are
      only evaluated when they are requested, Info and above is always
      evaluated.
    - Entries logged before the first connection are dropped, not buffered.
    - Trailing newlines are stripped, callback exceptions are swallowed,
      and connection create and destroy no longer take cs_main.
    - Log messages are no longer escaped, this can be done client-side.
    
    KernelLogger moves into the anonymous namespace now that every member
    is used. It stayed outside in the previous commit to avoid unused
    member warnings.
    
    Node logging is unchanged. Final commit of the series to make kernel
    logging struct-based instead of string-based.
    cfd2312f0f
  32. kernel: Set log level per connection and attach connections to contexts
    Change btck_logging_set_min_level to take a btck_LoggingConnection and apply
    only to that connection, and add btck_context_options_set_logger to attach a
    connection to the contexts created with the options. A new connection receives
    nothing until a context using it exists. The connection must outlive the
    context options and contexts using it, and destroying it while a context still
    uses it aborts.
    
    Previously every connection received all kernel log output at one
    process-wide level, and connections weren't passed to any other kernel
    function. Code using the API had no reason to keep a connection where it
    creates contexts, so separating log output per context later would have
    required restructuring callers rather than adding a parameter.
    
    Routing each context's entries only to its own connection is left for #30342.
    Until then a connection used by a context may also receive entries from other
    kernel operations, which the header documents. Because callers already attach
    connections where log output comes from, that follow-up narrows what
    connections receive without changing the API.
    
    This change was written with Claude Opus 5.5 (1M context).
    8e379446ec
  33. ryanofsky force-pushed on Sep 25, 2026
  34. ryanofsky renamed this:
    kernel: Improve logging API
    kernel: Attach logging connections to contexts, per-connection log levels
    on Sep 25, 2026
  35. DrahtBot removed the label Needs rebase on Sep 25, 2026
  36. stickies-v commented at 10:32 AM on October 6, 2026: contributor

    It's nice to see how small the diff for this branch has become after rebasing it onto #34374.

    It seems like the purpose of this PR is to carve out some (much smaller) logging API changes, to minimize downstream churn later when #30342 is merged. I fundamentally disagree with that premise, for a couple of reasons:

    • exposing contextualized logging when it doesn't actually exist is a contract regression until #30342 is merged
    • the kernel API is intentionally marked experimental, so we can break it whenever it makes sense, to not burden the development process any more than it already is. Users are aware that the interface will break. We should use this liberty while we have it

    I think if we want a contextualized logging API, the interface and functional changes should happen atomically in #30342.

    Approach NACK

  37. DrahtBot added the label Needs rebase on Oct 8, 2026
  38. DrahtBot commented at 4:33 PM on October 8, 2026: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-11 10:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me