Apply `env_logger` module filters on Dart & UniFFI binding loggers #874

erdemyerebasmaz · 2024-03-19T22:05:50Z

This PR addresses the issue where env_logger's module filters were not applying to logs listened through Dart & UniFFI binding loggers.

Changelist:

Filter Dart & UniFFI binding logs through env_logger module filters
Move loggers under logger.rs
Make filter levels customizable for each logger
Log node-logs according to their level
Apply changes to bindings & generate them
- Create LevelFilter struct that's usable from bindings.
- Add optional filter_level param to binding logger init & setters
- Map models:LevelFilter to log::LevelFilter.
  - Set LevelFilter::TRACE as default filter level

TODO:

Fix RN Bindings:
- Add optional filter level param to set log stream breez-sdk-rn-generator#23
Apply changes to:
- notification extension packages
- flutter plugin
- example projects
- test cases

erdemyerebasmaz · 2024-03-23T16:16:39Z

Depends on:

Add optional filter level param to set log stream breez-sdk-rn-generator#23 (tests pass using locally built bindings)
-Go & C# bindings to be publish for tests to pass

roeierez · 2024-03-25T08:46:08Z

libs/sdk-core/src/logger.rs

+    });
+}
+
+pub struct UniFFILogger {


Why taking the specific loggers out of their specific crates?
Doesn't UniFFILogger belong to the sdk-bindings crate?

Not necessarily, the loggers are not exposed through bindings.

I had brought them together to see the differences to find the root cause of the issue, and I prefer having them together, but we can bring them back to uniffi_bindings.rs & bindings.rs with little to no change.

If we expand upon logger settings as mentioned on #892, it would be hard to miss applying new changes on bindings when they are grouped together like this.

I agree putting these in the bindings crate seems logical

Done.

Move Dart & UniFFI loggers back to their respective places

JssDWt

Concept ACK! A few nits. (And ofcourse this needs a rebase, which will be painful)

JssDWt · 2024-10-31T13:51:33Z

libs/sdk-core/src/logger.rs

+debug,
+breez_sdk_core::backup=info,
+breez_sdk_core::breez_services=info,
+breez_sdk_core::input_parser=warn,
+breez_sdk_core::persist::reverseswap=info,
+breez_sdk_core::reverseswap=info,
+gl_client::node=info,
+gl_client::node::service=info,
+gl_client::tls=info,
+gl_client::scheduler=info,
+gl_client::signer=info,
+gl_client=debug,
+h2=warn,
+h2::client=info,
+h2::codec::framed_read=warn,
+h2::codec::framed_write=warn,
+h2::proto::connection=info,
+h2::proto::settings=info,
+hyper=warn,
+hyper::client::connect::dns=info,
+hyper::client::connect::https=info,
+lightning_signer=warn,
+lightning_signer::node=warn,
+reqwest=warn,
+reqwest::connect=warn,
+rustls=warn,
+rustls::anchors=warn,
+rustls::conn=warn,
+rustls::client::common=warn,
+rustls::client::hs=warn,
+rustls::client::tls13=warn,
+rustyline=warn,
+rusqlite_migration=warn,
+tower::buffer::worker=warn,
+vls_protocol_signer=warn,
+vls_protocol_signer::handler::HandlerBuilder::do_build=warn


NOTE: I've changed the log filters in #1098

Adding the missing sdk_common=debug, should be enough imo.

We can make the necessary individual changes after making comparisons if there are any.

JssDWt · 2024-10-31T13:51:46Z

libs/sdk-core/src/logger.rs

+            writeln!(
+                buf,
+                "[{} {} {}:{}] {}",
+                Local::now().format("%Y-%m-%d %H:%M:%S%.3f"),


Let's use UTC if possible with ISO8601 format

Good point! Done.

Switch timestamp format from local to UTC in ISO 8601 format

JssDWt · 2024-10-31T13:52:52Z

libs/sdk-core/src/logger.rs

+    });
+}
+
+pub struct UniFFILogger {


I agree putting these in the bindings crate seems logical

Dart, UniFFI & GlobalSDK logger's are moved under logger.rs .

- Create LevelFilter struct that's usable from bindings. - Add optional filter_level param to binding logger init & setters - Map models:LevelFilter to log::LevelFilter. - Set LevelFilter::TRACE as default filter level

Log `UNUSUAL` level node logs at `Error` level

Errors made around import resolution(sdk-common changes etc.) & removal of a crate introduced on this PR

JssDWt

Added a few comments

JssDWt · 2024-11-07T07:59:22Z

libs/sdk-core/src/binding.rs

+            );
+        }
+        *guard = Some(stream_sink);
+        drop(guard)


