Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion google-cloud-spanner/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@
<spanner.testenv.config.class>com.google.cloud.spanner.GceTestEnvConfig</spanner.testenv.config.class>
<spanner.testenv.instance>projects/directpath-prod-manual-testing/instances/spanner-testing</spanner.testenv.instance>
<spanner.gce.config.project_id>directpath-prod-manual-testing</spanner.gce.config.project_id>
<spanner.attempt_directpath>true</spanner.attempt_directpath>
<spanner.enable_direct_access>true</spanner.enable_direct_access>
<spanner.directpath_test_scenario>ipv4</spanner.directpath_test_scenario>
</systemPropertyVariables>
<forkedProcessTimeoutInSeconds>3000</forkedProcessTimeoutInSeconds>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public class SpannerOptions extends ServiceOptions<Spanner, SpannerOptions> {
private final CloseableExecutorProvider asyncExecutorProvider;
private final String compressorName;
private final boolean leaderAwareRoutingEnabled;
private final boolean attemptDirectPath;
private final boolean enableDirectAccess;
private final DirectedReadOptions directedReadOptions;
private final boolean useVirtualThreads;
private final OpenTelemetry openTelemetry;
Expand Down Expand Up @@ -806,7 +806,7 @@ protected SpannerOptions(Builder builder) {
asyncExecutorProvider = builder.asyncExecutorProvider;
compressorName = builder.compressorName;
leaderAwareRoutingEnabled = builder.leaderAwareRoutingEnabled;
attemptDirectPath = builder.attemptDirectPath;
enableDirectAccess = builder.enableDirectAccess;
directedReadOptions = builder.directedReadOptions;
useVirtualThreads = builder.useVirtualThreads;
openTelemetry = builder.openTelemetry;
Expand Down Expand Up @@ -849,6 +849,10 @@ default boolean isEnableApiTracing() {
return false;
}

default boolean isEnableDirectAccess() {
return false;
}

default boolean isEnableBuiltInMetrics() {
return true;
}
Expand Down Expand Up @@ -884,6 +888,8 @@ private static class SpannerEnvironmentImpl implements SpannerEnvironment {
"SPANNER_OPTIMIZER_STATISTICS_PACKAGE";
private static final String SPANNER_ENABLE_EXTENDED_TRACING = "SPANNER_ENABLE_EXTENDED_TRACING";
private static final String SPANNER_ENABLE_API_TRACING = "SPANNER_ENABLE_API_TRACING";
private static final String GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS =
"GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS";
private static final String SPANNER_ENABLE_END_TO_END_TRACING =
"SPANNER_ENABLE_END_TO_END_TRACING";
private static final String SPANNER_DISABLE_BUILTIN_METRICS = "SPANNER_DISABLE_BUILTIN_METRICS";
Expand Down Expand Up @@ -916,6 +922,11 @@ public boolean isEnableApiTracing() {
return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_API_TRACING));
}

@Override
public boolean isEnableDirectAccess() {
return Boolean.parseBoolean(System.getenv(GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS));
}

@Override
public boolean isEnableBuiltInMetrics() {
return !Boolean.parseBoolean(System.getenv(SPANNER_DISABLE_BUILTIN_METRICS));
Expand Down Expand Up @@ -1000,7 +1011,7 @@ public static class Builder
private String compressorName;
private String emulatorHost = System.getenv("SPANNER_EMULATOR_HOST");
private boolean leaderAwareRoutingEnabled = true;
private boolean attemptDirectPath = false;
private boolean enableDirectAccess = SpannerOptions.environment.isEnableDirectAccess();
private DirectedReadOptions directedReadOptions;
private boolean useVirtualThreads = false;
private OpenTelemetry openTelemetry;
Expand Down Expand Up @@ -1072,7 +1083,7 @@ protected Builder() {
this.channelProvider = options.channelProvider;
this.channelConfigurator = options.channelConfigurator;
this.interceptorProvider = options.interceptorProvider;
this.attemptDirectPath = options.attemptDirectPath;
this.enableDirectAccess = options.enableDirectAccess;
this.directedReadOptions = options.directedReadOptions;
this.useVirtualThreads = options.useVirtualThreads;
this.enableApiTracing = options.enableApiTracing;
Expand Down Expand Up @@ -1605,14 +1616,21 @@ public Builder disableLeaderAwareRouting() {
}

@BetaApi
public Builder enableDirectPath() {
this.attemptDirectPath = true;
public Builder enableDirectAccess() {
this.enableDirectAccess = true;
return this;
}

@BetaApi
public Builder disableDirectAccess() {
this.enableDirectAccess = false;
return this;
}

@ObsoleteApi("Use disableDirectAccess() instead")
@Deprecated
public Builder disableDirectPath() {
this.attemptDirectPath = false;
this.enableDirectAccess = false;
return this;
}

Expand Down Expand Up @@ -1980,8 +1998,14 @@ public DirectedReadOptions getDirectedReadOptions() {
}

@BetaApi
public Boolean isEnableDirectAccess() {
return enableDirectAccess;
}

@ObsoleteApi("Use isEnableDirectAccess() instead")
@Deprecated
public boolean isAttemptDirectPath() {
return attemptDirectPath;
return enableDirectAccess;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.spanner.connection;

import static com.google.cloud.spanner.connection.ConnectionProperties.ATTEMPT_DIRECT_PATH;
import static com.google.cloud.spanner.connection.ConnectionProperties.AUTOCOMMIT;
import static com.google.cloud.spanner.connection.ConnectionProperties.AUTO_CONFIG_EMULATOR;
import static com.google.cloud.spanner.connection.ConnectionProperties.AUTO_PARTITION_MODE;
Expand All @@ -29,6 +28,7 @@
import static com.google.cloud.spanner.connection.ConnectionProperties.DATA_BOOST_ENABLED;
import static com.google.cloud.spanner.connection.ConnectionProperties.DIALECT;
import static com.google.cloud.spanner.connection.ConnectionProperties.ENABLE_API_TRACING;
import static com.google.cloud.spanner.connection.ConnectionProperties.ENABLE_DIRECT_ACCESS;
import static com.google.cloud.spanner.connection.ConnectionProperties.ENABLE_END_TO_END_TRACING;
import static com.google.cloud.spanner.connection.ConnectionProperties.ENABLE_EXTENDED_TRACING;
import static com.google.cloud.spanner.connection.ConnectionProperties.ENCODED_CREDENTIALS;
Expand Down Expand Up @@ -1082,8 +1082,8 @@ boolean isExperimentalHost() {
return getInitialConnectionPropertyValue(IS_EXPERIMENTAL_HOST);
}

boolean isAttemptDirectPath() {
return getInitialConnectionPropertyValue(ATTEMPT_DIRECT_PATH);
boolean isEnableDirectAccess() {
return getInitialConnectionPropertyValue(ENABLE_DIRECT_ACCESS);
}

String getClientCertificate() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,9 @@ public class ConnectionProperties {
BOOLEANS,
BooleanConverter.INSTANCE,
Context.STARTUP);
static final ConnectionProperty<Boolean> ATTEMPT_DIRECT_PATH =
static final ConnectionProperty<Boolean> ENABLE_DIRECT_ACCESS =
create(
"attemptDirectPath",
"enableDirectAccess",
"Configure the connection to try to connect to Spanner using "
+ "DirectPath (true/false). The client will try to connect to Spanner "
+ "using a direct Google network connection. DirectPath will work only "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ static class SpannerPoolKey {
private final String clientCertificate;
private final String clientCertificateKey;
private final boolean isExperimentalHost;
private final boolean attemptDirectPath;
private final boolean enableDirectAccess;

@VisibleForTesting
static SpannerPoolKey of(ConnectionOptions options) {
Expand Down Expand Up @@ -199,7 +199,7 @@ private SpannerPoolKey(ConnectionOptions options) throws IOException {
this.clientCertificate = options.getClientCertificate();
this.clientCertificateKey = options.getClientCertificateKey();
this.isExperimentalHost = options.isExperimentalHost();
this.attemptDirectPath = options.isAttemptDirectPath();
this.enableDirectAccess = options.isEnableDirectAccess();
}

@Override
Expand All @@ -226,7 +226,7 @@ public boolean equals(Object o) {
&& Objects.equals(this.clientCertificate, other.clientCertificate)
&& Objects.equals(this.clientCertificateKey, other.clientCertificateKey)
&& Objects.equals(this.isExperimentalHost, other.isExperimentalHost)
&& Objects.equals(this.attemptDirectPath, other.attemptDirectPath);
&& Objects.equals(this.enableDirectAccess, other.enableDirectAccess);
}

@Override
Expand All @@ -249,7 +249,7 @@ public int hashCode() {
this.clientCertificate,
this.clientCertificateKey,
this.isExperimentalHost,
this.attemptDirectPath);
this.enableDirectAccess);
}
}

Expand Down Expand Up @@ -416,8 +416,8 @@ Spanner createSpanner(SpannerPoolKey key, ConnectionOptions options) {
if (key.isExperimentalHost) {
builder.setExperimentalHost(key.host);
}
if (key.attemptDirectPath) {
builder.enableDirectPath();
if (key.enableDirectAccess) {
builder.enableDirectAccess();
}
if (options.getConfigurator() != null) {
options.getConfigurator().configure(builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,8 @@ public GapicSpannerRpc(final SpannerOptions options) {
.withEncoding(compressorName))
.setHeaderProvider(headerProviderWithUserAgent)
.setAllowNonDefaultServiceAccount(true);
boolean isAttemptDirectPathXds = isEnableDirectPathXdsEnv() || options.isAttemptDirectPath();
if (isAttemptDirectPathXds) {
boolean isEnableDirectAccess = options.isEnableDirectAccess();
if (isEnableDirectAccess) {
defaultChannelProviderBuilder.setAttemptDirectPath(true);
// This will let the credentials try to fetch a hard-bound access token if the runtime
// environment supports it.
Expand Down Expand Up @@ -426,7 +426,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
spannerStubSettings, clientContext);
DIRECTPATH_CHANNEL_CREATED =
((GrpcTransportChannel) clientContext.getTransportChannel()).isDirectPath()
&& isAttemptDirectPathXds;
&& isEnableDirectAccess;
this.readRetrySettings =
options.getSpannerStubSettings().streamingReadSettings().getRetrySettings();
this.readRetryableCodes =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class GceTestEnvConfig implements TestEnvConfig {
public static final String GCE_CREDENTIALS_FILE = "spanner.gce.config.credentials_file";
public static final String GCE_STREAM_BROKEN_PROBABILITY =
"spanner.gce.config.stream_broken_probability";
public static final String ATTEMPT_DIRECT_PATH = "spanner.attempt_directpath";
public static final String ENABLE_DIRECT_ACCESS = "spanner.enable_direct_access";
public static final String DIRECT_PATH_TEST_SCENARIO = "spanner.directpath_test_scenario";

// IP address prefixes allocated for DirectPath backends.
Expand All @@ -64,7 +64,7 @@ public GceTestEnvConfig() {
double errorProbability =
Double.parseDouble(System.getProperty(GCE_STREAM_BROKEN_PROBABILITY, "0.0"));
checkState(errorProbability <= 1.0);
boolean attemptDirectPath = Boolean.getBoolean(ATTEMPT_DIRECT_PATH);
boolean enableDirectAccess = Boolean.getBoolean(ENABLE_DIRECT_ACCESS);
String directPathTestScenario = System.getProperty(DIRECT_PATH_TEST_SCENARIO, "");
SpannerOptions.Builder builder =
SpannerOptions.newBuilder()
Expand All @@ -85,15 +85,15 @@ public GceTestEnvConfig() {
}
SpannerInterceptorProvider interceptorProvider =
SpannerInterceptorProvider.createDefault().with(new GrpcErrorInjector(errorProbability));
if (attemptDirectPath) {
if (enableDirectAccess) {
interceptorProvider =
interceptorProvider.with(new DirectPathAddressCheckInterceptor(directPathTestScenario));
}
builder.setInterceptorProvider(interceptorProvider);
// DirectPath tests need to set a custom endpoint to the ChannelProvider
InstantiatingGrpcChannelProvider.Builder customChannelProviderBuilder =
InstantiatingGrpcChannelProvider.newBuilder();
if (attemptDirectPath) {
if (enableDirectAccess) {
customChannelProviderBuilder
.setEndpoint(DIRECT_PATH_ENDPOINT)
.setAttemptDirectPath(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1313,15 +1313,15 @@ public void testExperimentalHost() {
}

@Test
public void testAttemptDirectPath() {
public void testEnableDirectAccess() {
ConnectionOptions.Builder builderWithoutDirectPathParam = ConnectionOptions.newBuilder();
builderWithoutDirectPathParam.setUri(
"spanner://localhost:15000/instances/default/databases/singers-db;usePlainText=true");
assertFalse(builderWithoutDirectPathParam.build().isAttemptDirectPath());
assertFalse(builderWithoutDirectPathParam.build().isEnableDirectAccess());

ConnectionOptions.Builder builderWithDirectPathParam = ConnectionOptions.newBuilder();
builderWithDirectPathParam.setUri(
"spanner://localhost:15000/projects/default/instances/default/databases/singers-db;usePlainText=true;attemptDirectPath=true");
assertTrue(builderWithDirectPathParam.build().isAttemptDirectPath());
"spanner://localhost:15000/projects/default/instances/default/databases/singers-db;usePlainText=true;enableDirectAccess=true");
assertTrue(builderWithDirectPathParam.build().isEnableDirectAccess());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void testCredentialsProvider() throws Throwable {
.setConfigurator(
spannerOptions -> {
spannerOptions.setChannelConfigurator(ManagedChannelBuilder::usePlaintext);
spannerOptions.disableDirectPath();
spannerOptions.disableDirectAccess();
})
.build();

Expand Down Expand Up @@ -135,7 +135,7 @@ public void testCredentialsProvider() throws Throwable {
.setConfigurator(
spannerOptions -> {
spannerOptions.setChannelConfigurator(ManagedChannelBuilder::usePlaintext);
spannerOptions.disableDirectPath();
spannerOptions.disableDirectAccess();
})
.build();
try (Connection connection = options.getConnection()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public class ITDirectPathFallback {

// TODO(mohanli): Remove this temporary endpoint once DirectPath goes to public beta.
private static final String DIRECT_PATH_ENDPOINT = "aa423245250f2bbf.sandbox.googleapis.com:443";
private static final String ATTEMPT_DIRECT_PATH = "spanner.attempt_directpath";
private static final String ENABLE_DIRECT_ACCESS = "spanner.enable_direct_access";

public ITDirectPathFallback() {
// Create a transport channel provider that can intercept ipv6 packets.
Expand All @@ -112,7 +112,7 @@ public ITDirectPathFallback() {
public void setup() {
assume()
.withMessage("DirectPath integration tests can only run against DirectPathEnv")
.that(Boolean.getBoolean(ATTEMPT_DIRECT_PATH))
.that(Boolean.getBoolean(ENABLE_DIRECT_ACCESS))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe for a follow-up PR; I think that this will always return false, as we are parsing the string spanner.enable_direct_access as a boolean. Something similar was also true before this change, so I don't think we are breaking anything, but I doubt that this does what is intended to do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. We should do System.getProperty. I will raise a separate PR to fix this.

.isTrue();
// Get default spanner options for Ingetration test
SpannerOptions.Builder builder = env.getTestHelper().getOptions().toBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ private SpannerOptions createSpannerOptions() {
.setProjectId("[PROJECT]")
// Set a custom channel configurator to allow http instead of https.
.setChannelConfigurator(ManagedChannelBuilder::usePlaintext)
.disableDirectPath()
.disableDirectAccess()
.setHost("http://" + endpoint)
// Set static credentials that will return the static OAuth test token.
.setCredentials(STATIC_CREDENTIALS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ private static SpannerOptions createSpannerOptions(InetSocketAddress address, Se
.setProjectId("[PROJECT]")
// Set a custom channel configurator to allow http instead of https.
.setChannelConfigurator(ManagedChannelBuilder::usePlaintext)
.disableDirectPath()
.disableDirectAccess()
.setHost("http://" + endpoint)
// Set static credentials that will return the static OAuth test token.
.setCredentials(STATIC_CREDENTIALS)
Expand Down
Loading