Skip to content

Commit 203baae

Browse files
surbhigarg92cloud-java-botrahul2393
authored
feat: enable AFE and gRPC metrics for DP (#3852)
* feat: enable AFE and gRPC metrics for DP * check afe metrics when DP is enabled * check afe metrics when DP is enabled * mergeconflict * chore: generate libraries at Fri May 9 05:49:15 UTC 2025 * review comments * roundofferrorfordouble --------- Co-authored-by: cloud-java-bot <[email protected]> Co-authored-by: rahul2393 <[email protected]>
1 parent e205a2b commit 203baae

File tree

4 files changed

+64
-29
lines changed

4 files changed

+64
-29
lines changed

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

Lines changed: 10 additions & 2 deletions
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

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
367367
.withEncoding(compressorName))
368368
.setHeaderProvider(headerProviderWithUserAgent)
369369
.setAllowNonDefaultServiceAccount(true);
370-
String directPathXdsEnv = System.getenv("GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS");
371-
boolean isAttemptDirectPathXds = Boolean.parseBoolean(directPathXdsEnv);
370+
boolean isAttemptDirectPathXds = isEnableDirectPathXdsEnv();
372371
if (isAttemptDirectPathXds) {
373372
defaultChannelProviderBuilder.setAttemptDirectPath(true);
374373
defaultChannelProviderBuilder.setAttemptDirectPathXds();
@@ -678,7 +677,19 @@ private static boolean isEmulatorEnabled(SpannerOptions options, String emulator
678677
}
679678

680679
public static boolean isEnableAFEServerTiming() {
681-
return "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));
689+
}
690+
691+
public static boolean isEnableDirectPathXdsEnv() {
692+
return Boolean.parseBoolean(System.getenv("GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS"));
682693
}
683694

684695
private static final RetrySettings ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS =

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ public void onHeaders(Metadata metadata) {
133133
Boolean isDirectPathUsed =
134134
isDirectPathUsed(getAttributes().get(Grpc.TRANSPORT_ATTR_REMOTE_ADDR));
135135
addDirectPathUsedAttribute(compositeTracer, isDirectPathUsed);
136-
processHeader(metadata, tagContext, attributes, span, compositeTracer);
136+
processHeader(
137+
metadata, tagContext, attributes, span, compositeTracer, isDirectPathUsed);
137138
super.onHeaders(metadata);
138139
}
139140
},
@@ -151,7 +152,8 @@ private void processHeader(
151152
TagContext tagContext,
152153
Attributes attributes,
153154
Span span,
154-
CompositeTracer compositeTracer) {
155+
CompositeTracer compositeTracer,
156+
boolean isDirectPathUsed) {
155157
MeasureMap measureMap = STATS_RECORDER.newMeasureMap();
156158
String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY);
157159
try {
@@ -172,7 +174,7 @@ private void processHeader(
172174

173175
spannerRpcMetrics.recordGfeLatency((long) gfeLatency, attributes);
174176
spannerRpcMetrics.recordGfeHeaderMissingCount(0L, attributes);
175-
if (compositeTracer != null) {
177+
if (compositeTracer != null && !isDirectPathUsed) {
176178
compositeTracer.recordGFELatency(gfeLatency);
177179
}
178180
if (span != null) {
@@ -181,7 +183,7 @@ private void processHeader(
181183
} else {
182184
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext);
183185
spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes);
184-
if (compositeTracer != null) {
186+
if (compositeTracer != null && !isDirectPathUsed) {
185187
compositeTracer.recordGfeHeaderMissingCount(1L);
186188
}
187189
}

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

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
3535
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
3636
import com.google.cloud.spanner.connection.RandomResultSetGenerator;
37+
import com.google.cloud.spanner.spi.v1.GapicSpannerRpc;
3738
import com.google.common.base.Stopwatch;
3839
import com.google.common.collect.ImmutableList;
3940
import com.google.common.collect.Range;
@@ -190,16 +191,23 @@ public void testMetricsSingleUseQuery() {
190191
assertNotNull(attemptCountMetricData);
191192
assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributes)).isEqualTo(1);
192193

193-
MetricData gfeLatencyMetricData =
194-
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
195-
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
196-
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0);
197-
198194
assertFalse(
199195
checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
200-
assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
201196
assertFalse(
202197
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
198+
if (GapicSpannerRpc.isEnableDirectPathXdsEnv()) {
199+
// AFE metrics are enabled for DirectPath.
200+
MetricData afeLatencyMetricData =
201+
getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
202+
double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
203+
assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 1e-6);
204+
} else {
205+
MetricData gfeLatencyMetricData =
206+
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
207+
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
208+
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 1e-6);
209+
assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
210+
}
203211
}
204212

205213
private boolean isJava8() {
@@ -261,20 +269,19 @@ public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
261269
assertNotNull(attemptCountMetricData);
262270
assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributes)).isEqualTo(1);
263271

264-
MetricData gfeLatencyMetricData =
265-
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
266-
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
267-
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 0);
268-
269272
assertFalse(
270273
checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
271-
274+
assertFalse(
275+
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
272276
MetricData afeLatencyMetricData =
273277
getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
274278
double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
275-
assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 0);
276-
assertFalse(
277-
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
279+
assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 1e-6);
280+
281+
MetricData gfeLatencyMetricData =
282+
getMetricData(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME);
283+
double gfeLatencyValue = getAggregatedValue(gfeLatencyMetricData, expectedAttributes);
284+
assertEquals(fakeServerTiming.get(), gfeLatencyValue, 1e-6);
278285
} finally {
279286
writeableEnvironmentVariables.remove("GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS");
280287
}
@@ -445,13 +452,20 @@ public void testNoServerTimingHeader() throws IOException, InterruptedException
445452
.put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteSql")
446453
.build();
447454

448-
MetricData gfeConnectivityMetricData =
449-
getMetricData(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME);
450-
assertThat(getAggregatedValue(gfeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
451455
assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
452456
assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME));
453-
assertFalse(
454-
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
457+
if (GapicSpannerRpc.isEnableDirectPathXdsEnv()) {
458+
MetricData afeConnectivityMetricData =
459+
getMetricData(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME);
460+
assertThat(getAggregatedValue(afeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
461+
} else {
462+
MetricData gfeConnectivityMetricData =
463+
getMetricData(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME);
464+
assertThat(getAggregatedValue(gfeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
465+
assertFalse(
466+
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
467+
}
468+
455469
spannerNoHeader.close();
456470
serverNoHeader.shutdown();
457471
serverNoHeader.awaitTermination();

0 commit comments

Comments
 (0)