Skip to content

Commit d9bffe4

Browse files
chore: merge main into generate-libraries-main
2 parents 87118c0 + 53bc510 commit d9bffe4

File tree

5 files changed

+22
-136
lines changed

5 files changed

+22
-136
lines changed

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -923,16 +923,10 @@ public boolean isEnableBuiltInMetrics() {
923923

924924
@Override
925925
public boolean isEnableGRPCBuiltInMetrics() {
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
926+
// Enable gRPC built-in metrics as default unless explicitly
931927
// 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));
928+
return !Boolean.parseBoolean(
929+
System.getenv(SPANNER_DISABLE_DIRECT_ACCESS_GRPC_BUILTIN_METRICS));
936930
}
937931

938932
@Override

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

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@
8686
import com.google.common.base.Function;
8787
import com.google.common.base.MoreObjects;
8888
import com.google.common.base.Preconditions;
89-
import com.google.common.base.Supplier;
90-
import com.google.common.base.Suppliers;
9189
import com.google.common.collect.ImmutableList;
9290
import com.google.common.collect.ImmutableMap;
9391
import com.google.common.collect.ImmutableSet;
@@ -239,6 +237,7 @@ public class GapicSpannerRpc implements SpannerRpc {
239237
private static final String CLIENT_LIBRARY_LANGUAGE = "spanner-java";
240238
public static final String DEFAULT_USER_AGENT =
241239
CLIENT_LIBRARY_LANGUAGE + "/" + GaxProperties.getLibraryVersion(GapicSpannerRpc.class);
240+
public static boolean DIRECTPATH_CHANNEL_CREATED = false;
242241
private static final String API_FILE = "grpc-gcp-apiconfig.json";
243242

244243
private boolean rpcIsClosed;
@@ -281,8 +280,6 @@ public class GapicSpannerRpc implements SpannerRpc {
281280
private final int numChannels;
282281
private final boolean isGrpcGcpExtensionEnabled;
283282

284-
private Supplier<Boolean> directPathEnabledSupplier = () -> false;
285-
286283
private final GrpcCallContext baseGrpcCallContext;
287284

288285
public static GapicSpannerRpc create(SpannerOptions options) {
@@ -361,9 +358,7 @@ public GapicSpannerRpc(final SpannerOptions options) {
361358
SpannerInterceptorProvider.create(
362359
MoreObjects.firstNonNull(
363360
options.getInterceptorProvider(),
364-
SpannerInterceptorProvider.createDefault(
365-
options.getOpenTelemetry(),
366-
(() -> directPathEnabledSupplier.get()))))
361+
SpannerInterceptorProvider.createDefault(options.getOpenTelemetry())))
367362
// This sets the trace context headers.
368363
.withTraceContext(endToEndTracingEnabled, options.getOpenTelemetry())
369364
// This sets the response compressor (Server -> Client).
@@ -425,12 +420,9 @@ public GapicSpannerRpc(final SpannerOptions options) {
425420
this.spannerStub =
426421
GrpcSpannerStubWithStubSettingsAndClientContext.create(
427422
spannerStubSettings, clientContext);
428-
this.directPathEnabledSupplier =
429-
Suppliers.memoize(
430-
() -> {
431-
return ((GrpcTransportChannel) clientContext.getTransportChannel()).isDirectPath()
432-
&& isAttemptDirectPathXds;
433-
});
423+
DIRECTPATH_CHANNEL_CREATED =
424+
((GrpcTransportChannel) clientContext.getTransportChannel()).isDirectPath()
425+
&& isAttemptDirectPathXds;
434426
this.readRetrySettings =
435427
options.getSpannerStubSettings().streamingReadSettings().getRetrySettings();
436428
this.readRetryableCodes =
@@ -682,15 +674,9 @@ private static boolean isEmulatorEnabled(SpannerOptions options, String emulator
682674
}
683675

684676
public static boolean isEnableAFEServerTiming() {
685-
// Enable AFE metrics and add AFE header if:
686-
// 1. The env var SPANNER_DISABLE_AFE_SERVER_TIMING is explicitly set to "false", OR
687-
// 2. DirectPath is enabled AND the env var is not set to "true"
688-
// This allows metrics to be enabled by default when DirectPath is on, unless explicitly
677+
// Enable AFE metrics as default unless explicitly
689678
// disabled via env.
690-
String afeDisableEnv = System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING");
691-
boolean isDirectPathEnabled = isEnableDirectPathXdsEnv();
692-
return ("false".equalsIgnoreCase(afeDisableEnv))
693-
|| (isDirectPathEnabled && !"true".equalsIgnoreCase(afeDisableEnv));
679+
return !Boolean.parseBoolean(System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING"));
694680
}
695681

696682
public static boolean isEnableDirectPathXdsEnv() {

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import java.util.HashMap;
5757
import java.util.Map;
5858
import java.util.concurrent.ExecutionException;
59-
import java.util.function.Supplier;
6059
import java.util.logging.Level;
6160
import java.util.logging.Logger;
6261
import java.util.regex.Matcher;
@@ -99,12 +98,8 @@ class HeaderInterceptor implements ClientInterceptor {
9998
private static final Level LEVEL = Level.INFO;
10099
private final SpannerRpcMetrics spannerRpcMetrics;
101100

102-
private final Supplier<Boolean> directPathEnabledSupplier;
103-
104-
HeaderInterceptor(
105-
SpannerRpcMetrics spannerRpcMetrics, Supplier<Boolean> directPathEnabledSupplier) {
101+
HeaderInterceptor(SpannerRpcMetrics spannerRpcMetrics) {
106102
this.spannerRpcMetrics = spannerRpcMetrics;
107-
this.directPathEnabledSupplier = directPathEnabledSupplier;
108103
}
109104

110105
@Override
@@ -293,7 +288,7 @@ private Map<String, String> getBuiltInMetricAttributes(String key, DatabaseName
293288
BuiltInMetricsConstant.INSTANCE_ID_KEY.getKey(), databaseName.getInstance());
294289
attributes.put(
295290
BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY.getKey(),
296-
String.valueOf(this.directPathEnabledSupplier.get()));
291+
String.valueOf(GapicSpannerRpc.DIRECTPATH_CHANNEL_CREATED));
297292
return attributes;
298293
});
299294
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ public static SpannerInterceptorProvider createDefault(OpenTelemetry openTelemet
5656
}));
5757
}
5858

59+
@ObsoleteApi("DirectPathEnabledSupplier is not used")
5960
public static SpannerInterceptorProvider createDefault(
6061
OpenTelemetry openTelemetry, Supplier<Boolean> directPathEnabledSupplier) {
6162
List<ClientInterceptor> defaultInterceptorList = new ArrayList<>();
6263
defaultInterceptorList.add(new SpannerErrorInterceptor());
6364
defaultInterceptorList.add(
6465
new LoggingInterceptor(Logger.getLogger(GapicSpannerRpc.class.getName()), Level.FINER));
65-
defaultInterceptorList.add(
66-
new HeaderInterceptor(new SpannerRpcMetrics(openTelemetry), directPathEnabledSupplier));
66+
defaultInterceptorList.add(new HeaderInterceptor(new SpannerRpcMetrics(openTelemetry)));
6767
return new SpannerInterceptorProvider(ImmutableList.copyOf(defaultInterceptorList));
6868
}
6969

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

Lines changed: 8 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
import static org.junit.Assert.assertThrows;
2525
import static org.junit.Assert.assertTrue;
2626
import static org.junit.Assert.fail;
27-
import static org.junit.Assume.assumeFalse;
28-
import static org.junit.Assume.assumeTrue;
2927

3028
import com.google.api.gax.longrunning.OperationTimedPollAlgorithm;
3129
import com.google.api.gax.retrying.RetrySettings;
@@ -34,7 +32,6 @@
3432
import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
3533
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
3634
import com.google.cloud.spanner.connection.RandomResultSetGenerator;
37-
import com.google.cloud.spanner.spi.v1.GapicSpannerRpc;
3835
import com.google.common.base.Stopwatch;
3936
import com.google.common.collect.ImmutableList;
4037
import com.google.common.collect.Range;
@@ -51,7 +48,6 @@
5148
import io.opentelemetry.sdk.metrics.data.MetricData;
5249
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
5350
import java.io.IOException;
54-
import java.lang.reflect.Field;
5551
import java.net.InetSocketAddress;
5652
import java.time.Duration;
5753
import java.util.Collection;
@@ -195,19 +191,11 @@ public void testMetricsSingleUseQuery() {
195191
checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
196192
assertFalse(
197193
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-
}
194+
// AFE metrics are enabled for DirectPath.
195+
MetricData afeLatencyMetricData =
196+
getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
197+
double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
198+
assertEquals(fakeAFEServerTiming.get(), afeLatencyValue, 1e-6);
211199
}
212200

213201
private boolean isJava8() {
@@ -218,75 +206,6 @@ private boolean isWindows() {
218206
return System.getProperty("os.name").toLowerCase().contains("windows");
219207
}
220208

221-
@Test
222-
public void testMetricsSingleUseQueryWithAfeEnabled() throws Exception {
223-
assumeTrue(isJava8() && !isWindows());
224-
assumeFalse(System.getenv().containsKey("SPANNER_DISABLE_AFE_SERVER_TIMING"));
225-
226-
Class<?> classOfMap = System.getenv().getClass();
227-
Field field = classOfMap.getDeclaredField("m");
228-
field.setAccessible(true);
229-
Map<String, String> writeableEnvironmentVariables =
230-
(Map<String, String>) field.get(System.getenv());
231-
232-
try {
233-
writeableEnvironmentVariables.put("SPANNER_DISABLE_AFE_SERVER_TIMING", "false");
234-
235-
Stopwatch stopwatch = Stopwatch.createStarted();
236-
try (ResultSet resultSet = client.singleUse().executeQuery(SELECT_RANDOM)) {
237-
assertTrue(resultSet.next());
238-
assertFalse(resultSet.next());
239-
}
240-
241-
double elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);
242-
Attributes expectedAttributes =
243-
expectedCommonBaseAttributes.toBuilder()
244-
.putAll(expectedCommonRequestAttributes)
245-
.put(BuiltInMetricsConstant.STATUS_KEY, "OK")
246-
.put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteStreamingSql")
247-
.build();
248-
249-
MetricData operationLatencyMetricData =
250-
getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_LATENCIES_NAME);
251-
assertNotNull(operationLatencyMetricData);
252-
double operationLatencyValue =
253-
getAggregatedValue(operationLatencyMetricData, expectedAttributes);
254-
assertThat(operationLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
255-
256-
MetricData attemptLatencyMetricData =
257-
getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_LATENCIES_NAME);
258-
assertNotNull(attemptLatencyMetricData);
259-
double attemptLatencyValue = getAggregatedValue(attemptLatencyMetricData, expectedAttributes);
260-
assertThat(attemptLatencyValue).isIn(Range.closed(MIN_LATENCY, elapsed));
261-
262-
MetricData operationCountMetricData =
263-
getMetricData(metricReader, BuiltInMetricsConstant.OPERATION_COUNT_NAME);
264-
assertNotNull(operationCountMetricData);
265-
assertThat(getAggregatedValue(operationCountMetricData, expectedAttributes)).isEqualTo(1);
266-
267-
MetricData attemptCountMetricData =
268-
getMetricData(metricReader, BuiltInMetricsConstant.ATTEMPT_COUNT_NAME);
269-
assertNotNull(attemptCountMetricData);
270-
assertThat(getAggregatedValue(attemptCountMetricData, expectedAttributes)).isEqualTo(1);
271-
272-
assertFalse(
273-
checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME));
274-
assertFalse(
275-
checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME));
276-
MetricData afeLatencyMetricData =
277-
getMetricData(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME);
278-
double afeLatencyValue = getAggregatedValue(afeLatencyMetricData, expectedAttributes);
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);
285-
} finally {
286-
writeableEnvironmentVariables.remove("GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS");
287-
}
288-
}
289-
290209
@Test
291210
public void testMetricsWithGaxRetryUnaryRpc() {
292211
Stopwatch stopwatch = Stopwatch.createStarted();
@@ -454,17 +373,9 @@ public void testNoServerTimingHeader() throws IOException, InterruptedException
454373

455374
assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME));
456375
assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_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-
}
376+
MetricData afeConnectivityMetricData =
377+
getMetricData(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME);
378+
assertThat(getAggregatedValue(afeConnectivityMetricData, expectedAttributes)).isEqualTo(1);
468379

469380
spannerNoHeader.close();
470381
serverNoHeader.shutdown();

0 commit comments

Comments
 (0)