chore: lower sqlx query logging to debug and add no_file_log feature

[sheb-gregor]

Summary

Two logging tweaks:

• sqlx query logs → debug. sea-orm defaults sqlx_logging_level to Info, emitting a record per
  statement. Now set to Debug, so queries only appear when the consuming app's subscriber is at debug.
  Adds log as a direct dependency (already in the tree).
• New no_file_log cargo feature. When enabled, no log file is created and records go to stdout
  instead, using the same format (timestamp + file location). setup_logger's signature is unchanged,
  so all callers (setup_new_wallet, restore_backup, restore_from_vss) are untouched.

Notes

• no_file_log is a negative feature, so it breaks cargo's additive-feature convention: if any crate in a
  shared graph enables it, file logging is off for everyone.
• --all-features now includes no_file_log, so the documented test command runs with logs on stdout.
  The one test asserting on log-file creation (test/new.rs, permission-denied case) is gated accordingly;
  backup dir comparisons already excluded log.

[claude]

Review: chore: lower sqlx query logging to debug and add no_file_log feature

Small, well-scoped change, and the PR description is unusually honest about the trade-offs (thanks for calling out the negative-feature issue up front). The sqlx part looks right to me. Most of my comments are about the no_file_log half.

Reviewed statically against dev; I did not build or run the suite in this environment.

1. --all-features now compiles out the default logging path, so CI stops testing it

.github/workflows/test.yml:25 runs cargo test --all-features and README.md:49 documents the same command. With no_file_log in [features], both now build the stdout variant of setup_logger, which means:

• the file-based setup_logger (create/append, permission-denied → Error::IO mapping) is no longer exercised by the main test job;
• the single assertion covering it in src/wallet/test/new.rs is #[cfg(not(feature = "no_file_log"))], and new::fail isn't in the reldebug job's explicit test list (test.yml:38-47), so that assertion runs in no CI job after this PR;
• lint.yml:22 (clippy --all-features --all-targets -D warnings) also stops linting the file variant, though the --no-default-features --features electrum/esplora clippy runs still cover it.

Net effect: the configuration that actually ships (default features) loses its only logging test, while the new configuration gains none. That's the part I'd want addressed before merge.

Two ways out, in order of preference:

a) Make the feature additive — file_log, on by default:

default = ["electrum", ..., "file_log"]
file_log = []

--all-features then keeps file logging (default behaviour stays tested, the existing assertion keeps running), and opting out is --no-default-features --features electrum,.... This also addresses the viral-feature concern from the PR description: a crate elsewhere in the graph can no longer silently turn off your file logging, since dropping a default feature requires the top-level crate to opt out.

b) Keep no_file_log but drop it from the all-features job (enumerate features explicitly, or use cargo hack) and add a small dedicated job that builds/tests with it enabled.

Either way, worth pairing with an actual test for the new path (see #4).

2. Library diagnostics on stdout are risky — prefer stderr

src/utils.rs: PlainDecorator::new(std::io::stdout()).

rgb-lib is a library, and it's consumed via bindings/c-ffi and bindings/uniffi inside host processes that don't control this choice. Anything whose stdout is a protocol — a CLI emitting JSON, a process piping data downstream, an FFI host — gets its output corrupted by log records once this feature is on. Convention (and what RUST_LOG-style tooling expects) is diagnostics on stderr, and container log collectors capture stderr just as readily as stdout.

Suggest std::io::stderr(), or making the sink an explicit part of the feature contract if stdout is genuinely what the consuming app needs.

3. Multiple wallets → multiple async drains → interleaved records on one fd

With file logging, each wallet gets its own file, so one slog_async worker owns each writer. With no_file_log, every Wallet::new (setup_new_wallet → setup_logger) plus every restore_backup / restore_from_vss call builds a separate slog_async::Async drain over the same stdout fd.

FullFormat emits a record via several write! calls before the terminating newline. Stdout locks per write!, not per record, so two drain threads can interleave mid-record and produce garbled lines. A multi-wallet process — which this library explicitly supports, and which the test suite itself creates — is where this shows up.

Suggest a single process-wide logger for the stdout/stderr case, e.g.:

#[cfg(feature = "no_file_log")]
static STDERR_LOGGER: LazyLock<(Logger, Arc<AsyncGuard>)> = LazyLock::new(|| { ... });

The AsyncGuard return type makes this slightly awkward (callers store it in Wallet::_logger_guard), so it needs an Arc or a cloneable wrapper — but one drain per fd seems worth the small refactor. At minimum, a comment acknowledging the interleaving would help if you'd rather not restructure now.

4. No test covers the new code path

Nothing asserts the no_file_log behaviour. A symmetric counterpart to the gated-out assertion is cheap and would give the feature real coverage:

#[cfg(feature = "no_file_log")]
{
    // no log file is created, so a non-writable dir is not an error
    assert!(setup_logger(non_writable_path, Some("log")).is_ok());
}

plus, ideally, one test asserting wallet_dir.join("log") does not exist after Wallet::new with the feature on. That's the invariant users of the feature actually care about, and it's what would catch a future regression where something re-adds a file sink.

5. Duplicated drain construction

setup_logger and setup_stdout_logger repeat the FullFormat / use_custom_timestamp / use_file_location / slog_async chain. Since the two variants are never compiled together, a later format change can silently drift. One private builder shared by both:

fn build_logger<W: io::Write + Send + 'static>(w: W) -> (Logger, AsyncGuard) {
    let drain = FullFormat::new(PlainDecorator::new(w))
        .use_custom_timestamp(log_timestamp)
        .use_file_location();
    let (drain, guard) = slog_async::Async::new(drain.build().fuse()).build_with_guard();
    (Logger::root(drain.fuse(), o!()), guard)
}

Also, setup_stdout_logger is pub(crate) but only called from setup_logger in the same module — it can be private, or inlined away entirely by the above.

6. Minor notes

• The log dependency is clean. Cargo.lock resolves a single log 0.4.29, so no version skew with sea-orm's LevelFilter. (default-features = false is a no-op for log, which has none — harmless, and consistent with the surrounding entries.)
• sqlx slow-statement logging is unaffected. sqlx logs statements slower than its threshold at warn independently of log_statements; if that noise turns up, sqlx_slow_statements_logging_settings is the knob. Not needed for this PR.
• The feature isn't documented outside Cargo.toml. Neither README.md nor the lib.rs crate docs enumerate features today, so this is discretionary — but since the feature changes observable behaviour for consumers (no log file, output on stdout/stderr), a line in the crate docs would help. If you go with the additive file_log rename, documenting it matters more, since it becomes a default feature.
• The behaviour change for consumers of the Info-level query logs is real but well justified and clearly documented in the PR body; no objection.

Verdict

The sqlx change looks good to go. For no_file_log, #1 (CI coverage of the default path) is the one I'd call blocking, and #2/#3 are worth settling before this reaches consumers embedding the library. #4–#6 are cleanups.