Skip to content

Commit c2e6c0f

Browse files
committed
review comments
1 parent 4283a54 commit c2e6c0f

File tree

2 files changed

+20
-6
lines changed

2 files changed

+20
-6
lines changed

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

+11-4
Original file line numberDiff line numberDiff line change
@@ -923,8 +923,16 @@ 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
928+
// "false", OR
929+
// 2. DirectPath is enabled AND the env var is not set to "true"
930+
// This allows metrics to be enabled by default when DirectPath is on, unless explicitly
931+
// disabled via env.
932+
String grpcDisableEnv = System.getenv("SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS");
933+
boolean isDirectPathEnabled = GapicSpannerRpc.isEnableDirectPathXdsEnv();
934+
return ("false".equalsIgnoreCase(grpcDisableEnv))
935+
|| (isDirectPathEnabled && !"true".equalsIgnoreCase(grpcDisableEnv));
928936
}
929937

930938
@Override
@@ -1994,8 +2002,7 @@ public ApiTracerFactory getApiTracerFactory() {
19942002
}
19952003

19962004
public void enablegRPCMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder) {
1997-
if (GapicSpannerRpc.isEnableDirectPathXdsEnv()
1998-
|| SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) {
2005+
if (SpannerOptions.environment.isEnableGRPCBuiltInMetrics()) {
19992006
this.builtInMetricsProvider.enableGrpcMetrics(
20002007
channelProviderBuilder, this.getProjectId(), getCredentials(), this.monitoringHost);
20012008
}

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

+9-2
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,15 @@ 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
684+
// disabled via env.
685+
String afeDisableEnv = System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING");
686+
boolean isDirectPathEnabled = isEnableDirectPathXdsEnv();
687+
return ("false".equalsIgnoreCase(afeDisableEnv))
688+
|| (isDirectPathEnabled && !"true".equalsIgnoreCase(afeDisableEnv));
682689
}
683690

684691
public static boolean isEnableDirectPathXdsEnv() {

0 commit comments

Comments
 (0)