feat: enable grpc and afe metrics (#3896)

Author: surbhigarg92

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

@@ -923,16 +923,10 @@ public boolean isEnableBuiltInMetrics() {
 
     @Override
     public boolean isEnableGRPCBuiltInMetrics() {
-      // 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
+      // Enable gRPC built-in metrics as default 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));
+      return !Boolean.parseBoolean(
+          System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS));
     }
 
     @Override

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java

@@ -682,15 +682,9 @@ private static boolean isEmulatorEnabled(SpannerOptions options, String emulator
   }
 
   public static boolean isEnableAFEServerTiming() {
-    // 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
+    // Enable AFE metrics as default 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));
+    return !Boolean.parseBoolean(System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING"));
   }
 
   public static boolean isEnableDirectPathXdsEnv() {

google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java

@@ -24,8 +24,6 @@
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeFalse;
-import static org.junit.Assume.assumeTrue;
 
 import com.google.api.gax.longrunning.OperationTimedPollAlgorithm;
 import com.google.api.gax.retrying.RetrySettings;
@@ -34,7 +32,6 @@
 import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
 import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
 import com.google.cloud.spanner.connection.RandomResultSetGenerator;
-import com.google.cloud.spanner.spi.v1.GapicSpannerRpc;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Range;
@@ -51,7 +48,6 @@
 import io.opentelemetry.sdk.metrics.data.MetricData;
 import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
 import java.io.IOException;
-import java.lang.reflect.Field;
 import java.net.InetSocketAddress;
 import java.time.Duration;
 import java.util.Collection;
@@ -195,19 +191,11 @@ public void testMetricsSingleUseQuery() {
         checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
     assertFalse(
         checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
-    if (GapicSpannerRpc.isEnableDirectPathXdsEnv()) {
-      // AFE metrics are enabled for DirectPath.
-      MetricData afeLatencyMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
-      double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
-      assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 1e-6);
-    } else {
-      MetricData gfeLatencyMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
-      double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
-      assertEquals(fakeServerTiming.get(), gfeLatencyValue, 1e-6);
-      assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
-    }
+    // AFE metrics are enabled for DirectPath.
+    MetricData afeLatencyMetricData =
+        getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
+    double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
+    assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 1e-6);
   }
 
   private boolean isJava8() {
@@ -218,75 +206,6 @@ private boolean isWindows() {
     return System.getProperty("os.name").toLowerCase().contains("windows");
   }
 
-  @Test
-  public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
-    assumeTrue(isJava8() && !isWindows());
-    assumeFalse(System.getenv().containsKey("SPANNER_DISABLE_AFE_SERVER_TIMING"));
-
-    Class<?> classOfMap = System.getenv().getClass();
-    Field field = classOfMap.getDeclaredField("m");
-    field.setAccessible(true);
-    Map<String, String> writeableEnvironmentVariables =
-        (Map<String, String>) field.get(System.getenv());
-
-    try {
-      writeableEnvironmentVariables.put("SPANNER_DISABLE_AFE_SERVER_TIMING", "false");
-
-      Stopwatch stopwatch = Stopwatch.createStarted();
-      try (ResultSet resultSet = client.singleUse().executeQuery(SELECT_RANDOM)) {
-        assertTrue(resultSet.next());
-        assertFalse(resultSet.next());
-      }
-
-      double elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
-      Attributes expectedAttributes =
-          expectedCommonBaseAttributes.toBuilder()
-              .putAll(expectedCommonRequestAttributes)
-              .put(BuiltInMetricsConstant.STATUS_KEY, "OK")
-              .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteStreamingSql")
-              .build();
-
-      MetricData operationLatencyMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME);
-      assertNotNull(operationLatencyMetricData);
-      double operationLatencyValue =
-          getAggregatedValue(operationLatencyMetricData, expectedAttributes);
-      assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
-
-      MetricData attemptLatencyMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME);
-      assertNotNull(attemptLatencyMetricData);
-      double attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
-      assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
-
-      MetricData operationCountMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_COUNT_NAME);
-      assertNotNull(operationCountMetricData);
-      assertThat(getAggregatedValue(operationCountMetricData, expectedAttributes)).isEqualTo(1);
-
-      MetricData attemptCountMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME);
-      assertNotNull(attemptCountMetricData);
-      assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributes)).isEqualTo(1);
-
-      assertFalse(
-          checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
-      assertFalse(
-          checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
-      MetricData afeLatencyMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
-      double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
-      assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 1e-6);
-
-      MetricData gfeLatencyMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
-      double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
-      assertEquals(fakeServerTiming.get(), gfeLatencyValue, 1e-6);
-    } finally {
-      writeableEnvironmentVariables.remove("GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS");
-    }
-  }
-
   @Test
   public void testMetricsWithGaxRetryUnaryRpc() {
     Stopwatch stopwatch = Stopwatch.createStarted();
@@ -454,17 +373,9 @@ public void testNoServerTimingHeader() throws IOException, InterruptedException
 
     assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
     assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME));
-    if (GapicSpannerRpc.isEnableDirectPathXdsEnv()) {
-      MetricData afeConnectivityMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME);
-      assertThat(getAggregatedValue(afeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
-    } else {
-      MetricData gfeConnectivityMetricData =
-          getMetricData(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME);
-      assertThat(getAggregatedValue(gfeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
-      assertFalse(
-          checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
-    }
+    MetricData afeConnectivityMetricData =
+        getMetricData(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME);
+    assertThat(getAggregatedValue(afeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
 
     spannerNoHeader.close();
     serverNoHeader.shutdown();