diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index fd420f42457..21b2eb228cb 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -1062,4 +1062,34 @@ com/google/cloud/spanner/connection/Connection java.lang.Object getConnectionPropertyValue(com.google.cloud.spanner.connection.ConnectionProperty) + + + 7002 + com/google/cloud/spanner/CompositeTracer + void recordAFELatency(java.lang.Long) + + + 7002 + com/google/cloud/spanner/CompositeTracer + void recordAFELatency(java.lang.Float) + + + 7002 + com/google/cloud/spanner/CompositeTracer + void recordAfeHeaderMissingCount(java.lang.Long) + + + 7002 + com/google/cloud/spanner/CompositeTracer + void recordGFELatency(java.lang.Long) + + + 7002 + com/google/cloud/spanner/CompositeTracer + void recordGFELatency(java.lang.Float) + + 7002 + com/google/cloud/spanner/CompositeTracer + void recordGfeHeaderMissingCount(java.lang.Long) + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java index 2eb7c8d2971..67e75a1d383 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java @@ -99,21 +99,23 @@ class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder { void recordServerTimingHeaderMetrics( Float gfeLatency, Float afeLatency, - Long gfeHeaderMissingCount, - Long afeHeaderMissingCount, - Map attributes) { + Map attributes, + boolean isDirectPathUsed, + boolean isAfeEnabled) { io.opentelemetry.api.common.Attributes otelAttributes = toOtelAttributes(attributes); - if (gfeLatency != null) { - gfeLatencyRecorder.record(gfeLatency, otelAttributes); + if (!isDirectPathUsed) { + if (gfeLatency != null) { + gfeLatencyRecorder.record(gfeLatency, otelAttributes); + } else { + gfeHeaderMissingCountRecorder.add(1, otelAttributes); + } } - if (gfeHeaderMissingCount > 0) { - gfeHeaderMissingCountRecorder.add(gfeHeaderMissingCount, otelAttributes); - } - if (afeLatency != null) { - afeLatencyRecorder.record(afeLatency, otelAttributes); - } - if (afeHeaderMissingCount > 0) { - afeHeaderMissingCountRecorder.add(afeHeaderMissingCount, otelAttributes); + if (isAfeEnabled) { + if (afeLatency != null) { + afeLatencyRecorder.record(afeLatency, otelAttributes); + } else { + afeHeaderMissingCountRecorder.add(1, otelAttributes); + } } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java index cdf7dda2e6f..a982a3f11ab 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java @@ -39,10 +39,10 @@ class BuiltInMetricsTracer extends MetricsTracer implements ApiTracer { private final Map attributes = new HashMap<>(); private Float gfeLatency = null; private Float afeLatency = null; - private TraceWrapper traceWrapper; - private long gfeHeaderMissingCount = 0; - private long afeHeaderMissingCount = 0; + private final TraceWrapper traceWrapper; private final ISpan currentSpan; + private boolean isDirectPathUsed; + private boolean isAfeEnabled; BuiltInMetricsTracer( MethodName methodName, @@ -66,7 +66,7 @@ public void attemptSucceeded() { super.attemptSucceeded(); attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString()); builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics( - gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes); + gfeLatency, afeLatency, attributes, isDirectPathUsed, isAfeEnabled); } } @@ -80,7 +80,7 @@ public void attemptCancelled() { super.attemptCancelled(); attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString()); builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics( - gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes); + gfeLatency, afeLatency, attributes, isDirectPathUsed, isAfeEnabled); } } @@ -98,7 +98,7 @@ public void attemptFailedDuration(Throwable error, java.time.Duration delay) { super.attemptFailedDuration(error, delay); attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics( - gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes); + gfeLatency, afeLatency, attributes, isDirectPathUsed, isAfeEnabled); } } @@ -115,7 +115,7 @@ public void attemptFailedRetriesExhausted(Throwable error) { super.attemptFailedRetriesExhausted(error); attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics( - gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes); + gfeLatency, afeLatency, attributes, isDirectPathUsed, isAfeEnabled); } } @@ -132,24 +132,16 @@ public void attemptPermanentFailure(Throwable error) { super.attemptPermanentFailure(error); attributes.put(STATUS_ATTRIBUTE, extractStatus(error)); builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics( - gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes); + gfeLatency, afeLatency, attributes, isDirectPathUsed, isAfeEnabled); } } - void recordGFELatency(Float gfeLatency) { + public void recordServerTimingHeaderMetrics( + Float gfeLatency, Float afeLatency, boolean isDirectPathUsed, boolean isAfeEnabled) { this.gfeLatency = gfeLatency; - } - - void recordAFELatency(Float afeLatency) { + this.isDirectPathUsed = isDirectPathUsed; this.afeLatency = afeLatency; - } - - void recordGfeHeaderMissingCount(Long value) { - this.gfeHeaderMissingCount = value; - } - - void recordAfeHeaderMissingCount(Long value) { - this.afeHeaderMissingCount = value; + this.isAfeEnabled = isAfeEnabled; } @Override diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java index 5d3b416788f..105dbd0a512 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java @@ -191,50 +191,13 @@ public void addAttributes(Map attributes) { } } - public void recordGFELatency(Long gfeLatency) { + public void recordServerTimingHeaderMetrics( + Float gfeLatency, Float afeLatency, boolean isDirectPathUsed, boolean isAfeEnabled) { for (ApiTracer child : children) { if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordGFELatency(Float.valueOf(gfeLatency)); - } - } - } - - public void recordGfeHeaderMissingCount(Long value) { - for (ApiTracer child : children) { - if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordGfeHeaderMissingCount(value); - } - } - } - - public void recordAFELatency(Long afeLatency) { - for (ApiTracer child : children) { - if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordAFELatency(Float.valueOf(afeLatency)); - } - } - } - - public void recordAfeHeaderMissingCount(Long value) { - for (ApiTracer child : children) { - if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordAfeHeaderMissingCount(value); - } - } - } - - public void recordGFELatency(Float gfeLatency) { - for (ApiTracer child : children) { - if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordGFELatency(gfeLatency); - } - } - } - - public void recordAFELatency(Float afeLatency) { - for (ApiTracer child : children) { - if (child instanceof BuiltInMetricsTracer) { - ((BuiltInMetricsTracer) child).recordAFELatency(afeLatency); + ((BuiltInMetricsTracer) child) + .recordServerTimingHeaderMetrics( + gfeLatency, afeLatency, isDirectPathUsed, isAfeEnabled); } } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java index c89fedd7fe3..f6b0a4efd22 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java @@ -28,6 +28,9 @@ public class TransactionMutationLimitExceededException extends SpannerException private static final String ERROR_MESSAGE = "The transaction contains too many mutations."; + private static final String TRANSACTION_RESOURCE_LIMIT_EXCEEDED_MESSAGE = + "Transaction resource limits exceeded"; + /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ TransactionMutationLimitExceededException( DoNotConstructDirectly token, @@ -40,13 +43,17 @@ public class TransactionMutationLimitExceededException extends SpannerException } static boolean isTransactionMutationLimitException(ErrorCode code, String message) { - return code == ErrorCode.INVALID_ARGUMENT && message != null && message.contains(ERROR_MESSAGE); + return code == ErrorCode.INVALID_ARGUMENT + && message != null + && (message.contains(ERROR_MESSAGE) + || message.contains(TRANSACTION_RESOURCE_LIMIT_EXCEEDED_MESSAGE)); } static boolean isTransactionMutationLimitException(Throwable cause, ApiException apiException) { if (cause == null || cause.getMessage() == null - || !cause.getMessage().contains(ERROR_MESSAGE)) { + || !(cause.getMessage().contains(ERROR_MESSAGE) + || cause.getMessage().contains(TRANSACTION_RESOURCE_LIMIT_EXCEEDED_MESSAGE))) { return false; } // Spanner includes a hint that points to the Spanner limits documentation page when the error @@ -66,6 +73,9 @@ static boolean isTransactionMutationLimitException(Throwable cause, ApiException .getLinks(0) .getUrl() .equals("https://cloud.google.com/spanner/docs/limits"); + } else if (cause.getMessage().contains(TRANSACTION_RESOURCE_LIMIT_EXCEEDED_MESSAGE)) { + // This more generic error does not contain any additional details. + return true; } return false; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java index 0b02238a807..985ad68a429 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java @@ -28,14 +28,9 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.spanner.admin.database.v1.DatabaseName; -import io.grpc.CallOptions; -import io.grpc.Channel; -import io.grpc.ClientCall; -import io.grpc.ClientInterceptor; +import io.grpc.*; import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; import io.grpc.ForwardingClientCallListener.SimpleForwardingClientCallListener; -import io.grpc.Metadata; -import io.grpc.MethodDescriptor; import io.grpc.alts.AltsContextUtil; import io.opencensus.stats.MeasureMap; import io.opencensus.stats.Stats; @@ -91,6 +86,8 @@ class HeaderInterceptor implements ClientInterceptor { private static final Logger LOGGER = Logger.getLogger(HeaderInterceptor.class.getName()); private static final Level LEVEL = Level.INFO; private final SpannerRpcMetrics spannerRpcMetrics; + private Float gfeLatency; + private Float afeLatency; HeaderInterceptor(SpannerRpcMetrics spannerRpcMetrics) { this.spannerRpcMetrics = spannerRpcMetrics; @@ -113,24 +110,46 @@ public void start(Listener responseListener, Metadata headers) { TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName); Attributes attributes = getMetricAttributes(key, method.getFullMethodName(), databaseName); - Map builtInMetricsAttributes = - getBuiltInMetricAttributes(key, databaseName); - builtInMetricsAttributes.put(BuiltInMetricsConstant.REQUEST_ID_KEY.getKey(), requestId); - addBuiltInMetricAttributes(compositeTracer, builtInMetricsAttributes); - if (span != null) { - span.setAttribute(XGoogSpannerRequestId.REQUEST_ID, requestId); - } + super.start( new SimpleForwardingClientCallListener(responseListener) { @Override public void onHeaders(Metadata metadata) { - // Check if the call uses DirectPath by inspecting the ALTS context. - boolean isDirectPathUsed = AltsContextUtil.check(getAttributes()); - addDirectPathUsedAttribute(compositeTracer, isDirectPathUsed); - processHeader( - metadata, tagContext, attributes, span, compositeTracer, isDirectPathUsed); + String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); + try { + // Get gfe and afe Latency value + Map serverTimingMetrics = parseServerTimingHeader(serverTiming); + gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER); + afeLatency = serverTimingMetrics.get(AFE_TIMING_HEADER); + } catch (NumberFormatException e) { + LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming); + } + super.onHeaders(metadata); } + + @Override + public void onClose(Status status, Metadata trailers) { + // Record Built-in Metrics + boolean isDirectPathUsed = AltsContextUtil.check(getAttributes()); + boolean isAfeEnabled = GapicSpannerRpc.isEnableAFEServerTiming(); + recordSpan(span, requestId); + recordCustomMetrics(tagContext, attributes, isDirectPathUsed); + Map builtInMetricsAttributes = new HashMap<>(); + try { + builtInMetricsAttributes = getBuiltInMetricAttributes(key, databaseName); + } catch (ExecutionException e) { + LOGGER.log( + LEVEL, "Unable to get built-in metric attributes {}", e.getMessage()); + } + recordBuiltInMetrics( + compositeTracer, + builtInMetricsAttributes, + requestId, + isDirectPathUsed, + isAfeEnabled); + super.onClose(status, trailers); + } }, headers); } catch (ExecutionException executionException) { @@ -141,29 +160,12 @@ public void onHeaders(Metadata metadata) { }; } - private void processHeader( - Metadata metadata, - TagContext tagContext, - Attributes attributes, - Span span, - CompositeTracer compositeTracer, - boolean isDirectPathUsed) { + private void recordCustomMetrics( + TagContext tagContext, Attributes attributes, Boolean isDirectPathUsed) { + // Record OpenCensus and Custom OpenTelemetry Metrics MeasureMap measureMap = STATS_RECORDER.newMeasureMap(); - String serverTiming = metadata.get(SERVER_TIMING_HEADER_KEY); - try { - // Previous implementation parsed the GFE latency directly using: - // long latency = Long.parseLong(serverTiming.substring("gfet4t7; dur=".length())); - // This approach assumed the serverTiming header contained exactly one metric "gfet4t7". - // If additional metrics were introduced in the header, older versions of the library - // would fail to parse it correctly. To make the parsing more robust, the logic has been - // updated to handle multiple metrics gracefully. - - Map serverTimingMetrics = parseServerTimingHeader(serverTiming); - Float gfeLatency = serverTimingMetrics.get(GFE_TIMING_HEADER); - boolean isAfeEnabled = GapicSpannerRpc.isEnableAFEServerTiming(); - Float afeLatency = isAfeEnabled ? serverTimingMetrics.get(AFE_TIMING_HEADER) : null; - // Record OpenCensus and Custom OpenTelemetry Metrics + if (!isDirectPathUsed) { if (gfeLatency != null) { long gfeVal = gfeLatency.longValue(); measureMap.put(SPANNER_GFE_LATENCY, gfeVal); @@ -174,39 +176,35 @@ private void processHeader( measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L); spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes); } - measureMap.record(tagContext); + } + measureMap.record(tagContext); + } - // Record Built-in Metrics - if (compositeTracer != null) { - // GFE Latency Metrics - if (!isDirectPathUsed) { - if (gfeLatency != null) { - compositeTracer.recordGFELatency(gfeLatency); - } else { - compositeTracer.recordGfeHeaderMissingCount(1L); - } - } - // AFE Tracing - if (isAfeEnabled) { - if (afeLatency != null) { - compositeTracer.recordAFELatency(afeLatency); - } else { - compositeTracer.recordAfeHeaderMissingCount(1L); - } - } + private void recordSpan(Span span, String requestId) { + if (span != null) { + if (gfeLatency != null) { + span.setAttribute("gfe_latency", gfeLatency.toString()); } - - // Record Span Attributes - if (span != null) { - if (gfeLatency != null) { - span.setAttribute("gfe_latency", gfeLatency.toString()); - } - if (afeLatency != null) { - span.setAttribute("afe_latency", afeLatency.toString()); - } + if (afeLatency != null) { + span.setAttribute("afe_latency", afeLatency.toString()); } - } catch (NumberFormatException e) { - LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming); + span.setAttribute(XGoogSpannerRequestId.REQUEST_ID, requestId); + } + } + + private void recordBuiltInMetrics( + CompositeTracer compositeTracer, + Map builtInMetricsAttributes, + String requestId, + Boolean isDirectPathUsed, + Boolean isAfeEnabled) { + if (compositeTracer != null) { + builtInMetricsAttributes.put(BuiltInMetricsConstant.REQUEST_ID_KEY.getKey(), requestId); + builtInMetricsAttributes.put( + BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), Boolean.toString(isDirectPathUsed)); + compositeTracer.addAttributes(builtInMetricsAttributes); + compositeTracer.recordServerTimingHeaderMetrics( + gfeLatency, afeLatency, isDirectPathUsed, isAfeEnabled); } } @@ -309,19 +307,4 @@ private Map getBuiltInMetricAttributes(String key, DatabaseName return attributes; }); } - - private void addBuiltInMetricAttributes( - CompositeTracer compositeTracer, Map builtInMetricsAttributes) { - if (compositeTracer != null) { - compositeTracer.addAttributes(builtInMetricsAttributes); - } - } - - private void addDirectPathUsedAttribute( - CompositeTracer compositeTracer, Boolean isDirectPathUsed) { - if (compositeTracer != null) { - compositeTracer.addAttributes( - BuiltInMetricsConstant.DIRECT_PATH_USED_KEY.getKey(), Boolean.toString(isDirectPathUsed)); - } - } } 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 b5a62ba43b6..ce1bb87ed4a 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 @@ -69,21 +69,17 @@ public class OpenTelemetryBuiltInMetricsTracerTest extends AbstractNettyMockServ private static final Statement SELECT_RANDOM = Statement.of("SELECT * FROM random"); private static final Statement UPDATE_RANDOM = Statement.of("UPDATE random SET foo=1 WHERE id=1"); private static InMemoryMetricReader metricReader; - private static Map attributes = + private static final Map attributes = BuiltInMetricsProvider.INSTANCE.createClientAttributes(); - private static Attributes expectedCommonBaseAttributes = + private static final Attributes expectedCommonBaseAttributes = Attributes.builder() .put(BuiltInMetricsConstant.CLIENT_NAME_KEY, "spanner-java/") .put(BuiltInMetricsConstant.CLIENT_UID_KEY, attributes.get("client_uid")) .put(BuiltInMetricsConstant.INSTANCE_ID_KEY, "i") .put(BuiltInMetricsConstant.DATABASE_KEY, "d") .put(BuiltInMetricsConstant.DIRECT_PATH_ENABLED_KEY, "false") + .put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false") .build(); - ; - private static Attributes expectedCommonRequestAttributes = - Attributes.builder().put(BuiltInMetricsConstant.DIRECT_PATH_USED_KEY, "false").build(); - ; - private static final double MIN_LATENCY = 0; private DatabaseClient client; @@ -175,7 +171,6 @@ public void testMetricsSingleUseQuery() { double elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); Attributes expectedAttributes = expectedCommonBaseAttributes.toBuilder() - .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteStreamingSql") .build(); @@ -237,7 +232,6 @@ public void testMetricsWithGaxRetryUnaryRpc() { Attributes expectedAttributesBeginTransactionOK = expectedCommonBaseAttributes.toBuilder() - .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.BeginTransaction") .build(); @@ -323,7 +317,6 @@ public void testNoNetworkConnection() { Attributes expectedAttributesCreateSessionOK = expectedCommonBaseAttributes.toBuilder() - .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.CreateSession") // Include the additional attributes that are added by the HeaderInterceptor in the @@ -347,6 +340,10 @@ public void testNoNetworkConnection() { // Attempt count should have a failed metric point for CreateSession. assertEquals( 1, getAggregatedValue(attemptCountMetricData, expectedAttributesCreateSessionFailed), 0); + assertTrue( + checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME)); + assertTrue( + checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME)); } @Test @@ -383,13 +380,16 @@ public void testNoServerTimingHeader() throws IOException, InterruptedException Attributes expectedAttributes = expectedCommonBaseAttributes.toBuilder() - .putAll(expectedCommonRequestAttributes) .put(BuiltInMetricsConstant.STATUS_KEY, "OK") .put(BuiltInMetricsConstant.METHOD_KEY, "Spanner.ExecuteSql") .build(); assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_LATENCIES_NAME)); assertFalse(checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_LATENCIES_NAME)); + assertTrue( + checkIfMetricExists(metricReader, BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME)); + assertTrue( + checkIfMetricExists(metricReader, BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME)); spannerNoHeader.close(); serverNoHeader.shutdown(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RetryDmlAsPartitionedDmlMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RetryDmlAsPartitionedDmlMockServerTest.java index cce6b7fe083..d5e44fdcefc 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RetryDmlAsPartitionedDmlMockServerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/RetryDmlAsPartitionedDmlMockServerTest.java @@ -43,10 +43,37 @@ import io.grpc.StatusRuntimeException; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; -@RunWith(JUnit4.class) +@RunWith(Parameterized.class) public class RetryDmlAsPartitionedDmlMockServerTest extends AbstractMockServerTest { + private enum ExceptionType { + MutationLimitExceeded { + @Override + StatusRuntimeException createException() { + return createTransactionMutationLimitExceededException(); + } + }, + ResourceLimitExceeded { + @Override + StatusRuntimeException createException() { + return createTransactionResourceLimitExceededException(); + } + }; + + abstract StatusRuntimeException createException(); + } + + @Parameters(name = "exception = {0}") + public static Object[] data() { + return ExceptionType.values(); + } + + @SuppressWarnings("ClassEscapesDefinedScope") + @Parameter + public ExceptionType exceptionType; static StatusRuntimeException createTransactionMutationLimitExceededException() { Metadata.Key key = @@ -70,10 +97,16 @@ static StatusRuntimeException createTransactionMutationLimitExceededException() .asRuntimeException(trailers); } + static StatusRuntimeException createTransactionResourceLimitExceededException() { + return Status.INVALID_ARGUMENT + .withDescription("Transaction resource limits exceeded") + .asRuntimeException(); + } + @Test public void testTransactionMutationLimitExceeded_isNotRetriedByDefault() { mockSpanner.setExecuteSqlExecutionTime( - SimulatedExecutionTime.ofException(createTransactionMutationLimitExceededException())); + SimulatedExecutionTime.ofException(exceptionType.createException())); try (Connection connection = createConnection()) { connection.setAutocommit(true); @@ -95,7 +128,7 @@ public void testTransactionMutationLimitExceeded_isNotRetriedByDefault() { public void testTransactionMutationLimitExceeded_canBeRetriedAsPDML() { Statement statement = Statement.of("update test set value=1 where true"); mockSpanner.setExecuteSqlExecutionTime( - SimulatedExecutionTime.ofException(createTransactionMutationLimitExceededException())); + SimulatedExecutionTime.ofException(exceptionType.createException())); mockSpanner.putStatementResult( MockSpannerServiceImpl.StatementResult.update(statement, 100000L)); @@ -134,7 +167,7 @@ public void testTransactionMutationLimitExceeded_retryAsPDMLFails() { Statement statement = Statement.of("insert into test (id, value) select -id, value from test"); // The transactional update statement uses ExecuteSql(..). mockSpanner.setExecuteSqlExecutionTime( - SimulatedExecutionTime.ofException(createTransactionMutationLimitExceededException())); + SimulatedExecutionTime.ofException(exceptionType.createException())); mockSpanner.putStatementResult( MockSpannerServiceImpl.StatementResult.exception( statement, @@ -230,7 +263,7 @@ public void testTransactionMutationLimitExceeded_isWrappedAsCauseOfBatchUpdateEx Statement statement = Statement.of(sql); mockSpanner.putStatementResult( MockSpannerServiceImpl.StatementResult.exception( - statement, createTransactionMutationLimitExceededException())); + statement, exceptionType.createException())); try (Connection connection = createConnection()) { connection.setAutocommit(true); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GfeLatencyTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GfeLatencyTest.java index 5067eb09ff9..2f75a251fec 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GfeLatencyTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GfeLatencyTest.java @@ -17,8 +17,7 @@ package com.google.cloud.spanner.spi.v1; import static com.google.cloud.spanner.DisableDefaultMtlsProvider.disableDefaultMtlsProvider; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.*; import com.google.auth.oauth2.AccessToken; import com.google.auth.oauth2.OAuth2Credentials; @@ -55,10 +54,7 @@ import java.util.Random; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.*; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -259,7 +255,7 @@ public void testGfeMissingHeaderCountExecuteStreamingSql() throws InterruptedExc SpannerRpcViews.SPANNER_GFE_HEADER_MISSING_COUNT_VIEW, "google.spanner.v1.Spanner/ExecuteStreamingSql", true); - assertEquals(1, count1); + assertTrue(count1 >= 1); } @Test @@ -270,7 +266,7 @@ public void testGfeMissingHeaderExecuteSql() throws InterruptedException { long count = getMetric( SpannerRpcViews.SPANNER_GFE_HEADER_MISSING_COUNT_VIEW, - "google.spanner.v1.Spanner/ExecuteSql", + "google.spanner.v1.Spanner/Commit", false); assertEquals(0, count); @@ -280,7 +276,7 @@ public void testGfeMissingHeaderExecuteSql() throws InterruptedException { long count1 = getMetric( SpannerRpcViews.SPANNER_GFE_HEADER_MISSING_COUNT_VIEW, - "google.spanner.v1.Spanner/ExecuteSql", + "google.spanner.v1.Spanner/Commit", true); assertEquals(1, count1); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java index 89c874fe204..95faea16f62 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java @@ -50,10 +50,7 @@ import java.util.Random; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.*; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -73,6 +70,7 @@ public class SpannerRpcMetricsTest { private static DatabaseClient databaseClientNoHeader; private static String instanceId = "fake-instance"; private static String databaseId = "fake-database"; + private static String noHeaderdatabaseId = "fake-database-1"; private static String projectId = "fake-project"; private static AtomicInteger fakeServerTiming = new AtomicInteger(new Random().nextInt(1000) + 1); private static final Statement SELECT1AND2 = @@ -183,7 +181,7 @@ public void sendHeaders(Metadata headers) { createSpannerOptions(addressNoHeader, serverNoHeader).getService(); databaseClientNoHeader = spannerNoHeaderNoOpenTelemetry.getDatabaseClient( - DatabaseId.of(projectId, instanceId, databaseId)); + DatabaseId.of(projectId, instanceId, noHeaderdatabaseId)); } @AfterClass @@ -228,7 +226,8 @@ public void testGfeMissingHeaderExecuteSqlWithGlobalOpenTelemetry() throws Inter long count = getHeaderLatencyMetric( getMetricData("spanner/gfe_header_missing_count", inMemoryMetricReaderInjected), - "google.spanner.v1.Spanner/ExecuteSql"); + "google.spanner.v1.Spanner/Commit", + databaseId); assertEquals(0, count); databaseClientNoHeader @@ -237,7 +236,8 @@ public void testGfeMissingHeaderExecuteSqlWithGlobalOpenTelemetry() throws Inter long count1 = getHeaderLatencyMetric( getMetricData("spanner/gfe_header_missing_count", inMemoryMetricReader), - "google.spanner.v1.Spanner/ExecuteSql"); + "google.spanner.v1.Spanner/Commit", + noHeaderdatabaseId); assertEquals(1, count1); } @@ -273,9 +273,12 @@ private static SpannerOptions createSpannerOptions(InetSocketAddress address, Se .build(); } - private long getHeaderLatencyMetric(MetricData metricData, String methodName) { + private long getHeaderLatencyMetric(MetricData metricData, String methodName, String databaseId) { return metricData.getLongSumData().getPoints().stream() - .filter(x -> x.getAttributes().asMap().containsValue(methodName)) + .filter( + x -> + x.getAttributes().asMap().containsValue(methodName) + && x.getAttributes().asMap().containsValue(databaseId)) .findFirst() .get() .getValue();