Integrate rust logging by adespawn · Pull Request #423 · scylladb/nodejs-rs-driver

adespawn · 2026-03-27T13:29:27Z

This PR was created with Claude Opus and reviewed by me.

This PR introduces new API for logging. As discussed in person, we cannot keep the original interface, which we change with this PR.

Add configurable log forwarding from Rust driver to JS

Introduces a logging system that forwards internal driver log events to the
JS Client as 'log' EventEmitter events, with configurable severity
filtering.

Highlights

logLevel client option: Controls the minimum severity of forwarded
events. Set to one of trace, debug, info, warning, error, or
off (default). Filtering happens on the native side before crossing the
FFI boundary.
types.logLevels enum: Provides type-safe access to log level values
in both JS and TypeScript.
Per-client registration: Each Client registers its own callback on
connect() and unregisters on shutdown(). Multiple clients can coexist
with independent log levels.
Four separate event arguments: The 'log' event delivers
(level, target, message, furtherInfo) as separate arguments, matching
the cassandra-driver event signature.
Native implementation: A global tracing subscriber in Rust maintains
a registry of per-client NAPI callbacks. setupLogging() returns an
Option<u32> id (or null for off/invalid), used by removeLogging()
to unregister.

Documentation

New standalone Logging page covering levels, event
arguments, event sources, multi-client behavior, and usage examples.
New "Logging" section in the Migration guide
documenting differences from cassandra-driver: logging off by default,
verbose replaced by trace+debug, target replaces className,
cross-client event visibility.

Tests

Unit tests for setupLogging/removeLogging: level filtering, all five
levels delivered, furtherInfo with extra fields, multiple callbacks,
removal stops delivery.
Integration tests for log events during connect(), query execution, and
DDL operations.
API tests asserting logLevels enum values in JS and TypeScript.

Fixes: #16

Copilot

Pull request overview

This PR integrates Rust tracing-based logging into the Node.js driver by forwarding Rust log events across the N-API boundary and exposing a types.logLevels string enum to configure filtering from JavaScript/TypeScript.

Changes:

Added a Rust global tracing subscriber layer that forwards events to per-client registered JS callbacks with level filtering.
Introduced logLevel client option and types.logLevels enum (JS + TS typings), plus tests covering API surface and behavior.
Added end-user documentation and migration guide updates for the new logging system.

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`src/logging.rs`	New Rust forwarding layer + per-client callback registry + level filtering.
`lib/client.js`	Registers/unregisters Rust log callback based on `options.logLevel`.
`lib/client-options.js`	Adds `logLevel` option default + validation and JSDoc.
`lib/types/index.js`	Adds `types.logLevels` enum values.
`lib/types/index.d.ts`	Adds `logLevels` TypeScript enum.
`main.d.ts`	Adds `logLevel?: types.logLevels` to `ClientOptions`.
`src/tests/logging_tests.rs` / `src/tests/mod.rs`	Adds Rust test helpers to emit `tracing` events for JS tests.
`test/unit/logging-tests.js`	Unit tests for Rust->JS forwarding, multi-callback, and filtering.
`test/integration/supported/client-logging-tests.js`	Integration coverage for real Rust driver log events emitted via `Client`.
`test/unit-not-supported/api-tests.js`	Ensures `types.logLevels` exists in the public API surface.
`test/typescript/*`	Type-level coverage for `types.logLevels` and `Client({ logLevel })`.
`docs/src/logging.md` / `docs/src/migration_guide.md` / `docs/src/SUMMARY.md`	Documentation for logging and migration notes.
`Cargo.toml`	Adds `tracing` and `tracing-subscriber` dependencies.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 18 out of 20 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T10:43:26Z

