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.

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: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?

Off is filtered at the JS code.

https://github.com/scylladb/nodejs-rs-driver/blob/7eb8b18f9f75800449889af441dd213c2d4f2448/lib/client.js#L235

Invalid strings are also filtered
https://github.com/scylladb/nodejs-rs-driver/blob/7eb8b18f9f75800449889af441dd213c2d4f2448/lib/client-options.js#L737-L744
So this is a quiet ignoring of path that should not be taken

wprzytula

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

wprzytula · 2026-04-16T13:46:14Z

In-person review:

useless last integration test,
missing unit test of the visitor,
callback per Client is OK,
callbacks should be removed in a finalizer, not in shutdown,
take a look at CPP-RS's impl of the logger bridge.

adespawn · 2026-04-23T12:17:25Z

Rebase (no changes)

adespawn · 2026-04-27T12:19:37Z

Rebase (again)

adespawn · 2026-04-29T08:56:11Z

callbacks should be removed in a finalizer, not in shutdown,

Looking a bit more at it, I'm not sure it's a good idea: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry#notes_on_cleanup_callbacks

adespawn · 2026-04-29T09:15:40Z

Partial changes

useless last integration test

Done

adespawn · 2026-04-30T14:30:55Z

callbacks should be removed in a finalizer, not in shutdown,

Done, with a new test

adespawn · 2026-05-11T09:40:06Z

The only remaining thing now should be logging format (which we discussed during in person review).
Noteworthy changes:

Change to default behaviour
Internal notes on loggin

adespawn · 2026-05-19T14:38:45Z

Since I'm not exactly sure about the "Rework logging" commit, I didn't merge it to a proper place yet - will do it once those changes are be accepted

wprzytula

The commit structure and descriptions are bad. Please order the commits more neatly, describe them better, and make sure commits do not introduce bad code that you fix in a later commit.

wprzytula · 2026-05-21T08:00:06Z

            useBigIntAsLong: true,
            useBigIntAsVarint: true,
        },
+        logLevel: undefined,


🤔 This changes nothing, right? The key set to undefined is equivalent to no such key set, right?

wprzytula · 2026-05-21T08:06:20Z

+/**
+ * Validates the logLevel option.
+ * @param {string} logLevel
+ * @private
+ */
+function validateLogLevel(logLevel) {
+    if (
+        logLevel !== undefined &&
+        (typeof logLevel !== "string" || !validLogLevels.includes(logLevel))
+    ) {
+        throw new TypeError(
+            `logLevel must be one of ${validLogLevels.map((l) => `'${l}'`).join(", ")}`,
+        );
+    }
+}


💭 This seems buggy.

What if a non-string junk object is passed?

What if a value of the log level enum is passed?

wprzytula · 2026-05-23T19:18:55Z

