Skip to content

Commit a7dd7b5

Browse files
committed
review comments
1 parent 4283a54 commit a7dd7b5

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

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

+9-4
Original file line numberDiff line numberDiff line change
@@ -923,8 +923,14 @@ public boolean isEnableBuiltInMetrics() {
923923

924924
@Override
925925
public boolean isEnableGRPCBuiltInMetrics() {
926-
return "false"
927-
.equalsIgnoreCase(System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS));
926+
// Enable gRPC built-in metrics if:
927+
// 1. The env var SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS is explicitly set to "false", OR
928+
// 2. DirectPath is enabled AND the env var is not set to "true"
929+
// This allows metrics to be enabled by default when DirectPath is on, unless explicitly disabled via env.
930+
String grpcDisableEnv = System.getenv("SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS");
931+
boolean isDirectPathEnabled = GapicSpannerRpc.isEnableDirectPathXdsEnv();
932+
return ("false".equalsIgnoreCase(grpcDisableEnv)) ||
933+
(isDirectPathEnabled && !"true".equalsIgnoreCase(grpcDisableEnv));
928934
}
929935

930936
@Override
@@ -1994,8 +2000,7 @@ public ApiTracerFactory getApiTracerFactory() {
19942000
}
19952001

19962002
public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder) {
1997-
if (GapicSpannerRpc.isEnableDirectPathXdsEnv()
1998-
|| SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) {
2003+
if (SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) {
19992004
this.builtInMetricsProvider.enableGrpcMetrics(
20002005
channelProviderBuilder, this.getProjectId(), getCredentials(), this.monitoringHost);
20012006
}

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

+8-2
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,14 @@ private static boolean isEmulatorEnabled(SpannerOptions options, String emulator
677677
}
678678

679679
public static boolean isEnableAFEServerTiming() {
680-
return isEnableDirectPathXdsEnv()
681-
|| "false".equalsIgnoreCase(System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING"));
680+
// Enable AFE metrics and add AFE header if:
681+
// 1. The env var SPANNER_DISABLE_AFE_SERVER_TIMING is explicitly set to "false", OR
682+
// 2. DirectPath is enabled AND the env var is not set to "true"
683+
// This allows metrics to be enabled by default when DirectPath is on, unless explicitly disabled via env.
684+
String afeDisableEnv = System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING");
685+
boolean isDirectPathEnabled = isEnableDirectPathXdsEnv();
686+
return ("false".equalsIgnoreCase(afeDisableEnv)) ||
687+
(isDirectPathEnabled && !"true".equalsIgnoreCase(afeDisableEnv));
682688
}
683689

684690
public static boolean isEnableDirectPathXdsEnv() {

0 commit comments

Comments
 (0)