From 860ae7646a985786966af55c5d3be35981e0fab1 Mon Sep 17 00:00:00 2001
From: Mohan Li <67390330+mohanli-ml@users.noreply.github.com>
Date: Wed, 18 Oct 2023 13:21:47 -0700
Subject: [PATCH] feat: Log DirectPath misconfiguration (#2105)

---
 .../InstantiatingGrpcChannelProvider.java     | 34 +++++++++
 .../InstantiatingGrpcChannelProviderTest.java | 69 +++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
index 06d0c18e51..4e7a654f7e 100644
--- a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
+++ b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
@@ -64,6 +64,8 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import javax.annotation.Nullable;
 import javax.net.ssl.KeyManagerFactory;
 import org.threeten.bp.Duration;
@@ -82,6 +84,9 @@
  */
 @InternalExtensionOnly
 public final class InstantiatingGrpcChannelProvider implements TransportChannelProvider {
+  @VisibleForTesting
+  static final Logger LOG = Logger.getLogger(InstantiatingGrpcChannelProvider.class.getName());
+
   private static final String DIRECT_PATH_ENV_DISABLE_DIRECT_PATH =
       "GOOGLE_CLOUD_DISABLE_DIRECT_PATH";
   private static final String DIRECT_PATH_ENV_ENABLE_XDS = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS";
@@ -140,6 +145,7 @@ private InstantiatingGrpcChannelProvider(Builder builder) {
         builder.directPathServiceConfig == null
             ? getDefaultDirectPathServiceConfig()
             : builder.directPathServiceConfig;
+    logDirectPathMisconfig();
   }
 
   /**
@@ -266,6 +272,33 @@ boolean isDirectPathXdsEnabled() {
     return false;
   }
 
+  private void logDirectPathMisconfig() {
+    if (isDirectPathXdsEnabled()) {
+      // Case 1: does not enable DirectPath
+      if (!isDirectPathEnabled()) {
+        LOG.log(
+            Level.WARNING,
+            "DirectPath is misconfigured. Please set the attemptDirectPath option along with the"
+                + " attemptDirectPathXds option.");
+      } else {
+        // Case 2: credential is not correctly set
+        if (!isNonDefaultServiceAccountAllowed()) {
+          LOG.log(
+              Level.WARNING,
+              "DirectPath is misconfigured. Please make sure the credential is an instance of "
+                  + ComputeEngineCredentials.class.getName()
+                  + " .");
+        }
+        // Case 3: not running on GCE
+        if (!isOnComputeEngine()) {
+          LOG.log(
+              Level.WARNING,
+              "DirectPath is misconfigured. DirectPath is only available in a GCE environment.");
+        }
+      }
+    }
+  }
+
   private boolean isNonDefaultServiceAccountAllowed() {
     if (allowNonDefaultServiceAccount != null && allowNonDefaultServiceAccount) {
       return true;
@@ -275,6 +308,7 @@ private boolean isNonDefaultServiceAccountAllowed() {
 
   // DirectPath should only be used on Compute Engine.
   // Notice Windows is supported for now.
+  @VisibleForTesting
   static boolean isOnComputeEngine() {
     String osName = System.getProperty("os.name");
     if ("Linux".equals(osName)) {
diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java
index d58a3b56b6..edd7a73768 100644
--- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java
+++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java
@@ -56,6 +56,9 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.logging.Handler;
+import java.util.logging.LogRecord;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -503,4 +506,70 @@ protected Object getMtlsObjectFromTransportChannel(MtlsProvider provider)
             .build();
     return channelProvider.createMtlsChannelCredentials();
   }
+
+  @Test
+  public void testLogDirectPathMisconfigAttrempDirectPathNotSet() {
+    FakeLogHandler logHandler = new FakeLogHandler();
+    InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
+    InstantiatingGrpcChannelProvider provider =
+        InstantiatingGrpcChannelProvider.newBuilder().setAttemptDirectPathXds().build();
+    assertThat(logHandler.getAllMessages())
+        .contains(
+            "DirectPath is misconfigured. Please set the attemptDirectPath option along with the"
+                + " attemptDirectPathXds option.");
+    InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
+  }
+
+  @Test
+  public void testLogDirectPathMisconfigWrongCredential() {
+    FakeLogHandler logHandler = new FakeLogHandler();
+    InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
+    InstantiatingGrpcChannelProvider provider =
+        InstantiatingGrpcChannelProvider.newBuilder()
+            .setAttemptDirectPathXds()
+            .setAttemptDirectPath(true)
+            .build();
+    assertThat(logHandler.getAllMessages())
+        .contains(
+            "DirectPath is misconfigured. Please make sure the credential is an instance of"
+                + " com.google.auth.oauth2.ComputeEngineCredentials .");
+    InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
+  }
+
+  @Test
+  public void testLogDirectPathMisconfigNotOnGCE() {
+    FakeLogHandler logHandler = new FakeLogHandler();
+    InstantiatingGrpcChannelProvider.LOG.addHandler(logHandler);
+    InstantiatingGrpcChannelProvider provider =
+        InstantiatingGrpcChannelProvider.newBuilder()
+            .setAttemptDirectPathXds()
+            .setAttemptDirectPath(true)
+            .setAllowNonDefaultServiceAccount(true)
+            .build();
+    if (!InstantiatingGrpcChannelProvider.isOnComputeEngine()) {
+      assertThat(logHandler.getAllMessages())
+          .contains(
+              "DirectPath is misconfigured. DirectPath is only available in a GCE environment.");
+    }
+    InstantiatingGrpcChannelProvider.LOG.removeHandler(logHandler);
+  }
+
+  private static class FakeLogHandler extends Handler {
+    List<LogRecord> records = new ArrayList<>();
+
+    @Override
+    public void publish(LogRecord record) {
+      records.add(record);
+    }
+
+    @Override
+    public void flush() {}
+
+    @Override
+    public void close() throws SecurityException {}
+
+    List<String> getAllMessages() {
+      return records.stream().map(LogRecord::getMessage).collect(Collectors.toList());
+    }
+  }
 }