diff --git a/google-cloud-bigtable/clirr-ignored-differences.xml b/google-cloud-bigtable/clirr-ignored-differences.xml index 1ca5867295..4bb4684c38 100644 --- a/google-cloud-bigtable/clirr-ignored-differences.xml +++ b/google-cloud-bigtable/clirr-ignored-differences.xml @@ -145,4 +145,9 @@ com/google/cloud/bigtable/data/v2/stub/readrows/RowMerger * + + + 8001 + com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable + diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index c46539cddf..474c140392 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -87,7 +87,6 @@ import com.google.cloud.bigtable.data.v2.stub.changestream.GenerateInitialChangeStreamPartitionsUserCallable; import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamResumptionStrategy; import com.google.cloud.bigtable.data.v2.stub.changestream.ReadChangeStreamUserCallable; -import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerBatchedUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BigtableTracerUnaryCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory; @@ -509,11 +508,8 @@ private UnaryCallable> createBulkReadRowsCallable( UnaryCallable> tracedBatcher = new TracedBatcherUnaryCallable<>(readRowsUserCallable.all()); - UnaryCallable> withBigtableTracer = - new BigtableTracerBatchedUnaryCallable<>(tracedBatcher); - UnaryCallable> traced = - new TracedUnaryCallable<>(withBigtableTracer, clientContext.getTracerFactory(), span); + new TracedUnaryCallable<>(tracedBatcher, clientContext.getTracerFactory(), span); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -641,10 +637,9 @@ private UnaryCallable createBulkMutateRowsCallable() { UnaryCallable tracedBatcherUnaryCallable = new TracedBatcherUnaryCallable<>(userFacing); - UnaryCallable withBigtableTracer = - new BigtableTracerBatchedUnaryCallable<>(tracedBatcherUnaryCallable); UnaryCallable traced = - new TracedUnaryCallable<>(withBigtableTracer, clientContext.getTracerFactory(), spanName); + new TracedUnaryCallable<>( + tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -747,6 +742,9 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { ServerStreamingCallable convertException = new ConvertExceptionCallable<>(callable); + ServerStreamingCallable withBigtableTracer = + new BigtableTracerStreamingCallable<>(convertException); + RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>( new ApiResultRetryAlgorithm(), @@ -757,7 +755,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { return new MutateRowsRetryingCallable( clientContext.getDefaultCallContext(), - convertException, + withBigtableTracer, retryingExecutor, settings.bulkMutateRowsSettings().getRetryableCodes()); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java deleted file mode 100644 index 06722aaea5..0000000000 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerBatchedUnaryCallable.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.data.v2.stub.metrics; - -import com.google.api.core.ApiFuture; -import com.google.api.core.ApiFutures; -import com.google.api.core.InternalApi; -import com.google.api.gax.grpc.GrpcResponseMetadata; -import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.UnaryCallable; -import com.google.common.util.concurrent.MoreExecutors; -import javax.annotation.Nonnull; - -/** - * This callable will do everything described in {@link BigtableTracerUnaryCallable} except that it - * won't inject a {@link BigtableGrpcStreamTracer}. For batching calls, we only want to calculate - * the total time client is blocked because of flow control. - */ -@InternalApi -public class BigtableTracerBatchedUnaryCallable - extends BigtableTracerUnaryCallable { - - private UnaryCallable innerCallable; - - public BigtableTracerBatchedUnaryCallable( - @Nonnull UnaryCallable innerCallable) { - super(innerCallable); - this.innerCallable = innerCallable; - } - - @Override - public ApiFuture futureCall(RequestT request, ApiCallContext context) { - final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); - BigtableTracerUnaryCallback callback = - new BigtableTracerUnaryCallback( - (BigtableTracer) context.getTracer(), responseMetadata); - ApiFuture future = - innerCallable.futureCall(request, responseMetadata.addHandlers(context)); - ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); - return future; - } -} diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java index e5ec7b806b..7dfca8b753 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerUnaryCallable.java @@ -70,7 +70,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) } } - class BigtableTracerUnaryCallback implements ApiFutureCallback { + private class BigtableTracerUnaryCallback implements ApiFutureCallback { private final BigtableTracer tracer; private final GrpcResponseMetadata responseMetadata; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java index 181e0caaa7..c7a47942f2 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -491,12 +491,16 @@ public void testBatchBlockingLatencies() throws InterruptedException { verify(statsRecorderWrapper, timeout(1000).times(expectedNumRequests)) .putClientBlockingLatencies(throttledTime.capture()); - // Adding the first 2 elements should not get throttled since the batch is empty - assertThat(throttledTime.getAllValues().get(0)).isEqualTo(0); // After the first request is sent, batcher will block on add because of the server latency. // Blocking latency should be around server latency. assertThat(throttledTime.getAllValues().get(1)).isAtLeast(SERVER_LATENCY - 10); assertThat(throttledTime.getAllValues().get(2)).isAtLeast(SERVER_LATENCY - 10); + + verify(statsRecorderWrapper, timeout(100).times(expectedNumRequests)) + .recordAttempt(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); + + assertThat(zone.getAllValues()).containsExactly(ZONE, ZONE, ZONE); + assertThat(cluster.getAllValues()).containsExactly(CLUSTER, CLUSTER, CLUSTER); } }