review comments

surbhigarg92 · surbhigarg92 · commit a7dd7b5a06a9 · 2025-05-09T12:11:46.000+05:30
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java
@@ -923,8 +923,14 @@ public boolean isEnableBuiltInMetrics() {
 
     @Override
     public boolean isEnableGRPCBuiltInMetrics() {
-      return "false"
-          .equalsIgnoreCase(System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS));
+      // Enable gRPC built-in metrics if:
+      // 1. The env var SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS is explicitly set to "false", OR
+      // 2. DirectPath is enabled AND the env var is not set to "true"
+      // This allows metrics to be enabled by default when DirectPath is on, unless explicitly disabled via env.
+      String grpcDisableEnv = System.getenv("SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS");
+      boolean isDirectPathEnabled = GapicSpannerRpc.isEnableDirectPathXdsEnv();
+      return ("false".equalsIgnoreCase(grpcDisableEnv)) ||
+              (isDirectPathEnabled && !"true".equalsIgnoreCase(grpcDisableEnv));
     }
 
     @Override
@@ -1994,8 +2000,7 @@ public ApiTracerFactory getApiTracerFactory() {
   }
 
   public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder) {
-    if (GapicSpannerRpc.isEnableDirectPathXdsEnv()
-        || SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) {
+    if (SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) {
       this.builtInMetricsProvider.enableGrpcMetrics(
           channelProviderBuilder, this.getProjectId(), getCredentials(), this.monitoringHost);
     }
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java
@@ -677,8 +677,14 @@ private static boolean isEmulatorEnabled(SpannerOptions options, String emulator
   }
 
   public static boolean isEnableAFEServerTiming() {
-    return isEnableDirectPathXdsEnv()
-        || "false".equalsIgnoreCase(System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING"));
+    // Enable AFE metrics and add AFE header if:
+    // 1. The env var SPANNER_DISABLE_AFE_SERVER_TIMING is explicitly set to "false", OR
+    // 2. DirectPath is enabled AND the env var is not set to "true"
+    // This allows metrics to be enabled by default when DirectPath is on, unless explicitly disabled via env.
+    String afeDisableEnv = System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING");
+    boolean isDirectPathEnabled = isEnableDirectPathXdsEnv();
+    return ("false".equalsIgnoreCase(afeDisableEnv)) ||
+        (isDirectPathEnabled && !"true".equalsIgnoreCase(afeDisableEnv));
   }
 
   public static boolean isEnableDirectPathXdsEnv() {
