Skip to content

CASSJAVA-92: Local DC provided for nodetool clientstats #2036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: 4.x
Choose a base branch
from
+284 −20
Open

CASSJAVA-92: Local DC provided for nodetool clientstats #2036

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1afcf84
CASSJAVA-92: Local DC provided for nodetool clientstats
lukasz-antoniak Apr 14, 2025
101621a
Lookup data center from LBP
lukasz-antoniak Apr 17, 2025
3829053
Return local DC for all profiles
lukasz-antoniak May 22, 2025
9b11d53
Apply review comments
lukasz-antoniak Jun 4, 2025
3ce91e3
Refactor local DC to driver baggage
lukasz-antoniak Jul 17, 2025
95aa698
Improve JSON serialization
lukasz-antoniak Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions core/src/main/java/com/datastax/oss/driver/internal/core/context/StartupOptionsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package com.datastax.oss.driver.internal.core.context;

import com.datastax.dse.driver.api.core.config.DseDriverOption;
import com.datastax.oss.driver.api.core.config.DefaultDriverOption;
import com.datastax.oss.driver.api.core.config.DriverExecutionProfile;
import com.datastax.oss.driver.api.core.session.Session;
import com.datastax.oss.driver.api.core.uuid.Uuids;
Expand All @@ -33,6 +34,7 @@ public class StartupOptionsBuilder {

public static final String DRIVER_NAME_KEY = "DRIVER_NAME";
public static final String DRIVER_VERSION_KEY = "DRIVER_VERSION";
public static final String DRIVER_LOCAL_DC = "DRIVER_LOCAL_DC";
public static final String APPLICATION_NAME_KEY = "APPLICATION_NAME";
public static final String APPLICATION_VERSION_KEY = "APPLICATION_VERSION";
public static final String CLIENT_ID_KEY = "CLIENT_ID";
Expand All @@ -41,6 +43,7 @@ public class StartupOptionsBuilder {
private UUID clientId;
private String applicationName;
private String applicationVersion;
private String applicationLocalDc;

public StartupOptionsBuilder(InternalDriverContext context) {
this.context = context;
Expand Down Expand Up @@ -119,6 +122,12 @@ public Map<String, String> build() {
if (applicationVersion != null) {
builder.put(APPLICATION_VERSION_KEY, applicationVersion);
}
if (applicationLocalDc == null) {
applicationLocalDc = localDc(config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config here is just the default profile but any profile can define a local datacenter... which in turn means we need to think about how execution profiles interact with this functionality.

I'm going to raise this point on the JIRA ticket; I think it warrants further discussion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raised

}
if (applicationLocalDc != null) {
builder.put(DRIVER_LOCAL_DC, applicationLocalDc);
}

return builder.build();
}
Expand All @@ -142,4 +151,14 @@ protected String getDriverName() {
protected String getDriverVersion() {
return Session.OSS_DRIVER_COORDINATES.getVersion().toString();
}

private String localDc(DriverExecutionProfile profile) {
String dc = context.getLocalDatacenter(profile.getName()); // DC set programmatically
if (dc == null && profile.isDefined(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER)) {
dc =
profile.getString(
DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER); // DC from configuration
}
return dc;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultDriverContext already defines lazy instantiation for (and access to) the startup options map for a given run. Rather than splitting the logic for determining the contents of a STARTUP message between DefaultDriverContext and this class the majority of the logic in this class should be consolidated into the existing DefaultDriverContext methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have noticed that and though that other entries of the STARTUP options are added here. The justification of the logic would be that all "dedicated" properties for STARTUP message are lazily instantiated where you pointed out, whereas all properties taken from other components (e.g. compression) are automatically injected in build() method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, as I look at this again you're right, there's something of a bifurcation here already. The entries returned by DefaultDriverContext.buildStartupOptions() are more static key/value pairs, mostly (exclusively?) pairs that were used by Insights. Nearly all of those should be removed as part of CASSJAVA-73; driver name and version will stay but the rest should disappear.

So how should we format this data? That question is still under discussion in CASSJAVA-92... we probably need to settle on what the data should look like and then adjust this impl accordingly.

}
52 changes: 32 additions & 20 deletions ...test/java/com/datastax/dse/driver/internal/core/context/DseStartupOptionsBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,17 @@ public void before() {
when(defaultProfile.isDefined(DseDriverOption.CONTINUOUS_PAGING_PAGE_SIZE)).thenReturn(true);
}

private void buildContext(UUID clientId, String applicationName, String applicationVersion) {
this.driverContext =
new DefaultDriverContext(
configLoader,
ProgrammaticArguments.builder()
.withStartupClientId(clientId)
.withStartupApplicationName(applicationName)
.withStartupApplicationVersion(applicationVersion)
.build());
private void buildContext(
UUID clientId, String applicationName, String applicationVersion, String localDc) {
ProgrammaticArguments.Builder builder =
ProgrammaticArguments.builder()
.withStartupClientId(clientId)
.withStartupApplicationName(applicationName)
.withStartupApplicationVersion(applicationVersion);
if (localDc != null) {
builder.withLocalDatacenter(DriverExecutionProfile.DEFAULT_NAME, localDc);
}
this.driverContext = new DefaultDriverContext(configLoader, builder.build());
}

private void assertDefaultStartupOptions(Startup startup) {
Expand All @@ -86,7 +88,7 @@ private void assertDefaultStartupOptions(Startup startup) {
public void should_build_startup_options_with_no_compression_if_undefined() {
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("none");
buildContext(null, null, null);
buildContext(null, null, null, null);
Startup startup = new Startup(driverContext.getStartupOptions());
assertThat(startup.options).doesNotContainKey(Startup.COMPRESSION_KEY);
assertDefaultStartupOptions(startup);
Expand All @@ -97,7 +99,7 @@ public void should_build_startup_options_with_no_compression_if_undefined() {
public void should_build_startup_options_with_compression(String compression) {
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn(compression);
buildContext(null, null, null);
buildContext(null, null, null, null);
Startup startup = new Startup(driverContext.getStartupOptions());
// assert the compression option is present
assertThat(startup.options).containsEntry(Startup.COMPRESSION_KEY, compression);
Expand All @@ -110,7 +112,7 @@ public void should_build_startup_options_with_compression(String compression) {
public void should_fail_to_build_startup_options_with_invalid_compression() {
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("foobar");
buildContext(null, null, null);
buildContext(null, null, null, null);
assertThatIllegalArgumentException()
.isThrownBy(() -> new Startup(driverContext.getStartupOptions()));
}
Expand All @@ -120,7 +122,7 @@ public void should_build_startup_options_with_client_id() {
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("none");
UUID customClientId = Uuids.random();
buildContext(customClientId, null, null);
buildContext(customClientId, null, null, null);
Startup startup = new Startup(driverContext.getStartupOptions());
// assert the client id is present
assertThat(startup.options)
Expand All @@ -135,7 +137,7 @@ public void should_build_startup_options_with_client_id() {
public void should_build_startup_options_with_application_version_and_name() {
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("none");
buildContext(null, "Custom_App_Name", "Custom_App_Version");
buildContext(null, "Custom_App_Name", "Custom_App_Version", null);
Startup startup = new Startup(driverContext.getStartupOptions());
// assert the app name and version are present
assertThat(startup.options)
Expand All @@ -151,15 +153,17 @@ public void should_build_startup_options_with_all_options() {
// mock config to specify "snappy" compression
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("snappy");
when(defaultProfile.getName()).thenReturn(DriverExecutionProfile.DEFAULT_NAME);

UUID customClientId = Uuids.random();

buildContext(customClientId, "Custom_App_Name", "Custom_App_Version");
buildContext(customClientId, "Custom_App_Name", "Custom_App_Version", "dc6");
Startup startup = new Startup(driverContext.getStartupOptions());
assertThat(startup.options)
.containsEntry(StartupOptionsBuilder.CLIENT_ID_KEY, customClientId.toString())
.containsEntry(StartupOptionsBuilder.APPLICATION_NAME_KEY, "Custom_App_Name")
.containsEntry(StartupOptionsBuilder.APPLICATION_VERSION_KEY, "Custom_App_Version");
.containsEntry(StartupOptionsBuilder.APPLICATION_VERSION_KEY, "Custom_App_Version")
.containsEntry(StartupOptionsBuilder.DRIVER_LOCAL_DC, "dc6");
assertThat(startup.options).containsEntry(Startup.COMPRESSION_KEY, "snappy");
assertDefaultStartupOptions(startup);
}
Expand All @@ -172,25 +176,33 @@ public void should_use_configuration_when_no_programmatic_values_provided() {
.thenReturn("Config_App_Version");
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("none");
when(defaultProfile.getString(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER))
.thenReturn("Config_DC_1");
when(defaultProfile.isDefined(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER))
.thenReturn(true);
when(defaultProfile.getName()).thenReturn(DriverExecutionProfile.DEFAULT_NAME);

buildContext(null, null, null);
buildContext(null, null, null, null);
Startup startup = new Startup(driverContext.getStartupOptions());

assertThat(startup.options)
.containsEntry(StartupOptionsBuilder.APPLICATION_NAME_KEY, "Config_App_Name")
.containsEntry(StartupOptionsBuilder.APPLICATION_VERSION_KEY, "Config_App_Version");
.containsEntry(StartupOptionsBuilder.APPLICATION_VERSION_KEY, "Config_App_Version")
.containsEntry(StartupOptionsBuilder.DRIVER_LOCAL_DC, "Config_DC_1");
}

@Test
public void should_ignore_configuration_when_programmatic_values_provided() {
when(defaultProfile.getString(DefaultDriverOption.PROTOCOL_COMPRESSION, "none"))
.thenReturn("none");
when(defaultProfile.getName()).thenReturn(DriverExecutionProfile.DEFAULT_NAME);

buildContext(null, "Custom_App_Name", "Custom_App_Version");
buildContext(null, "Custom_App_Name", "Custom_App_Version", "us-west-2");
Startup startup = new Startup(driverContext.getStartupOptions());

assertThat(startup.options)
.containsEntry(StartupOptionsBuilder.APPLICATION_NAME_KEY, "Custom_App_Name")
.containsEntry(StartupOptionsBuilder.APPLICATION_VERSION_KEY, "Custom_App_Version");
.containsEntry(StartupOptionsBuilder.APPLICATION_VERSION_KEY, "Custom_App_Version")
.containsEntry(StartupOptionsBuilder.DRIVER_LOCAL_DC, "us-west-2");
}
}