Skip to content

Commit 6382ae4

Browse files
nimfnimf
committed
fix: Do not warn directpath unavailability outside GCE
1 parent 6a76397 commit 6382ae4

2 files changed

Lines changed: 48 additions & 73 deletions

File tree

‎gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java‎

Lines changed: 33 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -441,43 +441,39 @@ public boolean isDirectPathXdsEnabled() {
441441
// builder or createSingleChannel, only in getTransportChannel which creates the first channel
442442
// for a client.
443443
private void logDirectPathMisconfig() {
444-
if (isDirectPathXdsEnabled()) {
445-
if (!isDirectPathEnabled()) {
446-
// This misconfiguration occurs when Direct Path xDS is enabled, but Direct Path is not
447-
// Direct Path xDS can be enabled two ways: via environment variable or via builder.
448-
// Case 1: Direct Path is only enabled via xDS env var. We will _warn_ the user that this is
449-
// a misconfiguration if they intended to set the env var.
450-
if (isDirectPathXdsEnabledViaEnv()) {
451-
LOG.log(
452-
Level.WARNING,
453-
"Env var "
454-
+ DIRECT_PATH_ENV_ENABLE_XDS
455-
+ " was found and set to TRUE, but DirectPath was not enabled for this client. If this is intended for "
456-
+ "this client, please note that this is a misconfiguration and set the attemptDirectPath option as well.");
457-
}
458-
// Case 2: Direct Path xDS was enabled via Builder. Direct Path Traffic Director must be set
459-
// (enabled with `setAttemptDirectPath(true)`) along with xDS.
460-
// Here we warn the user about this.
461-
else if (isDirectPathXdsEnabledViaBuilderOption()) {
462-
LOG.log(
463-
Level.WARNING,
464-
"DirectPath is misconfigured. The DirectPath XDS option was set, but the attemptDirectPath option was not. Please set both the attemptDirectPath and attemptDirectPathXds options.");
465-
}
466-
} else {
467-
// Case 3: credential is not correctly set
468-
if (!isCredentialDirectPathCompatible()) {
469-
LOG.log(
470-
Level.WARNING,
471-
"DirectPath is misconfigured. Please make sure the credential is an instance of "
472-
+ ComputeEngineCredentials.class.getName()
473-
+ " .");
474-
}
475-
// Case 4: not running on GCE
476-
if (!isOnComputeEngine()) {
477-
LOG.log(
478-
Level.WARNING,
479-
"DirectPath is misconfigured. DirectPath is only available in a GCE environment.");
480-
}
444+
if (!isDirectPathXdsEnabled() || !isOnComputeEngine()) {
445+
return;
446+
}
447+
448+
if (!isDirectPathEnabled()) {
449+
// This misconfiguration occurs when Direct Path xDS is enabled, but Direct Path is not
450+
// Direct Path xDS can be enabled two ways: via environment variable or via builder.
451+
// Case 1: Direct Path is only enabled via xDS env var. We will _warn_ the user that this is
452+
// a misconfiguration if they intended to set the env var.
453+
if (isDirectPathXdsEnabledViaEnv()) {
454+
LOG.log(
455+
Level.WARNING,
456+
"Env var "
457+
+ DIRECT_PATH_ENV_ENABLE_XDS
458+
+ " was found and set to TRUE, but DirectPath was not enabled for this client. If this is intended for "
459+
+ "this client, please note that this is a misconfiguration and set the attemptDirectPath option as well.");
460+
}
461+
// Case 2: Direct Path xDS was enabled via Builder. Direct Path Traffic Director must be set
462+
// (enabled with `setAttemptDirectPath(true)`) along with xDS.
463+
// Here we warn the user about this.
464+
else if (isDirectPathXdsEnabledViaBuilderOption()) {
465+
LOG.log(
466+
Level.WARNING,
467+
"DirectPath is misconfigured. The DirectPath XDS option was set, but the attemptDirectPath option was not. Please set both the attemptDirectPath and attemptDirectPathXds options.");
468+
}
469+
} else {
470+
// Case 3: credential is not correctly set
471+
if (!isCredentialDirectPathCompatible()) {
472+
LOG.log(
473+
Level.WARNING,
474+
"DirectPath is misconfigured. Please make sure the credential is an instance of "
475+
+ ComputeEngineCredentials.class.getName()
476+
+ " .");
481477
}
482478
}
483479
}

‎gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java‎

Lines changed: 15 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -656,9 +656,12 @@ private void createAndCloseTransportChannel(InstantiatingGrpcChannelProvider pro
656656
.setCertificateBasedAccess(certificateBasedAccess)
657657
.build();
658658
createAndCloseTransportChannel(provider);
659-
assertThat(logHandler.getAllMessages())
660-
.contains(
661-
"DirectPath is misconfigured. The DirectPath XDS option was set, but the attemptDirectPath option was not. Please set both the attemptDirectPath and attemptDirectPathXds options.");
659+
660+
if (InstantiatingGrpcChannelProvider.isOnComputeEngine()) {
661+
assertThat(logHandler.getAllMessages())
662+
.contains(
663+
"DirectPath is misconfigured. The DirectPath XDS option was set, but the attemptDirectPath option was not. Please set both the attemptDirectPath and attemptDirectPathXds options.");
664+
}
662665
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
663666
}
664667

@@ -673,10 +676,12 @@ void testLogDirectPathMisconfig_AttemptDirectPathNotSetAndAttemptDirectPathXdsSe
673676
.setCertificateBasedAccess(certificateBasedAccess)
674677
.build();
675678
createAndCloseTransportChannel(provider);
676-
assertThat(logHandler.getAllMessages())
677-
.contains(
678-
"Env var GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS was found and set to TRUE, but DirectPath was not enabled for this client. If this is intended for "
679-
+ "this client, please note that this is a misconfiguration and set the attemptDirectPath option as well.");
679+
if (InstantiatingGrpcChannelProvider.isOnComputeEngine()) {
680+
assertThat(logHandler.getAllMessages())
681+
.contains(
682+
"Env var GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS was found and set to TRUE, but DirectPath was not enabled for this client. If this is intended for "
683+
+ "this client, please note that this is a misconfiguration and set the attemptDirectPath option as well.");
684+
}
680685
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
681686
}
682687

@@ -710,37 +715,11 @@ void testLogDirectPathMisconfigWrongCredential() throws Exception {
710715

711716
TransportChannel transportChannel = provider.getTransportChannel();
712717

713-
assertThat(logHandler.getAllMessages())
714-
.contains(
715-
"DirectPath is misconfigured. Please make sure the credential is an instance of"
716-
+ " com.google.auth.oauth2.ComputeEngineCredentials .");
717-
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
718-
719-
transportChannel.close();
720-
transportChannel.awaitTermination(10, TimeUnit.SECONDS);
721-
}
722-
723-
@Test
724-
void testLogDirectPathMisconfigNotOnGCE() throws Exception {
725-
FakeLogHandler logHandler = new FakeLogHandler();
726-
InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
727-
InstantiatingGrpcChannelProvider provider =
728-
InstantiatingGrpcChannelProvider.newBuilder()
729-
.setAttemptDirectPathXds()
730-
.setAttemptDirectPath(true)
731-
.setAllowNonDefaultServiceAccount(true)
732-
.setHeaderProvider(Mockito.mock(HeaderProvider.class))
733-
.setExecutor(Mockito.mock(Executor.class))
734-
.setEndpoint(DEFAULT_ENDPOINT)
735-
.setCertificateBasedAccess(certificateBasedAccess)
736-
.build();
737-
738-
TransportChannel transportChannel = provider.getTransportChannel();
739-
740-
if (!InstantiatingGrpcChannelProvider.isOnComputeEngine()) {
718+
if (InstantiatingGrpcChannelProvider.isOnComputeEngine()) {
741719
assertThat(logHandler.getAllMessages())
742720
.contains(
743-
"DirectPath is misconfigured. DirectPath is only available in a GCE environment.");
721+
"DirectPath is misconfigured. Please make sure the credential is an instance of"
722+
+ " com.google.auth.oauth2.ComputeEngineCredentials .");
744723
}
745724
InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
746725

0 commit comments

Comments
 (0)