Client config reporting (3.x) — stage 1 (groundwork): SESSION_ID + DRIVER_CONFIG plumbing - #973
Open
nikagra wants to merge 1 commit into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds configurable driver configuration reporting to Sequence Diagram(s)sequenceDiagram
participant ClusterBuilder
participant Configuration
participant ConnectionFactory
participant ControlConnection
participant DefaultDriverConfigReporter
ClusterBuilder->>Configuration: enable reporting
ConnectionFactory->>DefaultDriverConfigReporter: create reporter
ControlConnection->>ConnectionFactory: open(host, true)
ConnectionFactory->>DefaultDriverConfigReporter: addOption(options, true)
DefaultDriverConfigReporter-->>ConnectionFactory: SESSION_ID and DRIVER_CONFIG
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
nikagra
force-pushed
the
feature/driver-config-reporting-3x
branch
from
July 30, 2026 11:48
9366dd6 to
1ec3ff3
Compare
nikagra
marked this pull request as ready for review
July 30, 2026 11:51
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/src/main/java/com/datastax/driver/core/Connection.java`:
- Around line 1301-1310: The DriverConfigReporter currently lives on the
cluster-wide Connection.Factory, causing multiple Session instances to share one
SESSION_ID. Move reporter creation and ownership from Factory to the actual
Session layer, and ensure each session’s connections reuse that session-scoped
reporter; update affected wiring and tests to preserve per-session grouping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a97e0b8c-9fad-48bf-9c86-bd7c8c548623
📒 Files selected for processing (8)
driver-core/src/main/java/com/datastax/driver/core/Cluster.javadriver-core/src/main/java/com/datastax/driver/core/Configuration.javadriver-core/src/main/java/com/datastax/driver/core/Connection.javadriver-core/src/main/java/com/datastax/driver/core/ControlConnection.javadriver-core/src/main/java/com/datastax/driver/core/DefaultDriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/DriverConfigReporter.javadriver-core/src/test/java/com/datastax/driver/core/DefaultDriverConfigReporterTest.javadriver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmIT.java
dkropachev
reviewed
Jul 30, 2026
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
driver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmTest.java (1)
195-205: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAggregate
system.clientsacross all nodes.
client_optionsis node-local, butallDriverRows()queries only the node selected bysession(). On a multi-node cluster this can omit the control row or pooled rows, making the “exactly one DRIVER_CONFIG” and disabled-reporting assertions incomplete or flaky. Query every node, or explicitly pin and aggregate the control-connection node; include node identity in connection keys if aggregating.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@driver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmTest.java` around lines 195 - 205, Update allDriverRows() to query and aggregate system.clients from every cluster node instead of only the session-selected node, preserving the DRIVER_NAME filter and including node identity in any connection keys used by the assertions. Ensure the resulting rows cover control and pooled connections so the existing count and disabled-reporting checks remain complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@driver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmTest.java`:
- Around line 195-205: Update allDriverRows() to query and aggregate
system.clients from every cluster node instead of only the session-selected
node, preserving the DRIVER_NAME filter and including node identity in any
connection keys used by the assertions. Ensure the resulting rows cover control
and pooled connections so the existing count and disabled-reporting checks
remain complete.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cd52ce3-2107-4f14-989d-e96c708f2887
📒 Files selected for processing (8)
driver-core/pom.xmldriver-core/src/main/java/com/datastax/driver/core/Cluster.javadriver-core/src/main/java/com/datastax/driver/core/Connection.javadriver-core/src/main/java/com/datastax/driver/core/DefaultDriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/DriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/NoopDriverConfigReporter.javadriver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmTest.javadriver-core/src/test/java/com/datastax/driver/core/NoopDriverConfigReporterTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- driver-core/src/main/java/com/datastax/driver/core/Cluster.java
nikagra
force-pushed
the
feature/driver-config-reporting-3x
branch
from
July 30, 2026 20:29
6cfd5f3 to
0073d32
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/pom.xml`:
- Around line 171-179: Prevent Connection.Factory from triggering a Jackson
NoClassDefFoundError during connection setup when reporting dependencies are
absent. Update DefaultDriverConfigReporter and the reporting initialization
around Connection.Factory so Jackson-dependent classes are loaded only after
availability is verified, or make jackson-core and jackson-databind required
instead of optional; preserve addOption()’s handling for unavailable reporting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ebf3485-784f-4f81-9a28-103e79c40ea1
📒 Files selected for processing (11)
driver-core/pom.xmldriver-core/src/main/java/com/datastax/driver/core/Cluster.javadriver-core/src/main/java/com/datastax/driver/core/Configuration.javadriver-core/src/main/java/com/datastax/driver/core/Connection.javadriver-core/src/main/java/com/datastax/driver/core/ControlConnection.javadriver-core/src/main/java/com/datastax/driver/core/DefaultDriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/DriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/NoopDriverConfigReporter.javadriver-core/src/test/java/com/datastax/driver/core/DefaultDriverConfigReporterTest.javadriver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmTest.javadriver-core/src/test/java/com/datastax/driver/core/NoopDriverConfigReporterTest.java
nikagra
force-pushed
the
feature/driver-config-reporting-3x
branch
from
July 30, 2026 20:53
0073d32 to
9d61c1b
Compare
dkropachev
reviewed
Jul 30, 2026
nikagra
force-pushed
the
feature/driver-config-reporting-3x
branch
from
July 30, 2026 23:52
9d61c1b to
a48ac94
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@driver-core/pom.xml`:
- Line 167: Add <optional>true</optional> to the jackson-core and
jackson-databind dependency declarations in driver-core/pom.xml, leaving their
existing versions and configuration unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8cbf6aa0-0778-4ac5-8373-9d8624f5a532
📒 Files selected for processing (12)
driver-core/pom.xmldriver-core/src/main/java/com/datastax/driver/core/Cluster.javadriver-core/src/main/java/com/datastax/driver/core/Configuration.javadriver-core/src/main/java/com/datastax/driver/core/Connection.javadriver-core/src/main/java/com/datastax/driver/core/ControlConnection.javadriver-core/src/main/java/com/datastax/driver/core/DefaultDriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/DriverConfigReporter.javadriver-core/src/main/java/com/datastax/driver/core/NoopDriverConfigReporter.javadriver-core/src/test/java/com/datastax/driver/core/DefaultDriverConfigReporterTest.javadriver-core/src/test/java/com/datastax/driver/core/DriverConfigReportingCcmTest.javadriver-core/src/test/java/com/datastax/driver/core/NoopDriverConfigReporterTest.javapom.xml
💤 Files with no reviewable changes (1)
- pom.xml
dkropachev
requested changes
Jul 31, 2026
Comment on lines
+1446
to
+1463
| /** | ||
| * Enables driver configuration reporting. | ||
| * | ||
| * <p>When enabled, every connection sends a driver-generated {@code SESSION_ID} startup option | ||
| * (so the server can group every connection opened from this {@link Cluster}, across all of its | ||
| * {@link Session}s), and the control connection additionally sends a {@code DRIVER_CONFIG} JSON | ||
| * blob describing the effective driver configuration. The server stores these in {@code | ||
| * system.clients.client_options} — a per-node table, so only the node holding the control | ||
| * connection stores {@code DRIVER_CONFIG}; consumers must query and aggregate across all nodes. | ||
| * Disabled by default; when disabled there is no change on the wire. | ||
| * | ||
| * @param enabled whether driver configuration reporting is enabled. | ||
| */ | ||
| public Builder withDriverConfigReporting(boolean enabled) { | ||
| configurationBuilder.withDriverConfigReporting(enabled); | ||
| return this; | ||
| } | ||
|
|
Author
There was a problem hiding this comment.
Done in 40b507f3ab: Configuration.Builder now defaults it to true, and the CCM test builds its cluster with no withDriverConfigReporting call so default-on is covered end-to-end. One flag to raise: merged 4.x #967 ships this default-off and gates SESSION_ID behind it, so 3.x and 4.x now differ — want me to open a follow-up aligning 4.x with this?
…stage 1) Stage 1 (groundwork) of driver configuration reporting for the 3.x driver -- the 3.x counterpart of the 4.x feature (DRIVER-381/scylladb#967). Lets the driver report its effective configuration to ScyllaDB at connection time via new STARTUP options, so operators can inspect driver settings (system.clients.client_options) while investigating incidents. Two STARTUP options are added: - SESSION_ID: a dedicated, driver-generated UUID sent on every connection (control and pool) unconditionally, like DRIVER_NAME and DRIVER_VERSION, so the server can group all of a Cluster's connections -- including across multiple Sessions obtained from the same Cluster, since the control connection has no affiliation with any single Session. Independent of the user-settable CLIENT_ID. - DRIVER_CONFIG: a compact JSON blob describing the effective configuration, sent only on the control connection. Stage 1 emits only {"version":1}; the full report follows in stage 2 (scylladb#974). Enabled by default; opt out with Cluster.builder().withDriverConfigReporting(false). The report is built once, while the Cluster initializes, and the resulting string is reused for every control connection that Cluster opens -- it is never rebuilt while the session is in flight. Building it is fail-safe: any failure is swallowed and simply leaves DRIVER_CONFIG unset instead of breaking cluster initialization. New DriverConfigReporter / DefaultDriverConfigReporter (package com.datastax.driver.core) build the blob. Connection.Factory, of which there is one per Cluster, holds that Cluster's session id and the built report, and hands the report to the control connection as a constructor argument -- null everywhere else, which is what suppresses reporting. The control connection is identified by threading a reportConfig flag from ControlConnection.tryConnect through a new Connection.Factory.open(host, reportConfig) overload, since 3.x has no pre-existing signal identifying the control connection at STARTUP time. jackson-core/jackson-databind are enforced as plain required dependencies (as they already were in released 3.11.5.17), used to build the JSON blob; the orphaned jackson-dataformat-yaml dependency (dead since the Scylla Cloud config code was removed) is dropped, so consumers no longer inherit SnakeYAML. system.clients.client_options is per node, so DRIVER_CONFIG only appears on the node holding the control connection. Fixes DRIVER-382 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nikagra
force-pushed
the
feature/driver-config-reporting-3x
branch
from
July 31, 2026 16:57
a48ac94 to
40b507f
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 ☑️
Stage 1 (groundwork) of client configuration reporting for the 3.x driver — the 3.x counterpart of the 4.x feature. It lets the driver report its effective client configuration to ScyllaDB at connection time, so operators can inspect driver settings (via
system.clients.client_options) while investigating incidents. Follows the cross-driver JSON schema design.This is phase 1 of 2 — it lands the plumbing; the full configuration report (all groups) comes in phase 2 (#974).
Behavior 👶
Two
STARTUPoptions are added:SESSION_ID— a dedicated, driver-generated UUID, sent on every connection (control and pool), unconditionally — like theDRIVER_NAME/DRIVER_VERSIONoptions the driver already always sends. One id perCluster, shared by every connection opened from it — including across multipleSessions obtained from the sameCluster— so the server can group them. It's Cluster-scoped rather than Session-scoped because the control connection isn't tied to any particularSession(see discussion in review). Independent of the user-settableCLIENT_ID.DRIVER_CONFIG— a compact JSON blob, sent only on the control connection. Enabled by default; opt out withCluster.builder().withDriverConfigReporting(false). Stage 1 emits only{"version":1}; the full report follows in phase 2.system.clients.client_optionsis per node, so only the node holding the control connection stores this — consumers must query and aggregate across all nodes to see the full picture.The report is built once, while the
Clusterinitializes, and the resulting string is reused for every control connection thatClusteropens — it is never rebuilt while the session is in flight, so a control-connection reconnect costs nothing and always reports the same configuration. Building it is fail-safe: any failure is swallowed and simply leavesDRIVER_CONFIGunset rather than breaking cluster initialization.Changes ✍️
DriverConfigReporter/DefaultDriverConfigReporter(packagecom.datastax.driver.core) build theDRIVER_CONFIGpayload — a singlebuildReport()call that returns the JSON, ornullif it couldn't be built.Connection.Factory(one perCluster) holds that cluster'sSESSION_IDand, when reporting is enabled, the report built at initialization time. It hands the report to the control connection as a constructor argument —nulleverywhere else, andnullis exactly what suppressesDRIVER_CONFIG. No reporter instance is retained, and none is constructed at all when reporting is off.Connection.onOptionsReady(), right after the existingapplicationInfostartup-option hook.reportConfigflag fromControlConnection.tryConnectthrough a newConnection.Factory.open(host, reportConfig)overload — 3.x has no pre-existing signal that identifies the control connection atSTARTUPtime.Cluster.Builder→Configuration.Builder→Configuration(default true).jackson-core/jackson-databindare enforced as plain required dependencies (as they already were in released 3.11.5.17), used byDefaultDriverConfigReporterto build the JSON blob; the orphanedjackson-dataformat-yamldependency (dead since the Scylla Cloud config code was removed) is dropped, so consumers no longer inherit SnakeYAML. OSGi manifests carry mandatorycom.fasterxml.jackson.*;version="[2.18,3)"imports accordingly.Testing 🔨
Unit tests
DefaultDriverConfigReporterTest: reporting is enabled by default (bothConfiguration.builder()andCluster.builder()), the stage-1 report is{"version":1}, and report building is fail-safe (a throwing build yieldsnullinstead of propagating).driver-corecompiles and unit tests are green.Integration tests
DriverConfigReportingCcmTest(live ScyllaDB,@ScyllaVersion≥ 2026.1; named*Testrather than*ITso Surefire's default discovery actually picks it up — see review discussion) asserts what the server stores insystem.clients:SESSION_IDis stored for every connection of the cluster, andDRIVER_CONFIGfor exactly one (the control connection);Sessions opened from the sameClustershare oneSESSION_ID;SESSION_IDis still stored butDRIVER_CONFIGis not.The cluster under test uses the default configuration (no
withDriverConfigReportingcall), so these also cover default-on end-to-end. Each cluster's connections are identified by theSESSION_IDthe driver generated for it, so the assertions can't be confused by another cluster's rows. The end-to-end verification of the full report lands with phase 2.Verified live against ScyllaDB 2026.1 — all 3 methods pass.
Manual tests 🖐
Captured the actual
STARTUPframes on the wire with tshark, against a live single-node ScyllaDB 2026.1.0 (CCM), to directly confirm the plumbing — same method as #967's stage-1 verification for the 4.x driver.Environment: single node ScyllaDB 2026.1.0 on CCM, contact point
127.0.0.1:9042, minimalCluster.builder().addContactPoint("127.0.0.1").withPort(9042)— samples 1 and 2 with nowithDriverConfigReportingcall at all (i.e. the default), sample 3 withwithDriverConfigReporting(false).Sample 1: Control connection STARTUP frame (default configuration)
Note
Note both
DRIVER_CONFIGandSESSION_IDare present, with no configuration needed to enable themSample 2: Pool connection STARTUP frame (default configuration)
Note
Note only
SESSION_IDis present — same value as Sample 1, since it's the same clusterSample 3: Control connection STARTUP frame (
withDriverConfigReporting(false))Note
Note
DRIVER_CONFIGis gone, whileSESSION_IDis still sent — it is not governed by the flagNote on cross-driver consistency⚠️
The merged 4.x stage 1 (#967) currently differs from what this PR now does, on three points requested in review here: it ships disabled by default (
advanced.driver-config-reporting.enabled = false), it gatesSESSION_IDbehind that same flag, and it rebuilds the blob on every control-connection init. Happy to align 4.x in a follow-up if we want the two drivers to behave identically — just say the word and I'll open it.Follow-up (phase 2)
Fixes DRIVER-382
🤖 Generated with Claude Code