+// Collects all fields and values in a single log event into a single String, similarly to C++-rs driver
 impl Visit for MessageVisitor {
    fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) {
-        if field.name() == "message" {
-            let _ = write!(&mut self.message, "{:?}", value);
+        if self.log_message.is_empty() {
+            write!(self.log_message, "{field}: {value:?}").unwrap();
        } else {
-            if !self.extras.is_empty() {
-                self.extras.push_str(", ");
-            }
-
-            let _ = write!(&mut self.extras, "{}={:?}", field.name(), value);
-        }
-    }
-
-    fn record_str(&mut self, field: &Field, value: &str) {


🔧 Explain these changes in the commit message.

This commit introduces rust logging cleanup on client finalization. Since we cannot deterministically predict when the client will be finalized, this is done as a back-up to manual logging cleanup on client shutdown. This new cleanup will trigger only when the client didn't shut down yet.

Rust unit tests: - MessageVisitor field formatting - rust_level_to_js and parse_js_level_to_rust mapping correctness JS unit tests: - FFI forwarding delivers events at all 5 tracing levels - Message content survives FFI boundary intact - Target field contains Rust module path - Multiple registered callbacks each receive events (shared subscriber) - removeLogging stops delivery without affecting other callbacks - Level filtering only passes events at or above registered level - Visitor field formatting end-to-end (comma separation, debug quotes, int/bool repr) - Level-to-message pairing is correct across FFI - Default logLevel registers Rust callback and receives events - logLevel 'off' suppresses all events Integration tests (against real cluster): - Real Rust driver produces log events during connection - JS and Rust log events coexist during connection - cleanup both for shutdown and gc cleanup

At some point the code was changed, in a way that fixes scylladb#44. This commit fixes scylladb#44 by removing that redundant flag

adespawn self-assigned this Mar 27, 2026

adespawn added the area/statement-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 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

Comment thread lib/utils.js

Comment thread src/logging.rs

Comment thread src/logging.rs

Comment thread docs/src/logging.md

Comment thread docs/src/migration_guide.md

Comment thread test/integration/supported/client-logging-tests.js

Comment thread docs/src/migration_guide.md

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

Comment thread src/logging.rs Outdated

Comment thread src/logging.rs

wprzytula reviewed Apr 16, 2026

View reviewed changes

Comment thread docs/src/logging.md Outdated

Comment thread docs/src/logging.md Outdated

Comment thread docs/src/migration_guide.md Outdated

adespawn force-pushed the logging-tracing branch from 1589d55 to 64017f8 Compare April 23, 2026 12:17

adespawn force-pushed the logging-tracing branch 2 times, most recently from dc99462 to 50e2b46 Compare April 27, 2026 12:19

adespawn force-pushed the logging-tracing branch 2 times, most recently from 0060f73 to 0d05018 Compare April 29, 2026 09:15

adespawn force-pushed the logging-tracing branch 3 times, most recently from aa81fa5 to 463df12 Compare April 30, 2026 14:29

adespawn force-pushed the logging-tracing branch from 463df12 to 05934f7 Compare April 30, 2026 14:37

adespawn force-pushed the logging-tracing branch from 05934f7 to 4bdd886 Compare May 11, 2026 09:39

adespawn force-pushed the logging-tracing branch from 4bdd886 to 97d31f2 Compare May 13, 2026 08:31

adespawn modified the milestones: 0.5.0, GA May 18, 2026

adespawn force-pushed the logging-tracing branch 4 times, most recently from dc55048 to ad14fec Compare May 19, 2026 13:35

adespawn requested a review from wprzytula May 19, 2026 14:38

wprzytula reviewed May 23, 2026

View reviewed changes

adespawn force-pushed the logging-tracing branch from ad14fec to 3ed77a3 Compare May 25, 2026 08:23

adespawn added 8 commits May 25, 2026 10:39

Add log event forwarding infrastructure

e999e09

Add logLevel client option for configurable log filtering

f9f434e

Add logLevels API tests for JS and TypeScript type definitions

ee37dd8

Add documentation about logging

e39cf34

Remove redundant --exit flag from integration tests

db073e0

At some point the code was changed, in a way that fixes scylladb#44. This commit fixes scylladb#44 by removing that redundant flag

Internal notes on logging

eb5efa6

adespawn force-pushed the logging-tracing branch from 3ed77a3 to eb5efa6 Compare May 25, 2026 10:21

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wprzytula Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wprzytula left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wprzytula commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adespawn commented Apr 23, 2026

Uh oh!

adespawn commented Apr 27, 2026

Uh oh!

adespawn commented Apr 29, 2026

Uh oh!

adespawn commented Apr 29, 2026

Uh oh!

adespawn commented Apr 30, 2026

Uh oh!

adespawn commented May 11, 2026

Uh oh!

adespawn commented May 19, 2026

Uh oh!

wprzytula left a comment

adespawn commented Mar 27, 2026 •

edited

Loading

adespawn Apr 16, 2026 •

edited

Loading