Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP
@Nullable private final Credentials credentials;
@Nullable private final ChannelPrimer channelPrimer;
@Nullable private final Boolean attemptDirectPath;
@Nullable private final Boolean attemptDirectPathXds;
@Nullable private final Boolean allowNonDefaultServiceAccount;
@VisibleForTesting final ImmutableMap<String, ?> directPathServiceConfig;
@Nullable private final MtlsProvider mtlsProvider;
Expand All @@ -133,6 +134,7 @@ private InstantiatingGrpcChannelProvider(Builder builder) {
this.credentials = builder.credentials;
this.channelPrimer = builder.channelPrimer;
this.attemptDirectPath = builder.attemptDirectPath;
this.attemptDirectPathXds = builder.attemptDirectPathXds;
this.allowNonDefaultServiceAccount = builder.allowNonDefaultServiceAccount;
this.directPathServiceConfig =
builder.directPathServiceConfig == null
Expand Down Expand Up @@ -249,6 +251,21 @@ private boolean isDirectPathEnabled() {
return false;
}

@VisibleForTesting
boolean isDirectPathXdsEnabled() {
// Method 1: Enable DirectPath xDS by option.
if (Boolean.TRUE.equals(attemptDirectPathXds)) {
return true;
}
// Method 2: Enable DirectPath xDS by env.
String directPathXdsEnv = envProvider.getenv(DIRECT_PATH_ENV_ENABLE_XDS);
boolean isDirectPathXdsEnv = Boolean.parseBoolean(directPathXdsEnv);
if (isDirectPathXdsEnv) {
return true;
}
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add one more test case for the default value? Our sonar check is failing because the new code is lacking coverage, I know we cannot set environment variable in unit tests, but we should be able to test the default case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. PTAL. Thanks!

}

private boolean isNonDefaultServiceAccountAllowed() {
if (allowNonDefaultServiceAccount != null && allowNonDefaultServiceAccount) {
return true;
Expand Down Expand Up @@ -304,13 +321,13 @@ private ManagedChannel createSingleChannel() throws IOException {
ManagedChannelBuilder<?> builder;

// Check DirectPath traffic.
boolean isDirectPathXdsEnabled = false;
boolean useDirectPathXds = false;
if (isDirectPathEnabled() && isNonDefaultServiceAccountAllowed() && isOnComputeEngine()) {
CallCredentials callCreds = MoreCallCredentials.from(credentials);
ChannelCredentials channelCreds =
GoogleDefaultChannelCredentials.newBuilder().callCredentials(callCreds).build();
isDirectPathXdsEnabled = Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENV_ENABLE_XDS));
if (isDirectPathXdsEnabled) {
useDirectPathXds = isDirectPathXdsEnabled();
if (useDirectPathXds) {
// google-c2p: CloudToProd(C2P) Directpath. This scheme is defined in
// io.grpc.googleapis.GoogleCloudToProdNameResolverProvider.
// This resolver target must not have a port number.
Expand All @@ -337,7 +354,7 @@ private ManagedChannel createSingleChannel() throws IOException {
}
}
// google-c2p resolver requires service config lookup
if (!isDirectPathXdsEnabled) {
if (!useDirectPathXds) {
// See https://github.com/googleapis/gapic-generator/issues/2816
builder.disableServiceConfigLookUp();
}
Expand Down Expand Up @@ -435,6 +452,7 @@ public static final class Builder {
@Nullable private ChannelPrimer channelPrimer;
private ChannelPoolSettings channelPoolSettings;
@Nullable private Boolean attemptDirectPath;
@Nullable private Boolean attemptDirectPathXds;
@Nullable private Boolean allowNonDefaultServiceAccount;
@Nullable private ImmutableMap<String, ?> directPathServiceConfig;

Expand All @@ -461,6 +479,7 @@ private Builder(InstantiatingGrpcChannelProvider provider) {
this.channelPrimer = provider.channelPrimer;
this.channelPoolSettings = provider.channelPoolSettings;
this.attemptDirectPath = provider.attemptDirectPath;
this.attemptDirectPathXds = provider.attemptDirectPathXds;
this.allowNonDefaultServiceAccount = provider.allowNonDefaultServiceAccount;
this.directPathServiceConfig = provider.directPathServiceConfig;
this.mtlsProvider = provider.mtlsProvider;
Expand Down Expand Up @@ -669,6 +688,13 @@ public Builder setAllowNonDefaultServiceAccount(boolean allowNonDefaultServiceAc
return this;
}

/** Use DirectPath xDS. Only valid if DirectPath is attempted. */
@InternalApi("For internal use by google-cloud-java clients only")
public Builder setAttemptDirectPathXds() {
this.attemptDirectPathXds = true;
return this;
}

/**
* Sets a service config for direct path. If direct path is not enabled, the provided service
* config will be ignored.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,25 @@ public void testWithGCECredentials() throws IOException {
provider.getTransportChannel().shutdownNow();
}

@Test
public void testDirectPathXdsDisableByDefault() throws IOException {
InstantiatingGrpcChannelProvider provider =
InstantiatingGrpcChannelProvider.newBuilder().setAttemptDirectPath(true).build();

assertThat(provider.isDirectPathXdsEnabled()).isFalse();
}

@Test
public void testDirectPathXdsEnabled() throws IOException {
InstantiatingGrpcChannelProvider provider =
InstantiatingGrpcChannelProvider.newBuilder()
.setAttemptDirectPath(true)
.setAttemptDirectPathXds()
.build();

assertThat(provider.isDirectPathXdsEnabled()).isTrue();
}

@Test
public void testWithNonGCECredentials() throws IOException {
ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1);
Expand Down