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 index a25e8bfa99..790c602d45 100644 --- 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,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 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 index 0b3c6c649b..27d63c930d 100644 --- 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 @@ -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() { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java index 55c5cf4771..c3b91ad6c5 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetryBuiltInMetricsTracerTest.java +++ b/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 writeableEnvironmentVariables = - (Map) 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();