Driver config reporting — stage 2: full DRIVER_CONFIG report

[nikagra]

What ☑️

Stage 2 (the payload) of driver configuration reporting: replaces the stage-1 {"version":1}
placeholder with the full DRIVER_CONFIG report — the effective configuration of the driver's
default execution profile plus the context's policies, serialized to the normative cross-driver JSON
schema shape. Stage 1 (#967) is merged; this branch is rebased onto scylla-4.x, so there is no
stage-1 noise in the diff.

Gated behind advanced.driver-config-reporting.enabled, which ships enabled (per
@dkropachev's cross-driver review). Turning it off suppresses only the DRIVER_CONFIG blob —
SESSION_ID rides on every connection unconditionally, independently of this flag, so "off" is not
"zero change on the wire".

Read the commits in order; each is formatter-clean and green on its own.

Latest push — one new commit, and an amend that moved every SHA from feat: expose the latched policy state … onward.
4ddf4ca makes query.speculative-execution.policy report the max-executions and delay-ms the
running policy actually uses, rather than re-reading them from a profile that may have been reloaded
since — see the second bullet under Design decisions. The two accessors it needs were folded into
feat: expose the latched policy state the config report needs to read, where the other four
already live, so the history stays one concern per commit. Raised by @dkropachev on the 3.x port
(#974), where the same two fields were misreported as a custom policy.

The report 🧩

Built from the default execution profile + the context's policies, and rebuilt on every
control-connection init, so it always reflects the current (possibly runtime-reloaded) config.
Three groups: connection (connect timeout, request capacity, pooling, socket options,
reconnection policy, TLS when on, and the datacenter preference that scopes pooling),
control-plane (internal-query and schema-agreement timeouts), and query (per-request
defaults, plus the retry, load-balancing and speculative-execution policies).

Against the shipped default configuration, 938 bytes (pretty):

{
    "version": 1,
    "connection": {
        "connect": {
            "timeout-ms": 5000
        },
        "requests": {
            "in-flight": {
                "max": 1024
            },
            "orphaned": {
                "max": 256
            }
        },
        "pool": {
            "shard-aware": {
                "enabled": true
            }
        },
        "socket": {
            "tcp-no-delay": true,
            "keep-alive": false,
            "reuse-address": false
        },
        "reconnection": {
            "policy": {
                "type": "exponential",
                "base-ms": 1000,
                "max-ms": 60000
            }
        },
        "node-preference": {
            "type": "dc-auto"
        }
    },
    "control-plane": {
        "queries": {
            "system": {
                "timeout": {
                    "client-side-ms": 5000
                }
            }
        },
        "schema": {
            "agreement": {
                "timeout-ms": 10000
            }
        }
    },
    "query": {
        "defaults": {
            "page": {
                "size": 5000
            },
            "consistency": "LOCAL_ONE",
            "serial-consistency": "SERIAL",
            "idempotence": false,
            "client-timestamps": true,
            "request": {
                "timeout-ms": 2000
            }
        },
        "retry": {
            "policy": {
                "type": "standard-error-aware"
            }
        },
        "load-balancing": {
            "policy": {
                "type": "token-aware",
                "load-distribution": "shuffle",
                "fallback-to-non-preferred-nodes": false,
                "adaptive-ordering": {
                    "signals": [
                        "response-rate",
                        "in-flight-requests",
                        "recovery-state"
                    ]
                }
            },
            "node-preference": {
                "type": "dc-auto"
            }
        }
    }
}

Verified on the wire by a tshark capture of the STARTUP frames against a single-node CCM
ScyllaDB (protocol v4, default config): SESSION_ID on every connection, DRIVER_CONFIG only on the
control connection, and gone when the flag is off. Verified end to end through
system.clients.client_options (ScyllaDB 2026.1) and system_views.clients (Cassandra 4.1),
untruncated, with the one backend-conditional key differing as it should.

Invariants 🔒

• Fail-safe. Any failure while building the report is swallowed and logged at WARN; SESSION_ID
  is still emitted and only the config blob is dropped. RuntimeException only — deliberately not
  bare Error, so OutOfMemoryError/StackOverflowError still surface. The one Error this class
  can provoke is the InternalError that getClass().getSimpleName() raises for certain synthetic
  classes; that is caught at the call site, behind a package-private seam so the branch stays
  testable.
• Jackson can still be excluded. manual/core/integration documents that the driver "can operate
  normally without" Jackson, and DefaultDriverContext already honours that for Insights by checking
  DefaultDependencyChecker.isPresent(JACKSON) before building the listener. Reporting was a third
  Jackson user with no such check, and it is default-on and on the connection-init path — so on that
  classpath linking DefaultDriverConfigReporter raised NoClassDefFoundError, an Error raised
  while resolving the class rather than from any method it declares. Neither the try/catch above
  nor ProtocolInitHandler could contain it: every control connection failed and the session could
  not be built. Now the implementation is chosen up front, falling back to a NoopDriverConfigReporter
  that names no Jackson type anywhere (one reference would make loading it fail for exactly the
  deployments it exists to serve). Logged unconditionally, unlike Insights: nobody opted in to
  reporting, so nobody would think to look for a message saying it is off. Verified both ways against
  core's real runtime classpath with its Jackson jars removed.
• The reporter is resolved during session init, alongside the policies DefaultSession.init()
  already forces eagerly. It was the one component on the reporting path left lazy, which made a
  Netty event loop the first thread to load it and Jackson — jar reads, mid-STARTUP. This is also
  what makes the ordering buildJson()'s javadoc relies on true by construction.
• Size cap — 32KiB, matching gocql fix: release pre-acquired stream IDs #964 and csharp-driver Bump ch.qos.logback:logback-classic from 1.2.3 to 1.3.12 #262. Not just parity: STARTUP option
  values go through ByteBufPrimitiveCodec.writeString, which writes a 16-bit length prefix via
  ByteBuf.writeShort with no bounds check, so a value over 65535 bytes silently truncates the
  prefix modulo 65536 while still appending the whole body — a corrupt frame and a failed handshake,
  and not something the try/catch can save, since nothing throws. Parts of the report are
  user-supplied and unbounded (DC/rack names, consistency levels, custom policy class names), so
  without this the "reporting must never prevent a connection" invariant simply wasn't true. Measured
  on the UTF-8 bytes, since that is what the prefix counts.
• Omission principle. A key the driver has no equivalent for is left out entirely, never emitted
  as null. Same where an optional key's configured value is outside what the schema can express
  (a disabled request timeout, a disabled SO_LINGER, an unbounded page size), and same where the
  answer is genuinely unknown — the two cases the schema made optional for exactly that purpose.
• No missing config option costs more than the field it describes. Twelve reads used the
  no-fallback getters, which throw on an absent option, so a config source omitting any one of them
  dropped all ~34 fields behind a single WARN. Every read now either sits behind isDefined or
  passes an explicit fallback, and the schema picks which: an optional field falls back to the same
  "disabled" sentinel that already omits it, a required one to the value reference.conf documents.

Design decisions worth questioning 📐

• Policy groups use exact-class discrimination, not instanceof — so a user subclass of a
  built-in falls through to {type:"custom", name:<class>} instead of being misreported as the
  unmodified built-in.
• Policy parameters are read off the running instance, not the profile. connection.reconnection.policy
  and query.speculative-execution.policy describe the policy that is actually reconnecting and
  speculating. The built-ins latch these numbers into final fields when the context builds them, and
  advanced.speculative-execution-policy is documented as not modifiable at runtime, so a
  reloaded profile can carry values no request executes with — and, unlike a constructor, admits
  values the schema rejects: a negative delay-ms, or a max-executions of 1 that would drop the
  whole group while the policy still speculates. Reading the instance makes those ranges hold by
  construction. Two policy-derived values still read the profile — adaptive-ordering and
  fallback-to-non-preferred-nodes — because those policies expose no accessor for what they latched;
  the class javadoc names them rather than leaving it implied.
• control-plane.queries.system.timeout.server-side-ms is ScyllaDB-only. CassandraSchemaQueries
  adds a USING TIMEOUT clause to schema queries there, making
  advanced.metadata.schema.request-timeout a genuine server-side timeout on that backend; omitted
  on generic Cassandra. Driven by a scyllaDb signal derived on the control connection from the
  feature store's sharding info — the same proxy check shouldApplyUsingTimeout() already uses.
• Whether TLS is on reads getSslHandlerFactory(), not getSslEngineFactory() (per
  @sylwiaszunejko). The handler factory is the reference ChannelFactory installs the SSL handler
  from, and buildSslHandlerFactory() is the documented expert extension point (e.g. Netty's native
  OpenSSL): an override supplies no engine factory, so reading the engine factory reported such a
  session as plaintext when it is in fact encrypted. Host name validation is then read off the engine
  factory the active handler actually wraps — never through the context, which can name a different,
  unused one and whose LazyReference the reporter would be the first to force (keystore reads on a
  Netty event loop, mid-STARTUP).
• Two new SPI accessors, both tri-state. SslEngineFactory.isHostnameValidationRequired() and
  TimestampGenerator.isClientSide() return Optional<Boolean>, empty by default. Host name
  validation is a property of the JDK SSLEngine, unreadable through an opaque handler factory; and
  a custom TimestampGenerator is free to return Statement.NO_DEFAULT_TIMESTAMP and delegate to
  the coordinator, which no class check can detect and which calling next() to find out would have
  side effects. Both keys are now optional in the schema with absence defined as unknown, so an
  implementation that cannot answer is reported by omission rather than by a guessed boolean — which
  for these two fields would misdescribe a security control and a write-timestamp source. Both
  methods are default, so existing implementations keep compiling.
• Sub-millisecond durations floor at 1 ms. The schema counts whole milliseconds while the driver
  holds these options as Duration and schedules several in nanoseconds, so truncating a 500 µs
  timeout to 0 would report a live timeout as the very value the field defines as off. Applies to
  schema.agreement.timeout-ms, queries.system.timeout.client-side-ms,
  query.defaults.request.timeout-ms and reconnection.policy.delay-ms. Three fields are
  deliberately exempt, because 0 is what they really mean there: connection.connect.timeout-ms
  (Netty's CONNECT_TIMEOUT_MILLIS truncates identically, and 0 disables it), ...server-side-ms
  (the value goes on the wire as a USING TIMEOUT millisecond argument, so sub-millisecond really
  is 0ms server-side) and speculative-execution.policy.delay-ms (reference.conf documents
  sub-millisecond delays as equivalent to 0).
• connection.requests.orphaned.max is the effective threshold, not the configured one.
  ChannelFactory requires max-orphan-requests to stay below max-requests-per-connection and
  silently substitutes a quarter of the latter otherwise. Reporting the configured value would
  describe a threshold no connection was built with, so the correction lives in one place —
  ChannelFactory.effectiveMaxOrphanRequests(), which the channel setup itself calls.
• The two node-preference slots are filled differently, because in Java they mean different
  things. computeNodeDistance derives node distance from the local DC alone — a node outside it is
  IGNORED, and an IGNORED node gets no pool — so the datacenter genuinely scopes which nodes are
  connected to, and goes under connection.node-preference. The rack never reaches that method: it
  only reorders replicas at the head of a query plan, with connections still held across the whole
  local DC. So the full preference (rack included) goes under query.load-balancing.node-preference,
  and the connection group carries the datacenter half alone. Emitting the same object in both would
  claim a rack-scoped connection pool that does not exist.
• load-distribution is shuffle and adaptive-ordering maps to slow-replica avoidance. The
  built-ins shuffle the replica head of every query plan unconditionally
  (BasicLoadBalancingPolicy.shuffleHead, no config to disable), so round-robin would describe only
  the non-replica tail and replica-set would claim the order is untouched (see A1 for the one case
  this misses). Java has no latency-percentile ordering, so adaptive-ordering maps to the one real
  mechanism, DefaultLoadBalancingPolicy's slow-replica avoidance, with its signals read off
  avoidSlowReplicas rather than guessed — and latency deliberately absent, since those samples
  record when responses arrived, not how long they took. Its presence is also now the only thing
  distinguishing BasicLoadBalancingPolicy in the report, which has no such mechanism at all.

Spec conformance 🔍

The v1 schema is shipped verbatim as a test resource, byte-identical to the design document's
schema block, and every representative report is validated against it in
DefaultDriverConfigReporterTest — enforced, not asserted. A negative test confirms the validator
actually rejects an out-of-schema document.

Every report a stock configuration can produce validates. Two required fields are constrained more
tightly than the option behind them, but only one is reachable through a running driver. Both are
reported truthfully and pinned by tests that assert the violation:

• query.defaults.consistency is a closed enum while basic.request.consistency is an unvalidated
  string. The built-in load balancing policies resolve it through the ConsistencyLevelRegistry in
  their constructor, so an unknown name fails the session before any report exists — reaching this
  needs a custom registry defining extra names, which is the case CodeRabbit raised. This is the
  one real gap.
• connection.requests.in-flight.max must be positive, and nothing validates
  advanced.connection.max-requests-per-connection against that — ChannelFactory hands the value
  straight to StreamIdGenerator, which does not range-check it. An earlier revision of this
  description claimed such a setting starts a session; it does not. The connection fails first: a
  negative value makes StreamIdGenerator's BitSet throw while ChannelFactory is still building
  the channel, and 0 leaves no stream id for the control connection's own OPTIONS, which
  ChannelHandlerRequest fails on preAcquireId before STARTUP is composed. So this is unreachable
  by construction, not a live exposure. The value is still passed through and still pinned, so the
  behaviour stays defined if the driver ever stops failing that early. (The same setting would also
  drive orphaned.max negative — a second reason to read it as one unreachable shape rather than one
  field's gap.)

A third shape was reachable until this branch's latest push: query.speculative-execution.policy
took both its numbers from the profile, so a reload could put a negative delay-ms — which
nonNegativeInteger rejects — into an otherwise valid document, or drop the group while the policy
still speculated. Both now come off the policy, whose constructor admits neither.

Fabricating an admissible value would misreport a setting an operator may have chosen deliberately,
and dropping the whole report would punish every other group for one field.

Approximations, flagged not changed ⚠️

	Field	Reported as	Why that is an approximation	For
A1	load-balancing.policy.load-distribution	always shuffle	LWT / serial-consistency requests take newQueryPlanPreserveReplicas, which never shuffles — replica-set in schema terms — and default-lwt-request-routing-method ships as PRESERVE_REPLICA_ORDER. So every LWT statement on a default config is distributed the way the report says it is not. No single enum value is honest.	schema owner
A2	load-balancing.policy.fallback-to-non-preferred-nodes	max-nodes-per-remote-dc > 0 and a datacenter preference exists	Both terms are now required, which was the fix; one is still missing. maybeAddDcFailover also consults isDcFailoverAllowedForRequest, false for a DC-local consistency while allow-for-local-consistency-levels is off — and both of those ship as the default, so on a config that changes nothing but max-nodes-per-remote-dc the report says true while no ordinary statement fails over. Note the "it's per-request, a statement can override it" argument does not carry on its own: query.defaults.consistency is published under the same caveat. The real cost is that closing it needs ConsistencyLevelRegistry resolution of a string this report deliberately passes through unvalidated. A schema value meaning "conditional" is the honest fix.	java, deliberate
A3	connection.socket.keep-alive, .reuse-address	false when unset	The driver never touches either socket option unless configured, so the effective value is the platform's — which is what the schema asks for, and which StandardSocketOptions documents as system dependent. false holds for JDK NIO on Linux; unverified for the native transports. Both keys are required, so omission is not available.	java
A4	control-plane.queries.system.timeout.client-side-ms	CONTROL_CONNECTION_TIMEOUT	Schema queries' own client-side wait is METADATA_SCHEMA_REQUEST_TIMEOUT, so the two siblings do not describe the same query — an operator debugging a slow schema query reads the wrong number. Already on the thread with @dkropachev; the fix is a queries.schema sibling, blocked today by additionalProperties:false.	schema owner
A5	node-preference datacenter / rack values	trimmed, blank treated as unset	OptionalLocalDcHelper / OptionalLocalRackHelper pass the configured string to the policy verbatim and match with Objects.equals, so a configured " dc1 " is reported as dc1 while the running policy matches no node. Kept and test-pinned: nonEmptyString leaves no way to report "", and type:"dc" with the key omitted is invalid too.	java, deliberate
A6	query.load-balancing.node-preference type:"rack"	whenever a DC and a rack are configured	Rack awareness lives only in DefaultLoadBalancingPolicy; BasicLoadBalancingPolicy never reads localRack, and PRESERVE_REPLICA_ORDER ignores it as well. Kept — the value is configured, and hiding a real setting is the worse failure mode.	java
A7	both node-preference slots, for a custom load balancing policy	a configured datacenter is reported whatever the policy is	Both parents claim an effect only the built-ins produce: connection's says the DC decides which nodes hold a pool, which holds because BasicLoadBalancingPolicy#computeNodeDistance makes an out-of-DC node IGNORED; query.load-balancing's says it scopes routing. A custom policy computes distance itself and need not read local-datacenter or withLocalDatacenter at all. Kept on A6's grounds. Deliberately asymmetric with the no-DC case, where the group is omitted rather than reporting a dc-auto the SPI never promises: nothing is inferred on a custom policy's behalf, while what was configured is passed through.	java, deliberate

Two cosmetic ones, noted for completeness: a negative schema.agreement.timeout-ms normalizes to 0
(same outcome as 0, one extra round trip, and the schema cannot say "negative"); and
connection.connect.timeout-ms is reported as a full long while DefaultNettyOptions narrows it
with intValue(), so a connect timeout past ~24.8 days wraps in Netty.

Follow-up ⏭️

For the schema owner — all for the document rather than here:

• The revision updated the schema block but not the prose: the per-driver mapping tables still
  describe fields the schema no longer has, and both sample payloads still show the pre-restructure
  flat envelope, so they fail validation against the document's own schema.
• $id and version still say v1 / const: 1 although earlier revisions removed a required
  top-level group and renamed load-balancing fields. By the schema's own versioning rule that is a
  major bump; harmless while every implementation is unreleased, but a v1 consumer cannot tell the
  shapes apart.
• No size limit is specified even though all three drivers now enforce 32KiB.
• dc-auto carries the inferred value in plain local-dc while rack-auto uses an explicit
  inferred- prefix. Implemented as specified; the asymmetry is easy to misread.
• node-location-preference has no "no preference" variant (raised by @dkropachev). Omitting the
  optional group is the schema-valid answer and is what this PR does, but a none type would say it
  positively.
• query.defaults.consistency needs either a wider type or a documented rule for names outside its
  enum — the one conformance gap a running Java driver can still produce. (An earlier revision of
  this list also asked the spec to define consumer behaviour for a non-positive in-flight.max;
  withdrawn — see Spec conformance, no session can reach it.)
• control-plane.queries.system.timeout groups client-side-ms and server-side-ms as two views of
  one timeout. For Java they are not — see A4; a queries.schema sibling would let each class of
  query carry an honest pair.
• speculative-execution.policy.percentile is exclusiveMinimum: 0, while 3.x's
  PercentileSpeculativeExecutionPolicy accepts 0.0 — so an accurate report of that configuration
  is out of schema (raised by @dkropachev on Client config reporting (3.x) — stage 2: full DRIVER_CONFIG report #974). Unreachable from Java 4.x, which has no percentile
  policy at all, but the schema is shared.

Other:

• The 3.x port (Client config reporting (3.x) — stage 2: full DRIVER_CONFIG report #974, DRIVER-382) lags this branch by several schema revisions and needs the same
  restructure. The sub-millisecond reconnection floor does not carry over: 3.x's
  ConstantReconnectionPolicy holds a long delayMs, so there is no sub-millisecond value to
  truncate. Separate PR, separate branch. Traffic goes the other way too: the
  speculative-execution source-of-truth fix in this push was raised there first, and 3.x additionally
  had to stop reporting both built-ins as custom, which this branch never did.
• Two CodeRabbit flags asking to "restore opt-in" reporting are declined, not overlooked — the
  default-on flip is intentional; reasoning is on the threads.
• DriverBlockHoundIntegrationIT is JDK 14+ only and was not run locally. With reporting on by
  default the report is built on a Netty event loop; reasoned safe (no SSL factory resolution or IO
  with the default config, and Jackson is in-memory), but worth watching in CI. The larger half of
  that risk is gone: the reporter is now resolved during session init, so the event loop is no longer
  the first thread to load it and Jackson.
• The 3.x port needs the Jackson guard too, if 3.x makes Jackson excludable the same way. Not checked
  here.
• One thing found while re-auditing and not changed: relative links in prose across
  manual/ render with a spurious # prefix (href="#../configuration/reference/") — a site-wide
  MyST artifact affecting pre-existing links too, so it wants its own issue rather than a partial fix
  here. The one link this PR would have added was dropped for that reason.

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The driver now reports expanded default-profile configuration through control-connection STARTUP options. ProtocolInitHandler derives ScyllaDB status from negotiated sharding information. SESSION_ID remains stable for a session and is sent on every connection. Tests validate the stage-2 payload against a version 1 schema.

Sequence Diagram(s)

sequenceDiagram
  participant StartupOptionsBuilder
  participant ProtocolInitHandler
  participant FeatureStore
  participant DriverConfigReporter
  StartupOptionsBuilder->>ProtocolInitHandler: provide stable SESSION_ID
  ProtocolInitHandler->>FeatureStore: read sharding information
  ProtocolInitHandler->>DriverConfigReporter: build control-connection DRIVER_CONFIG

Loading

Possibly related PRs

• scylladb/java-driver#967: Provides related driver configuration reporting startup plumbing.
• scylladb/java-driver#973: Extends the related SESSION_ID and DRIVER_CONFIG startup flow.

Suggested labels: P1, area/Driver_-_java-driver-4.x

Suggested reviewers: dkropachev, sylwiaszunejko

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.64% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The description references related follow-up work and issue numbers that align with the reporting and schema changes.
Out of Scope Changes check	✅ Passed	The supporting API, protocol, schema, test, and documentation changes directly support full DRIVER_CONFIG reporting.
Title check	✅ Passed	The title clearly identifies the pull request as implementing the full DRIVER_CONFIG report for stage 2.
Description check	✅ Passed	The description directly explains the full DRIVER_CONFIG report, its behavior, safeguards, schema, and test coverage.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Fix failing CI checks

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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
`@core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java`:
- Around line 1178-1187: Make the reporting documentation backend-neutral across
DefaultDriverOption, TypedDriverOption, and reference.conf: replace
ScyllaDB-only wording with server-side terminology or explicitly document both
storage paths, system.clients for ScyllaDB and system_views.clients for
Cassandra 4.1. Update all three affected sites consistently without changing the
reporting behavior.

🪄 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: ab5c16aa-71fa-4f9a-9659-654b6920ae50

📥 Commits

Reviewing files that changed from the base of the PR and between 088290a and c6f7ca3.

📒 Files selected for processing (12)

• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java

[coderabbitai]

🧹 Nitpick comments (4)

core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java (1)

51-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Contract extension is consistent with implementation and callers.

The new scyllaDb param and its "only meaningful with reportDriverConfig" contract match DefaultDriverConfigReporter.populateStartupOptions and ProtocolInitHandler's caller.

One minor note for the future: this interface now has two adjacent boolean parameters (reportDriverConfig, scyllaDb), which is a classic call-site readability/mix-up risk (e.g. populateStartupOptions(opts, true, false) reads ambiguously without named-parameter comments, as seen in the test file). Not blocking, but if a third flag is ever added, consider a small options value object instead.

🤖 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
`@core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java`
around lines 51 - 57, The comment identifies no required code change; the
current scyllaDb parameter and contract are consistent with the implementation
and callers. Leave DriverConfigReporter.populateStartupOptions and its call
sites unchanged, and only consider introducing an options value object if
another boolean flag is added later.

core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java (1)

194-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Centralize the ScyllaDB predicate. Both this startup path and CassandraSchemaQueries.shouldApplyUsingTimeout() key off the same shardingInfo != null signal; a shared helper would keep control-plane reporting and schema-query behavior in sync.

🤖 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
`@core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java`
around lines 194 - 210, Centralize the ScyllaDB detection based on
getShardingInfo() != null in a shared helper, then update ProtocolInitHandler’s
startup reporting and CassandraSchemaQueries.shouldApplyUsingTimeout() to use
it. Preserve the existing featureStore population flow and behavior while
ensuring both paths rely on the same predicate.

core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java (1)

136-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

control no longer exercises the control-connection path.

Both calls pass reportDriverConfig=false, so the map named control is identical to pool. Passing true for the control map keeps the test name honest and additionally proves the session id is stable when the config blob is built.

♻️ Suggested tweak

-    reporter.populateStartupOptions(control, false, false);
+    reporter.populateStartupOptions(control, /* reportDriverConfig= */ true, false);
     reporter.populateStartupOptions(pool, false, false);

🤖 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
`@core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java`
around lines 136 - 145, Update should_use_a_stable_session_id_across_connections
so the control map calls reporter.populateStartupOptions with
reportDriverConfig=true, while keeping the pool call false and preserving the
session ID equality assertion.

integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java (1)

145-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stage-2 payload assertion is copy-pasted across both integration tests. Both classes carry an identical assertDriverConfigPayload (same Javadoc, same checks); every future stage-2 assertion has to be added twice and will silently drift otherwise.

• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java#L145-L169: move this helper into a shared test utility (e.g. a package-private DriverConfigReportAssertions class in this package) and call it from here.
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java#L125-L149: delete the local copy and call the shared helper instead.

🤖 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
`@integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java`
around lines 145 - 169, Extract the duplicated assertDriverConfigPayload helper
into a package-private shared DriverConfigReportAssertions test utility,
preserving its existing JSON parsing and stage-2 validation checks. In
integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
lines 145-169, replace the local helper with a call to the shared utility; in
integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java
lines 125-149, delete the local copy and call the same utility.

🤖 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.

Nitpick comments:
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java`:
- Around line 194-210: Centralize the ScyllaDB detection based on
getShardingInfo() != null in a shared helper, then update ProtocolInitHandler’s
startup reporting and CassandraSchemaQueries.shouldApplyUsingTimeout() to use
it. Preserve the existing featureStore population flow and behavior while
ensuring both paths rely on the same predicate.

In
`@core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java`:
- Around line 51-57: The comment identifies no required code change; the current
scyllaDb parameter and contract are consistent with the implementation and
callers. Leave DriverConfigReporter.populateStartupOptions and its call sites
unchanged, and only consider introducing an options value object if another
boolean flag is added later.

In
`@core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java`:
- Around line 136-145: Update should_use_a_stable_session_id_across_connections
so the control map calls reporter.populateStartupOptions with
reportDriverConfig=true, while keeping the pool call false and preserving the
session ID equality assertion.

In
`@integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java`:
- Around line 145-169: Extract the duplicated assertDriverConfigPayload helper
into a package-private shared DriverConfigReportAssertions test utility,
preserving its existing JSON parsing and stage-2 validation checks. In
integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
lines 145-169, replace the local helper with a call to the shared utility; in
integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java
lines 125-149, delete the local copy and call the same utility.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5db2451e-3027-4740-bf80-230694676062

📥 Commits

Reviewing files that changed from the base of the PR and between c6f7ca3 and 24062e9.

📒 Files selected for processing (15)

• core/pom.xml
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java
• core/src/test/resources/config/driver-config-report-v1.schema.json
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java
• pom.xml

🚧 Files skipped from review as they are similar to previous changes (5)

• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/resources/reference.conf
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

[nikagra]

@coderabbitai Thanks for the review — dispositions for the nitpicks, all pushed into the amended stage-2 commit:

• D (test didn't exercise the control path): Fixed. should_use_a_stable_session_id_across_connections now passes reportDriverConfig=true for the control map, so it actually builds the DRIVER_CONFIG blob while asserting the session id is stable across connections.
• E (duplicated assertDriverConfigPayload): Fixed. Extracted into a shared, package-private DriverConfigReportingAssertions in the same package; both DriverConfigReportingCcmIT and DriverConfigReportingSimulacronIT now call it.
• B (two adjacent boolean params → options object): Leaving as-is for now, matching the note — the flags are documented at the interface and the call sites; worth revisiting with an options value object only if a third flag is ever added.
• C (centralize the shardingInfo != null predicate): Skipping, deliberately. The two call sites reach sharding info through different objects and types — ProtocolInitHandler via ProtocolFeatureStore.getShardingInfo() → ShardingInfo.ConnectionShardingInfo, and CassandraSchemaQueries.shouldApplyUsingTimeout() via DriverChannel.getShardingInfo() → ShardingInfo. A shared predicate would be two trivial null-checks on unrelated types with no real logic to reuse, and both sites already cross-reference each other in comments.

Unrelated to the nitpicks but included in the same push: a TLS reporting correctness fix — tls.hostname-verification now reflects the SSL factory's real state via SslEngineFactory.requireHostnameValidation() rather than the advanced.ssl-engine-factory.hostname-validation option, which only governs the built-in DefaultSslEngineFactory (a factory supplied through SessionBuilder.withSslContext(...) would otherwise be mis-reported as validating).

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Relevant files =="
fd -t f . core/src/main integration-tests/src/test | rg 'DefaultDriverConfigReporter|DriverConfigReporting|SslEngineFactory|ProtocolInitHandler'

echo
echo "== Updated stable-session test and config reporting calls =="
rg -n -C 5 'should_use_a_stable_session_id_across_connections|populateStartupOptions\s*\(' \
  core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java \
  core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java \
  core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java \
  core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java

echo
echo "== Shared integration assertion helper and callers =="
rg -n -C 4 'class DriverConfigReportingAssertions|assertDriverConfigPayload\s*\(' \
  integration-tests/src/test/java/com/datastax/oss/driver/core/config

echo
echo "== TLS hostname-verification reporting and SSL factory contract =="
rg -n -C 5 'hostname-verification|requireHostnameValidation\s*\(' \
  core/src/main/java core/src/test/java

Length of output: 40821

---

@nikagra Thanks for the detailed dispositions. I verified the amended changes:

• D: should_use_a_stable_session_id_across_connections now uses reportDriverConfig=true for the control connection, so the report-generation path is exercised.
• E: DriverConfigReportingAssertions.assertDriverConfigPayload is shared package-private code and is called by both CCM and Simulacron integration tests.
• B/C: Agreed—given the two well-documented flags and the distinct sharding-info types, neither abstraction adds enough value at present.
• TLS: The correction is right: tls.hostname-verification now reflects SslEngineFactory.requireHostnameValidation(). The amended tests cover both a validating factory and a programmatic factory whose real state differs from the config option.

---

✏️ Learnings added

Learnt from: nikagra
URL: https://github.com/scylladb/java-driver/pull/968

Timestamp: 2026-07-30T13:44:06.515Z
Learning: In `core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java`, `tls.hostname-verification` must describe the effective `SslEngineFactory.requireHostnameValidation()` state, not `advanced.ssl-engine-factory.hostname-validation`, because the latter only configures the built-in `DefaultSslEngineFactory` and does not describe factories supplied programmatically (for example with `SessionBuilder.withSslContext(...)`).

---

Learnt from: nikagra
URL: https://github.com/scylladb/java-driver/pull/968

Timestamp: 2026-07-30T13:44:06.515Z
Learning: The ScyllaDB detection null checks in `core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java` and `core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java` intentionally remain local: they traverse distinct APIs and types (`ProtocolFeatureStore.getShardingInfo()` returning `ShardingInfo.ConnectionShardingInfo` versus `DriverChannel.getShardingInfo()` returning `ShardingInfo`), so a shared predicate would not centralize meaningful behavior.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

_{You are interacting with an AI system.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java (1)

129-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Prove that the reporting row is the control connection.

This verifies cardinality, not identity: a regression that emits DRIVER_CONFIG on one pool connection and omits it on the control connection still passes. Add a reliable control-connection marker/correlation before validating the payload, as the Simulacron test already does.

🤖 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
`@integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java`
around lines 129 - 138, Update the reporting-row assertions in
DriverConfigReportingCcmIT to correlate the DRIVER_CONFIG row with the control
connection using the same reliable marker or correlation mechanism as the
existing Simulacron test. Validate that the identified row is the control
connection before calling assertDriverConfigPayload, while preserving the
existing single-row cardinality check.

core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java (1)

321-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider single-argument override helpers to cut the reporterWith(...) boilerplate.

The 7-arg reporterWith(defaults(map -> {}), mock(ExponentialReconnectionPolicy.class), mock(DefaultRetryPolicy.class), mock(NoSpeculativeExecutionPolicy.class), mock(DefaultLoadBalancingPolicy.class), mock(TimestampGenerator.class), Optional.empty()) call is repeated ~15 times across this file, varying in exactly one argument. Thin wrappers (or a small builder) would make each test's intent obvious.

♻️ Sketch

private DefaultDriverConfigReporter reporterWithReconnection(ReconnectionPolicy p) {
  return reporterWith(
      defaults(map -> {}),
      p,
      mock(DefaultRetryPolicy.class),
      mock(NoSpeculativeExecutionPolicy.class),
      mock(DefaultLoadBalancingPolicy.class),
      mock(TimestampGenerator.class),
      Optional.empty());
}
// likewise reporterWithRetry / reporterWithSpecEx / reporterWithLb / reporterWithSsl

🤖 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
`@core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java`
around lines 321 - 352, Reduce repeated seven-argument setup in
DefaultDriverConfigReporterTest by adding thin single-argument reporterWith
helper methods for the varying policy/configuration dependencies, including
reconnection policy and the analogous retry, speculative execution,
load-balancing, and SSL cases. Update the affected tests, such as
should_report_constant_reconnection_policy and
should_report_custom_reconnection_policy, to use the appropriate helper while
preserving their existing mocks and assertions.

🤖 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 `@core/src/test/resources/config/driver-config-report-v1.schema.json`:
- Around line 779-801: Update the consistency enum in the schema near the
consistency and serial-consistency properties to accept SERIAL and LOCAL_SERIAL
alongside the existing request consistency values. Keep the serial-consistency
property unchanged.

In
`@integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingAssertions.java`:
- Around line 29-43: Configure the shared OBJECT_MAPPER used by
assertDriverConfigPayload to enable
DeserializationFeature.FAIL_ON_TRAILING_TOKENS, ensuring readTree rejects valid
JSON followed by extra tokens while preserving the existing payload assertions.

---

Nitpick comments:
In
`@core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java`:
- Around line 321-352: Reduce repeated seven-argument setup in
DefaultDriverConfigReporterTest by adding thin single-argument reporterWith
helper methods for the varying policy/configuration dependencies, including
reconnection policy and the analogous retry, speculative execution,
load-balancing, and SSL cases. Update the affected tests, such as
should_report_constant_reconnection_policy and
should_report_custom_reconnection_policy, to use the appropriate helper while
preserving their existing mocks and assertions.

In
`@integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java`:
- Around line 129-138: Update the reporting-row assertions in
DriverConfigReportingCcmIT to correlate the DRIVER_CONFIG row with the control
connection using the same reliable marker or correlation mechanism as the
existing Simulacron test. Validate that the identified row is the control
connection before calling assertDriverConfigPayload, while preserving the
existing single-row cardinality check.

🪄 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: 74f644f4-90dd-478c-b6ac-fecd6b72bac0

📥 Commits

Reviewing files that changed from the base of the PR and between 24062e9 and d6ac737.

📒 Files selected for processing (20)

• core/pom.xml
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/ProgrammaticSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/SslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java
• core/src/test/resources/config/driver-config-report-v1.schema.json
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingAssertions.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java
• pom.xml

🚧 Files skipped from review as they are similar to previous changes (4)

• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/resources/reference.conf
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

[coderabbitai]

🧹 Nitpick comments (1)

core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java (1)

194-203: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Centralize the ScyllaDB detection check

getShardingInfo() != null is used here and again in CassandraSchemaQueries.shouldApplyUsingTimeout(). A shared helper would keep driver-config reporting and USING TIMEOUT gating aligned if the detection logic changes later.

🤖 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
`@core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java`
around lines 194 - 203, Centralize the ScyllaDB detection currently implemented
by getShardingInfo() != null into a shared helper, then update the
ProtocolInitHandler flow and CassandraSchemaQueries.shouldApplyUsingTimeout() to
use it. Preserve the existing featureStore null handling and ensure both
driver-config reporting and USING TIMEOUT gating rely on the same detection
logic.

🤖 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.

Nitpick comments:
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java`:
- Around line 194-203: Centralize the ScyllaDB detection currently implemented
by getShardingInfo() != null into a shared helper, then update the
ProtocolInitHandler flow and CassandraSchemaQueries.shouldApplyUsingTimeout() to
use it. Preserve the existing featureStore null handling and ensure both
driver-config reporting and USING TIMEOUT gating rely on the same detection
logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: af57b7b6-6e4e-44fd-9609-e06c113aa08b

📥 Commits

Reviewing files that changed from the base of the PR and between d6ac737 and 5e69715.

📒 Files selected for processing (20)

• core/pom.xml
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/ProgrammaticSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/SslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java
• core/src/test/resources/config/driver-config-report-v1.schema.json
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingAssertions.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java
• pom.xml

🚧 Files skipped from review as they are similar to previous changes (4)

• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/resources/reference.conf

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java (1)

930-966: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider a small builder for the reporter fixtures.

The 7-argument and 8-argument reporterWith calls repeat across about twenty tests, and each call varies only one argument. A builder that starts from the default policy set and overrides one collaborator would remove that repetition and make each test state its single variable.

Example shape:

private final class ReporterBuilder {
  private DriverExecutionProfile profile = defaults(map -> {});
  private ReconnectionPolicy reconnection = mock(ExponentialReconnectionPolicy.class);
  // ... remaining collaborators with the same defaults as defaultsReporter()
  ReporterBuilder reconnection(ReconnectionPolicy p) { this.reconnection = p; return this; }
  DefaultDriverConfigReporter build() { /* wire the mock context */ }
}

Each test then reads builder().reconnection(mock(ConstantReconnectionPolicy.class)).build().

🤖 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
`@core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java`
around lines 930 - 966, Refactor the repeated reporter fixture setup around the
overloaded reporterWith methods into a small ReporterBuilder that initializes
the same defaults as defaultsReporter() and exposes fluent overrides for
individual collaborators, including the programmatic local datacenter. Update
the affected tests to build reporters by overriding only the variable under
test, while preserving the existing mock context wiring and behavior.

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java (1)

152-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider package-private visibility for buildJson.

DefaultDriverConfigReporterTest is in the same package, com.datastax.oss.driver.internal.core.context. Package-private visibility therefore supports the test override without adding a subclass extension point that the javadoc must then qualify with thread-safety caveats.

♻️ Proposed change

-  protected String buildJson(boolean scyllaDb) {
+  String buildJson(boolean scyllaDb) {

If a production subclass hook is intended, keep protected and disregard this suggestion.

🤖 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
`@core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java`
at line 152, Change the buildJson method in DefaultDriverConfigReporter from
protected to package-private visibility, allowing
DefaultDriverConfigReporterTest to override it within the same package without
exposing a production subclass extension point.

🤖 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 `@core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java`:
- Line 403: Restore opt-in driver configuration reporting by setting
TypedDriverOption.DRIVER_CONFIG_REPORTING_ENABLED to false in OptionsMap. Update
DriverConfigReportingSimulacronIT at lines 53-60 and 145-160 to assert and
enable reporting explicitly as needed, and update upgrade_guide/README.md lines
40-44 to document that reporting is disabled by default.

In
`@core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java`:
- Around line 23-33: Change driver-config reporting defaults from enabled to
disabled across OptionsMap.fillWithDriverDefaults and
DefaultDriverConfigReporter, then update the corresponding documentation and
tests to reflect false as the default. Preserve explicit opt-in behavior,
ensuring default sessions do not send the DRIVER_CONFIG startup option.

In
`@integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java`:
- Around line 133-142: Update the assertions in DriverConfigReportingCcmIT to
identify the DRIVER_CONFIG row by matching its connection local address and port
against the control connection, using the existing control-connection details
and clientOptions helpers. Preserve the single-row and payload assertions, but
ensure a pooled connection cannot satisfy the test.

In `@upgrade_guide/README.md`:
- Around line 42-44: Update the fenced configuration block in the README to
specify the HOCON language identifier, changing the opening fence to use hocon
while preserving the existing configuration content.

---

Nitpick comments:
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java`:
- Line 152: Change the buildJson method in DefaultDriverConfigReporter from
protected to package-private visibility, allowing
DefaultDriverConfigReporterTest to override it within the same package without
exposing a production subclass extension point.

In
`@core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java`:
- Around line 930-966: Refactor the repeated reporter fixture setup around the
overloaded reporterWith methods into a small ReporterBuilder that initializes
the same defaults as defaultsReporter() and exposes fluent overrides for
individual collaborators, including the programmatic local datacenter. Update
the affected tests to build reporters by overriding only the variable under
test, while preserving the existing mock context wiring and behavior.

🪄 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: e40d8974-45a6-4285-bc5b-2d6d30c334dc

📥 Commits

Reviewing files that changed from the base of the PR and between 5e69715 and cfda714.

📒 Files selected for processing (26)

• core/pom.xml
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/ProgrammaticSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/SslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandler.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverContext.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/DriverConfigReporter.java
• core/src/main/java/com/datastax/oss/driver/internal/core/context/StartupOptionsBuilder.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/dse/driver/internal/core/context/DseStartupOptionsBuilderTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporterTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/context/StartupOptionsBuilderTest.java
• core/src/test/resources/config/driver-config-report-v1.schema.json
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingAssertions.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingCcmIT.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingSimulacronIT.java
• pom.xml
• upgrade_guide/README.md

🚧 Files skipped from review as they are similar to previous changes (13)

• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/ProgrammaticSslEngineFactory.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverConfigReportingAssertions.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/CassandraSchemaQueries.java
• core/pom.xml
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/api/core/ssl/SslEngineFactory.java
• core/src/test/resources/config/driver-config-report-v1.schema.json
• pom.xml
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java

[nikagra]

Dispositions for the two nitpicks:

• buildJson → package-private: done. The test override lives in the same package, so nothing else was needed, and the thread-safety javadoc dropped its production-subclass caveat accordingly.
• Builder for the reporter fixtures: skipping, matching the "⚖️ Poor tradeoff" label — the explicit reporterWith(...) calls keep each test's full collaborator set visible at the call site, which is worth more here than the deduplication.

[nikagra]

On the two nitpicks from the last CodeRabbit pass: buildJson is package-private now (312b5226b7) — the only override is the test one, in the same package, so no subclass hook is exposed. The ReporterBuilder fixture is deliberately skipped: test-only readability across ~25 call sites, not worth rewriting the suite at this point in review.

[Copilot]

Pull request overview

Copilot reviewed 34 out of 35 changed files in this pull request and generated no new comments.

Suppressed comments (4)

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java:690

• The report can invent a dc-auto preference for an arbitrary custom load-balancing policy. With no configured DC/rack and a policy that is not a BasicLoadBalancingPolicy, this method returns a NodeLocation with all-null fields, which both serializers turn into dc-auto; however, the custom-policy SPI does not require DC inference and a custom policy may remain DC-agnostic. Omit the preference when neither configured nor resolved location evidence exists, except for built-ins known to infer one, and add coverage for a DC-agnostic custom policy.

    // Exact-class check, like the policy branches above: a user subclass may well discover a local
    // DC of its own, so only the built-in itself is known to stay datacenter-agnostic.
    if (configuredDc == null && policy.getClass() == BasicLoadBalancingPolicy.class) {
      return null;

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java:899

• Trimming changes the value that the running policy actually uses. OptionalLocalDcHelper and OptionalLocalRackHelper preserve configured strings verbatim and compare them exactly, so a configured " dc1 " does not match dc1, while this report claims that it does. Preserve non-empty DC/rack strings exactly (and handle only the truly empty value by omission or explicit schema-gap behavior) so diagnostics do not fabricate an effective preference.

  /** The string with surrounding whitespace removed, or {@code null} if that leaves it empty. */
  @Nullable
  private static String trimmedToNull(@Nullable String s) {
    if (s == null) {
      return null;
    }
    String trimmed = s.trim();
    return trimmed.isEmpty() ? null : trimmed;

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java:69

• This acknowledged staleness contradicts the report's purpose of describing effective configuration. These load-balancing and speculative-execution options are non-modifiable at runtime and are latched into final policy fields, but the reporter reads the reloaded profile, so after a reload it can advertise behavior the running policies do not implement. Expose and read the latched values, as reconnectionPolicy() already does, instead of knowingly emitting inaccurate diagnostics.

This issue also appears in the following locations of the same file:

• line 687
• line 892

 * The same staleness applies elsewhere — {@code DefaultLoadBalancingPolicy}'s slow-avoidance flag
 * behind {@code adaptive-ordering}, {@code BasicLoadBalancingPolicy}'s max-nodes-per-remote-dc
 * behind {@code fallback-to-non-preferred-nodes}, the speculative-execution delays — but those
 * policies expose no accessor for the latched value, so reading them would need new API. There,
 * immediately after a reload, the report can still show a value the already-running policy doesn't
 * reflect yet, until that policy is rebuilt.

core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java:343

• This comment is stale: the reporter now uses only the presence of sharding info as the ScyllaDB signal and no longer derives pool sizing from the shard count. Describe the count assertion as verification that ConnectionShardingInfo was unwrapped correctly.

    // The reporter receives the node-level info unwrapped from ConnectionShardingInfo, carrying the
    // SCYLLA_NR_SHARDS count above — which is what it reports the connection pool's sizing from.
    assertThat(capturedShardingInfo.get()).isNotNull();
    assertThat(capturedShardingInfo.get().getShardsCount()).isEqualTo(4);

[Copilot]

Pull request overview

Copilot reviewed 34 out of 35 changed files in this pull request and generated no new comments.

Suppressed comments (4)

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java:940

• trimmedToNull changes the reported DC/rack even though the load-balancing helpers use the configured string verbatim (OptionalLocalDcHelper.java:71-79 and OptionalLocalRackHelper.java:34-38). For example, " dc1 " matches no dc1 nodes at runtime but is reported as "dc1"; an empty/whitespace value can also be turned into dc-auto even though the policy treats it as explicitly configured and does not infer. Preserve non-empty values exactly, and omit an unrepresentable empty value without treating it as absent for inference.

    String trimmed = s.trim();

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java:626

• This reports true from the raw max-remote-nodes option even when BasicLoadBalancingPolicy has no local DC. In that mode maybeAddDcFailover returns immediately and all nodes are treated as local (BasicLoadBalancingPolicy.java:577-578, 724-726), so the option is inert and both node-preference groups are omitted, yet the report claims fallback to non-preferred nodes is active. Gate this value on an effective node preference/local DC and cover the DC-agnostic Basic policy with a positive failover setting.

    n.put(
        "fallback-to-non-preferred-nodes",
        config.getInt(DefaultDriverOption.LOAD_BALANCING_DC_FAILOVER_MAX_NODES_PER_REMOTE_DC, 0)
            > 0);

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverConfigReporter.java:333

• The class documentation now correctly lists two required fields with no schema-valid form (in-flight.max and query.defaults.consistency), but this comment still says there is only one. Keep the count consistent.

    // clamped — the one field left with no schema-valid form, see the class javadoc.

core/src/test/java/com/datastax/oss/driver/internal/core/channel/ProtocolInitHandlerTest.java:341

• This comment is stale: the reporter intentionally reads only whether sharding info is present and no longer reports pool sizing or consumes the shard count. Update it to describe the unwrapping/backend signal being tested.

    // The reporter receives the node-level info unwrapped from ConnectionShardingInfo, carrying the
    // SCYLLA_NR_SHARDS count above — which is what it reports the connection pool's sizing from.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.