Report SESSION_ID and the driver configuration in the STARTUP options - stage 1 - #262
Conversation
📝 WalkthroughWalkthroughThe driver adds configurable driver configuration reporting. It sends compact, size-limited JSON on control-connection startup options. Startup options include a per-factory Sequence Diagram(s)sequenceDiagram
participant Builder
participant Configuration
participant ConnectionFactory
participant StartupRequestFactory
participant StartupOptionsFactory
participant DriverConfigReporter
Builder->>Configuration: configure driver reporting
Configuration->>DriverConfigReporter: create reporter
ConnectionFactory->>StartupRequestFactory: create control or non-control connection
StartupRequestFactory->>StartupOptionsFactory: create startup options
StartupOptionsFactory->>StartupOptionsFactory: add SESSION_ID
StartupOptionsFactory->>DriverConfigReporter: report DRIVER_CONFIG for control connections
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds client identification and configuration reporting to CQL STARTUP options.
Changes:
- Adds a cluster-wide
SESSION_ID. - Reports versioned
DRIVER_CONFIGJSON on control connections. - Adds configuration controls and unit coverage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Cassandra/Requests/StartupRequestFactory.cs |
Tracks control-connection status. |
src/Cassandra/Requests/StartupOptionsFactory.cs |
Adds session ID and configuration options. |
src/Cassandra/Requests/IStartupOptionsFactory.cs |
Extends the startup-options contract. |
src/Cassandra/Requests/IDriverConfigReporter.cs |
Defines configuration reporting. |
src/Cassandra/Requests/DriverConfigReporter.cs |
Builds bounded JSON reports. |
src/Cassandra/Connections/IConnectionFactory.cs |
Documents control-connection reporting. |
src/Cassandra/Connections/ConnectionFactory.cs |
Distinguishes control and pooled connections. |
src/Cassandra/Configuration.cs |
Configures reporting and its default. |
src/Cassandra/Builder.cs |
Adds the public reporting toggle. |
src/Cassandra.Tests/TestConfigurationBuilder.cs |
Updates test configuration construction. |
src/Cassandra.Tests/Requests/StartupOptionsFactoryTests.cs |
Tests session and configuration options. |
src/Cassandra.Tests/Requests/DriverConfigReporterTests.cs |
Tests reporting behavior. |
src/Cassandra.Tests/HostConnectionPoolTests.cs |
Updates startup factory construction. |
src/Cassandra.Tests/ConnectionTests.cs |
Updates connection tests and formatting. |
src/Cassandra.IntegrationTests/Core/ConnectionTests.cs |
Updates integration connection construction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Send an identifier that is shared by every connection of a Cluster instance, so that the server can correlate them with each other. It is generated by the driver rather than derived from the CLIENT_ID option, which reports the application supplied Configuration.ClusterId and is therefore not guaranteed to be unique.
9861a60 to
3f19cb7
Compare
![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 `@src/Cassandra.Tests/TestConfigurationBuilder.cs`:
- Around line 56-60: Change the StartupOptionsFactory property in
TestConfigurationBuilder to default to null instead of constructing a
StartupOptionsFactory with DriverConfigReporter(true). Preserve explicitly
assigned test factories so Configuration can create the factory using
DriverConfigReportingEnabled when none is provided.
🪄 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: 84be9777-f42c-45b5-b2a9-c2258351e398
📒 Files selected for processing (16)
src/Cassandra.IntegrationTests/Core/ConnectionTests.cssrc/Cassandra.Tests/BuilderTests.cssrc/Cassandra.Tests/ConnectionTests.cssrc/Cassandra.Tests/HostConnectionPoolTests.cssrc/Cassandra.Tests/Requests/DriverConfigReporterTests.cssrc/Cassandra.Tests/Requests/StartupOptionsFactoryTests.cssrc/Cassandra.Tests/TestConfigurationBuilder.cssrc/Cassandra/Builder.cssrc/Cassandra/Configuration.cssrc/Cassandra/Connections/ConnectionFactory.cssrc/Cassandra/Connections/IConnectionFactory.cssrc/Cassandra/Requests/DriverConfigReporter.cssrc/Cassandra/Requests/IDriverConfigReporter.cssrc/Cassandra/Requests/IStartupOptionsFactory.cssrc/Cassandra/Requests/StartupOptionsFactory.cssrc/Cassandra/Requests/StartupRequestFactory.cs
3f19cb7 to
caeadf2
Compare
nikagra
left a comment
nikagra
left a comment
There was a problem hiding this comment.
Reviewed against the sibling implementations — gocql#964, java-driver#967 (merged) and java-driver#968 (stage 2). The design is sound, CI is green, and every wire-level choice here agrees with where the other drivers converged. Inline comments cover the rest; two findings have no line to anchor to.
Parity check
| C# #262 | gocql#964 | java #967→#968 | |
|---|---|---|---|
SESSION_ID on every connection, ungated |
✅ | ✅ | ✅ (moved out of the reporter in #968) |
SESSION_ID beside CLIENT_ID, not in the reporter |
✅ | — | ✅ (#968) |
DRIVER_CONFIG on the control connection only |
✅ | ✅ | ✅ |
| Ships enabled | ✅ | ✅ | ✅ (flipped in #968) |
| Fail-safe when the report can't be built | ✅ | ✅ | ✅ |
| Size cap | ✅ 32 KiB | ✅ 32 KiB | ❌ none |
| Integration tests | ❌ none | ✅ | ✅ |
Noting explicitly so it doesn't get re-litigated: default-on and ungated SESSION_ID are correct as written. java#967 originally shipped disabled and gated both keys, but #968 flips enabled = true per @dkropachev's cross-driver review and moves SESSION_ID_KEY into StartupOptionsBuilder.build() next to CLIENT_ID_KEY, explicitly not governed by the flag. Putting SESSION_ID in StartupOptionsFactory alongside ClientIdOption is where java ended up too — this PR got there first.
The prerequisite #967 recorded for enabling by default ("confirm ScyllaDB tolerates unknown STARTUP keys and has no problematic client_options value-length cap") is discharged in #968: CCM runs against ScyllaDB 2026.1.9 and Cassandra 4.1.10, payload round-tripping through both system.clients and system_views.clients with no truncation.
1. No integration test
Both siblings ship one; this is the only driver without. The whole point is server-side visibility, but nothing asserts the keys reach client_options, and nothing asserts the handshake still succeeds now that two new keys go out by default. Two portable designs:
- gocql shape (
TestDriverConfigReporting) — querysystem_views.clients/system.clients, poll (the table is populated asynchronously as connections finish setting up), assertDRIVER_CONFIG == {"version":1}. - java shape (
DriverConfigReportingSimulacronIT) — pin the wire contract with Simulacron. The infrastructure is already here:SimulacronBase.GetLogs(),RequestLog.FrameandQueryType.Startupall exist undersrc/Cassandra.IntegrationTests/SimulacronAPI/Models/Logs/. It needs only aStartupMessagemodel and aFrame.GetStartupMessage()wrapper beside the existingGetQueryMessage()/GetBatchMessage().
Two details from java's ITs worth copying either way:
- Identify the control connection independently — by its
REGISTERframe, or by matching address+port against the control channel — so "control connection only" can't be satisfied by a pooled connection that happened to be first. - Assert payload content, not key presence. java uses a shared
DriverConfigReportingAssertions(valid JSON,version == 1as an integer, plus an always-present group) precisely so an empty or stage-1-only payload can't slip through a key-presence check once stage 2 lands.
2. No upgrade-guide entry
docs/source/upgrade-guide/index.md exists and gets no update here. java#968 added a ### 4.19.2.1 section covering both startup options and how to switch the report off. Worth matching — this is operator-facing and ships enabled, so "what are these two new keys in client_options, and how do I stop sending them" is a question users will actually have.
Describe the effective driver configuration to the cluster as the DRIVER_CONFIG startup option, so that operators can inspect the settings of a client while investigating an incident. ScyllaDB exposes it in the client_options column of its clients table. The configuration is the same for every connection, so only the control connection reports it. Connections learn whether they are the control connection from the StartupRequestFactory the ConnectionFactory builds for them, which keeps the flag out of Connection itself. The report only holds the schema version for now, the configuration groups follow. Building it is fail safe and its size is capped, because a diagnostic aid must never keep a connection from being established. Reporting can be turned off with Builder.WithDriverConfigReporting.
…tions Verify over an actual connection to Simulacron that: - Only the control connection reports DRIVER_CONFIG, with the expected schema version, and that Builder.WithDriverConfigReporting(false) suppresses it. - Every connection of a Cluster instance reports the same SESSION_ID, and distinct Cluster instances report distinct ones. Adds Frame.GetStartupMessage() and the backing StartupMessage model to the Simulacron log helpers, following the existing GetQueryMessage/ GetBatchMessage pattern, so STARTUP frames can be inspected like other request types.
caeadf2 to
aa8310f
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 `@src/Cassandra/Requests/DriverConfigReporter.cs`:
- Around line 101-106: Implement PopulateConfig in DriverConfigReporter to
serialize the effective driver configuration groups into the supplied JObject
report, rather than leaving DRIVER_CONFIG with only its schema version. Reuse
the existing driver configuration values and serialization conventions, and
preserve the protected virtual extension point so subclasses can add further
groups.
🪄 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: QUIET
Plan: Pro Plus
Run ID: 0880d022-9468-4a2e-9f6a-fe985511dc1c
📒 Files selected for processing (19)
src/Cassandra.IntegrationTests/Core/ConnectionTests.cssrc/Cassandra.IntegrationTests/Core/StartupOptionsTests.cssrc/Cassandra.IntegrationTests/SimulacronAPI/Models/Logs/Frame.cssrc/Cassandra.IntegrationTests/SimulacronAPI/Models/Logs/StartupMessage.cssrc/Cassandra.Tests/BuilderTests.cssrc/Cassandra.Tests/ConnectionTests.cssrc/Cassandra.Tests/HostConnectionPoolTests.cssrc/Cassandra.Tests/Requests/DriverConfigReporterTests.cssrc/Cassandra.Tests/Requests/StartupOptionsFactoryTests.cssrc/Cassandra.Tests/TestConfigurationBuilder.cssrc/Cassandra/Builder.cssrc/Cassandra/Configuration.cssrc/Cassandra/Connections/ConnectionFactory.cssrc/Cassandra/Connections/IConnectionFactory.cssrc/Cassandra/Requests/DriverConfigReporter.cssrc/Cassandra/Requests/IDriverConfigReporter.cssrc/Cassandra/Requests/IStartupOptionsFactory.cssrc/Cassandra/Requests/StartupOptionsFactory.cssrc/Cassandra/Requests/StartupRequestFactory.cs
Two additions to the CQL STARTUP options the driver sends, both aimed at making a client easier to identify and inspect from the server side. ScyllaDB exposes startup options in the client_options column of its system.clients table.
Report SESSION_ID in the STARTUP options
Sends an identifier shared by every connection of a Cluster instance, so the server can correlate connections belonging to the same client.
It is generated by the driver rather than derived from the existing CLIENT_ID option, which reports the application supplied Configuration.ClusterId and is therefore not guaranteed to be unique.
Report the driver configuration on the control connection
Describes the effective driver configuration to the cluster as the DRIVER_CONFIG startup option, a JSON document, so that operators can inspect the settings of a client while investigating an incident.
Refs: https://scylladb.atlassian.net/browse/DRIVER-383