+/// Initialized (and the global subscriber installed) on the first call to
+/// `setup_logging`.
+static CALLBACKS: OnceLock<RwLock<Vec<RegisteredCallback>>> = OnceLock::new();
+
+/// Return the callback registry, lazily installing the global tracing
+/// subscriber the very first time this is called.
+fn get_or_init_callbacks() -> &'static RwLock<Vec<RegisteredCallback>> {
+    CALLBACKS.get_or_init(|| {
+        let subscriber = Registry::default().with(JsForwardingLayer);
+        tracing::subscriber::set_global_default(subscriber).unwrap_or_else(|e| {
+            // This will fail only of there is already a global subscriber, which we should prevent from happening.
+            panic!("This is likely due to a bug in the driver: failed to initialized logger. {e}");
+        });


set_global_default() failure currently triggers a panic!(). Given the crate is configured with panic = "abort" (Cargo.toml), this would hard-abort the Node.js process if any other Rust code in the process has already installed a global tracing subscriber. This seems too risky for a logging feature; consider handling the error non-fatally (e.g., skip forwarding / return None / store a flag) and fix the message/typos ("only if", "initialize").

Suggested change

/// Initialized (and the global subscriber installed) on the first call to

/// `setup_logging`.

static CALLBACKS: OnceLock<RwLock<Vec<RegisteredCallback>>> = OnceLock::new();

/// Return the callback registry, lazily installing the global tracing

/// subscriber the very first time this is called.

fn get_or_init_callbacks() -> &'static RwLock<Vec<RegisteredCallback>> {

CALLBACKS.get_or_init(|| {

let subscriber = Registry::default().with(JsForwardingLayer);

tracing::subscriber::set_global_default(subscriber).unwrap_or_else(|e| {

// This will fail only of there is already a global subscriber, which we should prevent from happening.

panic!("This is likely due to a bug in the driver: failed to initialized logger. {e}");

});

/// Initialized on the first call to `setup_logging`. We also try to install

/// the global subscriber at that time, but continue without JS log forwarding

/// if another global subscriber has already been installed.

static CALLBACKS: OnceLock<RwLock<Vec<RegisteredCallback>>> = OnceLock::new();

/// Return the callback registry, lazily attempting to install the global

/// tracing subscriber the very first time this is called.

fn get_or_init_callbacks() -> &'static RwLock<Vec<RegisteredCallback>> {

CALLBACKS.get_or_init(|| {

let subscriber = Registry::default().with(JsForwardingLayer);

let _ = tracing::subscriber::set_global_default(subscriber);

We all love LLMs. One time they say A, the other time they say not A.

I would say, explicit error is better here

wprzytula · 2026-04-10T07:43:53Z

* New "Logging" section in the [Migration guide](docs/src/migration_guide.md)
  documenting differences from `cassandra-driver`: logging off by default,

I have doubts. You say that you change the default logging level from (?) to "off". Why?
Especially ERROR level, but also WARN level logs are often used as a way to signal user
important situation in the driver. For example, Rust Driver automatically logs warnings
that come from the DB on the WARN level. It is expected that driver's users see them by default.

adespawn · 2026-04-10T08:06:56Z

It is expected that driver's users see them by default.

You still need to add some logic to be able to see any logs (client.on("log"...).

Why?

Considering the above, I assumed that users when creating the logic will also set the desired log level.
But on the other hand, this logging should be mostly API compatible, so we could set it to WARN/ ERROR by default, so transitioning users have some default logging...

wprzytula · 2026-04-15T14:06:10Z

It is expected that driver's users see them by default.

You still need to add some logic to be able to see any logs (client.on("log"...).

Why?

Considering the above, I assumed that users when creating the logic will also set the desired log level. But on the other hand, this logging should be mostly API compatible, so we could set it to WARN/ ERROR by default, so transitioning users have some default logging...

Yep. Let's default to WARN.

wprzytula · 2026-04-15T14:45:53Z

+/// Generic parameters: `CalleeHandled = false`, `Weak = true` so that the
+/// callback does not prevent the Node.js event-loop from exiting.
+type LogCallback = ThreadsafeFunction<
+    /* T: */ FnArgs<(String, String, String, String)>,
+    /* Return: */ (),
+    /* CallJsBackArgs: */ FnArgs<(String, String, String, String)>,


❓ What is CallJsBackArgs and why it's the same as T? Why are they distinct?

wprzytula · 2026-04-15T14:46:39Z

+/// Monotonically increasing id counter for registered callbacks.
+static NEXT_ID: AtomicU32 = AtomicU32::new(0);


⛏️ Maybe let's use AtomicU64 to have a larger pool of IDs?

wprzytula · 2026-04-15T14:49:16Z

+/// Registry of all currently-active per-client callbacks.
+/// Initialized (and the global subscriber installed) on the first call to
+/// `setup_logging`.
+static CALLBACKS: OnceLock<RwLock<Vec<RegisteredCallback>>> = OnceLock::new();


🤔 I don't get why we need both OnceLock and RwLock. RwLock is enough to provide interior mutability, while it can be initialized to RwLock::new(Vec::new)).

What OnceLock does give is once-only semantics for the global tracing subscriber initialization. Perhaps we can do without that, though?

Ah, yes. You anyway initialize with RwLock::new(Vec::new()). In this case, you can replace OnceLock with Once, and use that Once for initializing the global subscriber only.

wprzytula · 2026-04-15T14:52:27Z

+/// Maps a Rust `tracing::Level` to the JS log-level strings.
+fn level_to_js(level: &Level) -> &'static str {
+    match *level {
+        Level::TRACE => "trace",
+        Level::DEBUG => "debug",
+        Level::INFO => "info",
+        Level::WARN => "warning",
+        Level::ERROR => "error",
+    }
+}


🔧 maybe: rust_level_to_js_level?

wprzytula · 2026-04-15T14:52:40Z

+/// Parses a JS-side log-level name.
+/// Returns `None` for `"off"` or unrecognized strings.
+fn parse_level(level: &str) -> Option<Level> {
+    match level {
+        "trace" => Some(Level::TRACE),
+        "debug" => Some(Level::DEBUG),
+        "info" => Some(Level::INFO),
+        "warning" => Some(Level::WARN),
+        "error" => Some(Level::ERROR),
+        _ => None,
+    }
+}


maybe: parse_js_level_to_rust_level?

wprzytula · 2026-04-15T14:53:19Z

+/// Returns `None` for `"off"` or unrecognized strings.
+fn parse_level(level: &str) -> Option<Level> {
+    match level {
+        "trace" => Some(Level::TRACE),
+        "debug" => Some(Level::DEBUG),
+        "info" => Some(Level::INFO),
+        "warning" => Some(Level::WARN),
+        "error" => Some(Level::ERROR),
+        _ => None,
+    }


🤔 Shouldn't this treat unknown strings as Result::Err, while off as Option::None?

wprzytula · 2026-04-15T14:53:40Z

+struct MessageVisitor {
+    message: String,
+    extras: String,
+}


🔧 Missing documentation.

wprzytula · 2026-04-15T14:56:13Z

+            self.extras
+                .push_str(&format!("{}={:?}", field.name(), value));


🔧 This needlessly allocates. Instead of format!, use String's implementation of std::fmt::Write, with write!(fmt, string, "{}={:?}", field.name(), value).

wprzytula · 2026-04-15T14:57:37Z

+        if field.name() == "message" {
+            self.message = format!("{:?}", value);


🔧 This needlessly allocates, while there already is a String created with String::new() in MessageVisitor::new(). Instead of format!, use String's implementation of std::fmt::Write, with write!(fmt, &mut self.message, "{:?}", value).

wprzytula · 2026-04-15T14:57:57Z

+    fn record_str(&mut self, field: &Field, value: &str) {
+        if field.name() == "message" {
+            self.message = value.to_owned();
+        } else {
+            if !self.extras.is_empty() {
+                self.extras.push_str(", ");
+            }
+            self.extras.push_str(&format!("{}={}", field.name(), value));
+        }
+    }


ditto about needless allocations

wprzytula

For now, reviewed up till the second commit, inclusively.

wprzytula · 2026-04-15T15:03:03Z

+        // At least one callback wants this event — extract the fields.
+        let level_str = level_to_js(event_level).to_owned();
+        let target = meta.target().to_owned();
+
+        let mut visitor = MessageVisitor::new();
+        event.record(&mut visitor);
+
+        for cb in callbacks.iter() {
+            if *event_level <= cb.min_level {
+                cb.callback.call(
+                    FnArgs {
+                        data: (
+                            level_str.clone(),
+                            target.clone(),


🔧 Small optimisation:

Suggested change

// At least one callback wants this event — extract the fields.

let level_str = level_to_js(event_level).to_owned();

let target = meta.target().to_owned();

let mut visitor = MessageVisitor::new();

event.record(&mut visitor);

for cb in callbacks.iter() {

if *event_level <= cb.min_level {

cb.callback.call(

FnArgs {

data: (

level_str.clone(),

target.clone(),

// At least one callback wants this event — extract the fields.

let level_str = level_to_js(event_level);

let target = meta.target();

let mut visitor = MessageVisitor::new();

event.record(&mut visitor);

for cb in callbacks.iter() {

if *event_level <= cb.min_level {

cb.callback.call(

FnArgs {

data: (

level_str.to_owned(),

target.to_owned(),

wprzytula · 2026-04-15T15:07:56Z

+/// Register a per-client logging callback.
+///
+/// Each call adds a **new** callback to the global registry, allowing
+/// multiple `Client` instances to receive Rust driver log events
+/// independently.
+///
+/// Returns a numeric `id` that must be passed to [`removeLogging`] when the
+/// client shuts down, so the callback can be unregistered.
+///
+/// Returns `None` (`null` on the JS side) when `min_level` is `"off"` or
+/// unrecognized — no callback is registered in that case.
+#[napi]
+pub fn setup_logging(callback: LogCallback, min_level: String) -> Option<u32> {
+    let level = parse_level(&min_level)?;
+
+    let id = NEXT_ID.fetch_add(1, Ordering::Relaxed);
+
+    let callbacks = get_or_init_callbacks();
+    callbacks.write().unwrap().push(RegisteredCallback {
+        id,
+        callback,
+        min_level: level,
+    });
+
+    Some(id)
+}


❓ Assume there are two Clients, each with its own Rust Driver's Session, and both clients use logging. Will both clients receive logs originating from their own Session or from any Session?

IIUC, they will get logs from any Session. Therefore, setting the logging callbacks per Client makes no sense for me. There should only be a global callback optionally set. Otherwise, what's the use of separate per-Client logging settings if all Clients see all logs anyway?

adespawn self-assigned this Mar 27, 2026

adespawn added the area/query-execution label Mar 27, 2026

adespawn force-pushed the logging-tracing branch 4 times, most recently from ab97f6e to 18b85d2 Compare April 3, 2026 09:58

adespawn changed the title ~~[LLM draft] Integrate logging~~ Integrate rust logging Apr 3, 2026

adespawn marked this pull request as ready for review April 3, 2026 09:59

adespawn requested a review from Copilot April 3, 2026 10:01

Copilot started reviewing on behalf of adespawn April 3, 2026 10:01 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

adespawn added 5 commits April 3, 2026 12:16

Add log event forwarding infrastructure

8b24ae5

Add logLevel client option for configurable log filtering

d09baf6

Add logging unit and integration tests

0f981da

Add logLevels API tests for JS and TypeScript type definitions

65616e6

Add documentation about logging

1589d55

adespawn force-pushed the logging-tracing branch from 18b85d2 to 1589d55 Compare April 3, 2026 10:32

adespawn added this to the 0.5.0 milestone Apr 3, 2026

adespawn requested a review from Copilot April 3, 2026 10:37

Copilot started reviewing on behalf of adespawn April 3, 2026 10:38 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

adespawn requested a review from wprzytula April 3, 2026 10:55

wprzytula reviewed Apr 15, 2026

View reviewed changes

wprzytula requested changes Apr 15, 2026

View reviewed changes

		/// Monotonically increasing id counter for registered callbacks.
		static NEXT_ID: AtomicU32 = AtomicU32::new(0);

		self.extras
		.push_str(&format!("{}={:?}", field.name(), value));

		if field.name() == "message" {
		self.message = format!("{:?}", value);

Conversation

adespawn commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add configurable log forwarding from Rust driver to JS

Highlights

Documentation

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wprzytula commented Apr 10, 2026

Uh oh!

adespawn commented Apr 10, 2026

Uh oh!

wprzytula commented Apr 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wprzytula left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adespawn commented Mar 27, 2026 •

edited

Loading