2024 11 configurable logging system with foreign #3
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
Recently, `rustls` bumped their MSRV to 1.71. As we depend on them and don't want to continuously pin this security-critical dependency back, we're looking to bump our MSRV to a version >= 1.71, too. Here, we propose to bump the MSRV to rustc version 1.75, which was chosen because: a) it's > 1 year old b) it provides a buffer to 1.71, i.e., if some crate bumped to a version > 1.71, there is a chance we don't immediately have to react again c) it stabilized `async fn`s in traits (see https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html), which might become handy for related crates.
…o-1.75 Bump MSRV to rustc v1.75
…ollow-up Mention new MSRV in README
Previously, we decided to continue parsing any fields if we failed to parse a known (i.e., `lightning` or `lno`) parameter failed to parse. This however just hides the error and is a bit anti-idiomatic even though allowing to use *some* URI fields even in the face of incompatible formats for others. Here we therefore opt to fail parsing the URI if any field fails.
…-ldk Update BDK to 1.0.0 and LDK to 0.1.0
Previously, we would cast `FeeRate::to_sat_per_kwu` to `u32`, which however might result in `u32::max_value` being used if our fee estimation source delivers huge bogus data. Here, we make sure to use the fallback rate if we would do so otherwise.
So far, we did not verify gossip data received via the peer-to-peer network. As a small fix in the latest LDK release allows us properly set the gossip verifier, we do so, and start verifying gossip when the bitcoind RPC gossip source is set.
…al-fee-rate-conversion-overflow Default to fallbacks for huge bogus values in fee estimation conversion
…ying-gossip Start verifying gossip data via bitcoind RPC
* Add flexible log writer interface for multiple destinations * Implement filesystem writing capability via FilesystemLogger * Prefix LDK-based objects with 'Ldk' for consistency * Add configuration options for log file path and log level
* Add support for user-provided custom logger to write logs to, allowing users to provide any logger that implements LogWriter * Add test to cover this use case, implementing Log- Writer for the mock, in-memory MockLogger.
This commit includes several improvements aimed at simplifying code, improving readability, and optimizing performance: - Inline enum struct fields to reduce indirection and clutter. - Rename structs to improve clarity and better reflect their purpose. - Enable specific feature flags in dependencies for a more streamlined build. - Eliminate unnecessary data allocations to optimize memory usage. - Correct minor grammatical error in documentation.
- allocates Strings only when necessary with `uniffi` feature, otherwise, keeps LogRecord fileds as lifetime references.
This commit adds a CustomLogWriter class to the kotlin library test, configuring the writer via the exposed node builder and tests the ability to log to the custom writer destination.
…ehavior - Removed trait name in description. - Expanded the description of the `LogWriter` trait to clarify its behavior when the `uniffi` feature is enabled or disabled. - Improved explanation of the trait’s responsibilities, including handling log messages and forwarding to outputs.
- Add a custom Debug implementation for LogWriterConfig to work around NodeBuilder's Debug constraints. - Remove the Debug trait from LogWriter, as it is no longer needed due to the custom Debug impl on LogWriterConfig. - Remove the Debug implementation from Writer for consistency.
- Revert MockLogger configuration and assertions - Remove clutter from setting up LogWriter across all tests
- Remove remnants of TestLogWriter and MockLogger that will be reintroduced in a later PR to add a more structured TestConfig object.
Removes the default_log_* functions that were hitherto public functions used to share private constants. These constants (DEFAULT_LOG_LEVEL and the renamed DEFAULT_LOG_FILE_PATH) have been made public instead.
With this, we avoid cloning the log writer config before calls to setup_logger. The default behaiour is also handled at a single location.
Additionally, clone Arc to custom_log_writer idiomatically.
Replacing the FilesystemLoggerConfig previously used by the NodeBuilder to configure the logging to the filesystem.
Removes them from LogWriterConfig and new_custom_writer
With this attribute, the LogWriter trait can be implemented on the foreign side, e.g. in Kotlin, the CustomLogWriter can implement LogWriter directly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 this PR does
This PR tests the introduction of the
WithForeignattribute to theLogWritertrait so it can be implemented on the foreign side.