This is done automatically when the guard goes out of scope.

JssDWt · 2024-11-07T08:00:56Z

libs/sdk-core/src/binding.rs

+use log::{
+    max_level, set_boxed_logger, set_max_level, warn, Log, Metadata, Record, STATIC_MAX_LEVEL,
+};
+use parking_lot::RwLock;


What's the reason to use parking_lot here? Can we use std::sync::RwLock or tokio::sync::RwLock?

JssDWt · 2024-11-07T08:03:30Z

libs/sdk-core/src/breez_services.rs

+                            let prefix_len = l.line.find(':').map_or(0, |len| len + 2);
+                            let log_message = l.line.split_at(prefix_len).1.trim_start();
+                            match l.line.to_ascii_lowercase().as_str() {
+                                s if s.starts_with("unusual") => error!("node-logs: {}", log_message),


I think unusual can be warn

And broken needs to be added as error

JssDWt · 2024-11-07T08:08:35Z

libs/sdk-core/src/logger.rs

+breez_sdk_core::backup=info,
+breez_sdk_core::breez_services=info,
+breez_sdk_core::input_parser=warn,
+breez_sdk_core::persist::reverseswap=info,
+breez_sdk_core::reverseswap=info,
+sdk_common=debug,
+gl_client::node=info,
+gl_client::node::service=info,
+gl_client::tls=info,
+gl_client::scheduler=info,
+gl_client::signer=info,
+gl_client=debug,


If I understand correctly, debug level logs would be filtered out regardless of the client's loglevel. I would put all these at debug, or even trace, because they would eventually be filtered out by the client log level.

JssDWt · 2024-11-07T08:09:05Z

libs/sdk-core/src/logger.rs

+/* env_logger */
+
+const ENV_LOGGER_FILTER: &str = r#"
+debug,


This should either depend on client log level or be info in my opinion

Perhaps we should create one of these filter statements per log level

dangeross

These RN codegen changes in breez/breez-sdk-rn-generator#23 should be applied in libs/sdk-bindings/bindings-react-native

erdemyerebasmaz force-pushed the logger branch 2 times, most recently from b544d81 to 265f0d6 Compare March 21, 2024 18:43

erdemyerebasmaz requested review from ok300, roeierez and dangeross March 21, 2024 23:10

erdemyerebasmaz marked this pull request as ready for review March 21, 2024 23:10

erdemyerebasmaz force-pushed the logger branch from f5161f7 to 8cdbad4 Compare March 22, 2024 11:44

erdemyerebasmaz mentioned this pull request Mar 22, 2024

Add optional filter level param to set log stream breez/breez-sdk-rn-generator#23

Open

erdemyerebasmaz force-pushed the logger branch from 8cdbad4 to 290d3a2 Compare March 22, 2024 11:49

erdemyerebasmaz changed the title ~~Consolidate loggers under logger.rs~~ Apply env_logger module filters on Dart & UniFFI binding loggers Mar 22, 2024

erdemyerebasmaz force-pushed the logger branch 4 times, most recently from 18a4b02 to 39d9220 Compare March 23, 2024 13:46

erdemyerebasmaz mentioned this pull request Mar 23, 2024

Filter out node logs below Info level breez/c-breez#848

Open

erdemyerebasmaz force-pushed the logger branch 2 times, most recently from 086d763 to 2bbebbb Compare March 23, 2024 16:14

erdemyerebasmaz force-pushed the logger branch from 2bbebbb to 7896004 Compare March 23, 2024 16:33

roeierez requested changes Mar 25, 2024

View reviewed changes

erdemyerebasmaz requested a review from roeierez March 27, 2024 23:03

erdemyerebasmaz force-pushed the logger branch 2 times, most recently from 94788be to 66df72b Compare March 28, 2024 11:22

erdemyerebasmaz linked an issue Apr 15, 2024 that may be closed by this pull request

Print logs from Dart & UniFFI binding loggers on LogCat & OSLog #892

Open

erdemyerebasmaz mentioned this pull request May 29, 2024

Add global SDK logger breez/breez-sdk-liquid#242

Merged

erdemyerebasmaz mentioned this pull request Oct 8, 2024

improve log levels #1098

Merged

JssDWt reviewed Oct 31, 2024

View reviewed changes

erdemyerebasmaz added 3 commits November 5, 2024 17:32

Fix pattern matching for InvalidMinSendable

8a5194b

Move loggers under logger.rs

b5e8ba4

Dart, UniFFI & GlobalSDK logger's are moved under logger.rs .

Make filter levels customizable for each logger

badc177

erdemyerebasmaz added 3 commits November 5, 2024 17:37

Filter binding logs through env_logger filters

a6d7564

Apply changes to bindings & generate them

dd4bf74

- Create LevelFilter struct that's usable from bindings. - Add optional filter_level param to binding logger init & setters - Map models:LevelFilter to log::LevelFilter. - Set LevelFilter::TRACE as default filter level

Log node-logs according to their level

ef9c7db

Log `UNUSUAL` level node logs at `Error` level

erdemyerebasmaz force-pushed the logger branch from 66df72b to 7c23fbf Compare November 5, 2024 14:42

erdemyerebasmaz added 7 commits November 5, 2024 19:00

Generate RN Bindings

53dc29f

Update test cases on bindings

f4d2401

Apply filter level changes to Flutter plugin

7811d43

Refactor "Log node-logs according to their level"

1519d46

Resolve regressions introduced during rebase

e1d4683

Errors made around import resolution(sdk-common changes etc.) & removal of a crate introduced on this PR

Switch timestamp format from local to UTC in ISO 8601 format

9f22d19

Move Dart & UniFFI loggers back to their respective places

ef70a96

erdemyerebasmaz force-pushed the logger branch from 8f661bb to ef70a96 Compare November 5, 2024 16:00

Regenerate RN bindings

6c6118d

erdemyerebasmaz force-pushed the logger branch 2 times, most recently from 8842abf to 32aef81 Compare November 6, 2024 07:28

erdemyerebasmaz added 2 commits November 6, 2024 10:42

Set default filter level on uniFFI bindings tests

c88d257

Format test_breez_sdk.kts using ktlint

7d3a462

erdemyerebasmaz force-pushed the logger branch from 32aef81 to 7d3a462 Compare November 6, 2024 07:43

JssDWt reviewed Nov 7, 2024

View reviewed changes

dangeross reviewed Nov 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply `env_logger` module filters on Dart & UniFFI binding loggers #874

Apply `env_logger` module filters on Dart & UniFFI binding loggers #874

erdemyerebasmaz commented Mar 19, 2024 •

edited

Loading

erdemyerebasmaz commented Mar 23, 2024

roeierez Mar 25, 2024

erdemyerebasmaz Mar 27, 2024 •

edited

Loading

JssDWt Oct 31, 2024

erdemyerebasmaz Nov 5, 2024

JssDWt left a comment

JssDWt Oct 31, 2024

erdemyerebasmaz Nov 5, 2024

JssDWt Oct 31, 2024

erdemyerebasmaz Nov 5, 2024

JssDWt Oct 31, 2024

JssDWt left a comment

JssDWt Nov 7, 2024

JssDWt Nov 7, 2024

JssDWt Nov 7, 2024

JssDWt Nov 7, 2024

JssDWt Nov 7, 2024

JssDWt Nov 7, 2024

dangeross left a comment

Apply env_logger module filters on Dart & UniFFI binding loggers #874

Are you sure you want to change the base?

Apply env_logger module filters on Dart & UniFFI binding loggers #874

Conversation

erdemyerebasmaz commented Mar 19, 2024 • edited Loading

Changelist:

TODO:

erdemyerebasmaz commented Mar 23, 2024

Choose a reason for hiding this comment

erdemyerebasmaz Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JssDWt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JssDWt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dangeross left a comment

Choose a reason for hiding this comment

Apply `env_logger` module filters on Dart & UniFFI binding loggers #874

Apply `env_logger` module filters on Dart & UniFFI binding loggers #874

erdemyerebasmaz commented Mar 19, 2024 •

edited

Loading

erdemyerebasmaz Mar 27, 2024 •

edited

Loading