From 7fe99330a7a21a7a3f6e251df937833ab3140a42 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:07:03 -0400 Subject: [PATCH 01/41] Rebase on HEAD --- .../v2/stub/metrics/BigtableGrpcStreamTracer.java | 10 +++++++--- .../bigtable/data/v2/stub/metrics/BigtableTracer.java | 8 ++++++++ .../data/v2/stub/metrics/BuiltinMetricsConstants.java | 2 ++ .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 9 +++++++++ .../v2/stub/metrics/BuiltinMetricsTracerFactory.java | 11 +++++++++++ .../cloud/bigtable/data/v2/stub/metrics/Util.java | 3 ++- 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 80fcdd0419..230fb894d4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -26,9 +26,11 @@ class BigtableGrpcStreamTracer extends ClientStreamTracer { private final BigtableTracer tracer; + private final Deadline deadline; - public BigtableGrpcStreamTracer(BigtableTracer tracer) { + public BigtableGrpcStreamTracer(BigtableTracer tracer, Deadline deadline) { this.tracer = tracer; + this.deadline = deadline; } @Override @@ -39,15 +41,17 @@ public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalU static class Factory extends ClientStreamTracer.Factory { private final BigtableTracer tracer; + private final Deadline deadline; - Factory(BigtableTracer tracer) { + Factory(BigtableTracer tracer, Deadline deadline) { this.tracer = tracer; + this.deadline = deadline; } @Override public ClientStreamTracer newClientStreamTracer( ClientStreamTracer.StreamInfo info, Metadata headers) { - return new BigtableGrpcStreamTracer(tracer); + return new BigtableGrpcStreamTracer(tracer, deadline); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index d0e307d510..e10e758837 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -93,4 +93,12 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { public void grpcMessageSent() { // noop } + + /** + * Set the remaining customer specified deadline so it can be exported in a metric. This will + * be called in BuiltinMetricsTracer. + */ + public void setRemainingDeadline(long deadlineRemaining) { + // noop + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index d85300828b..02638549ef 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -58,6 +58,8 @@ public class BuiltinMetricsConstants { static final String SERVER_LATENCIES_NAME = "server_latencies"; static final String FIRST_RESPONSE_LATENCIES_NAME = "first_response_latencies"; static final String APPLICATION_BLOCKING_LATENCIES_NAME = "application_latencies"; + + static final String REMAINING_DEADLINE_NAME = "remaining_deadline"; static final String CLIENT_BLOCKING_LATENCIES_NAME = "throttling_latencies"; static final String PER_CONNECTION_ERROR_COUNT_NAME = "per_connection_error_count"; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 7a3f54913e..dea6a86eca 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -96,6 +96,8 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; + + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -109,6 +111,7 @@ class BuiltinMetricsTracer extends BigtableTracer { DoubleHistogram firstResponseLatenciesHistogram, DoubleHistogram clientBlockingLatenciesHistogram, DoubleHistogram applicationBlockingLatenciesHistogram, + DoubleHistogram remainingDeadlineHistogram, LongCounter connectivityErrorCounter, LongCounter retryCounter) { this.operationType = operationType; @@ -121,6 +124,7 @@ class BuiltinMetricsTracer extends BigtableTracer { this.firstResponseLatenciesHistogram = firstResponseLatenciesHistogram; this.clientBlockingLatenciesHistogram = clientBlockingLatenciesHistogram; this.applicationBlockingLatenciesHistogram = applicationBlockingLatenciesHistogram; + this.remainingDeadlineHistogram = remainingDeadlineHistogram; this.connectivityErrorCounter = connectivityErrorCounter; this.retryCounter = retryCounter; } @@ -268,6 +272,11 @@ public void grpcMessageSent() { grpcMessageSentDelay.set(attemptTimer.elapsed(TimeUnit.NANOSECONDS)); } + @Override + public void setRemainingDeadline(long deadlineRemaining) { + // update remaining deadline variable + } + @Override public void disableFlowControl() { flowControlIsDisabled = true; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index f0ac656978..0f4aaa224f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -22,6 +22,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.FIRST_RESPONSE_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; @@ -55,6 +56,8 @@ public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; + + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -108,6 +111,13 @@ public static BuiltinMetricsTracerFactory create( "The latency of the client application consuming available response data.") .setUnit(MILLISECOND) .build(); + remainingDeadlineHistogram = + meter + .histogramBuilder(REMAINING_DEADLINE_NAME) + .setDescription( + "The remaining customer specified deadline at the end of the request.") + .setUnit(MILLISECOND) + .build(); connectivityErrorCounter = meter .counterBuilder(CONNECTIVITY_ERROR_COUNT_NAME) @@ -135,6 +145,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op firstResponseLatenciesHistogram, clientBlockingLatenciesHistogram, applicationBlockingLatenciesHistogram, + remainingDeadlineHistogram, connectivityErrorCounter, retryCounter); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 4c3fd7a42d..fad955e3c4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -222,9 +222,10 @@ static GrpcCallContext injectBigtableStreamTracer( if (context instanceof GrpcCallContext) { GrpcCallContext callContext = (GrpcCallContext) context; CallOptions callOptions = callContext.getCallOptions(); + Deadline deadline = callOptions.getDeadline(); return responseMetadata.addHandlers( callContext.withCallOptions( - callOptions.withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer)))); + callOptions.withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer, deadline)))); } else { // context should always be an instance of GrpcCallContext. If not throw an exception // so we can see what class context is. From bd36596dcf67e88de5740f8c096fdb1a85fdfa9a Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 4 Oct 2024 12:26:40 -0400 Subject: [PATCH 02/41] Add override to CompositeTracer --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 3 +++ .../bigtable/data/v2/stub/metrics/CompositeTracer.java | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index dea6a86eca..9aa4352434 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -86,6 +86,8 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); + private long deadlineRemaining; + // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them // to milliseconds and use DoubleHistogram. This should minimize the chance of a data @@ -275,6 +277,7 @@ public void grpcMessageSent() { @Override public void setRemainingDeadline(long deadlineRemaining) { // update remaining deadline variable + this.deadlineRemaining = deadlineRemaining; } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index d89aa90c6b..d0410989ea 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -227,9 +227,15 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { } @Override +<<<<<<< HEAD public void grpcMessageSent() { for (BigtableTracer tracer : bigtableTracers) { tracer.grpcMessageSent(); +======= + public void setRemainingDeadline(long deadlineRemaining) { + for (BigtableTracer tracer : bigtableTracers) { + tracer.setRemainingDeadline(deadlineRemaining); +>>>>>>> 0ad9a399 (Add override to CompositeTracer) } } } From 965e4bd049fe3225d695b04bb8a77405db8b0458 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 4 Oct 2024 12:30:52 -0400 Subject: [PATCH 03/41] Remove blank lines --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java | 1 - .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 1 - 2 files changed, 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index 02638549ef..a9c968de12 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -58,7 +58,6 @@ public class BuiltinMetricsConstants { static final String SERVER_LATENCIES_NAME = "server_latencies"; static final String FIRST_RESPONSE_LATENCIES_NAME = "first_response_latencies"; static final String APPLICATION_BLOCKING_LATENCIES_NAME = "application_latencies"; - static final String REMAINING_DEADLINE_NAME = "remaining_deadline"; static final String CLIENT_BLOCKING_LATENCIES_NAME = "throttling_latencies"; static final String PER_CONNECTION_ERROR_COUNT_NAME = "per_connection_error_count"; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 9aa4352434..47bc5c90ac 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -98,7 +98,6 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; From 2068604cceb3bcf925beb1a1cfc3811e3024f0cf Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 4 Oct 2024 12:56:47 -0400 Subject: [PATCH 04/41] Fix missing imports --- .../bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java | 1 + .../com/google/cloud/bigtable/data/v2/stub/metrics/Util.java | 1 + 2 files changed, 2 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 230fb894d4..1f4c946c64 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import io.grpc.ClientStreamTracer; +import io.grpc.Deadline; import io.grpc.Metadata; /** diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index fad955e3c4..abd15179cd 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -34,6 +34,7 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.InvalidProtocolBufferException; import io.grpc.CallOptions; +import io.grpc.Deadline; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusException; From affb8523abb341341ebb29b1572ab72ebb0e0600 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 7 Oct 2024 11:01:09 -0400 Subject: [PATCH 05/41] Only record remaining deadline if it is not null --- .../metrics/BigtableGrpcStreamTracer.java | 7 ++++++ .../metrics/BuiltinMetricsTracerTest.java | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 1f4c946c64..19f0247f3a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -36,7 +36,14 @@ public BigtableGrpcStreamTracer(BigtableTracer tracer, Deadline deadline) { @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { +<<<<<<< HEAD tracer.grpcMessageSent(); +======= + tracer.grpcChannelQueuedLatencies(stopwatch.elapsed(TimeUnit.NANOSECONDS)); + if (deadline != null) { + tracer.setRemainingDeadline(deadline.timeRemaining(TimeUnit.MILLISECONDS)); + } +>>>>>>> 2651506d (Only record remaining deadline if it is not null) } static class Factory extends ClientStreamTracer.Factory { 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 ba300f502d..66b4019ada 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 @@ -23,6 +23,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.CONNECTIVITY_ERROR_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METHOD_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.STATUS_KEY; @@ -42,10 +43,12 @@ import com.google.api.gax.batching.BatchingSettings; import com.google.api.gax.batching.FlowControlSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; +import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.NotFoundException; import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.StreamController; +import com.google.api.gax.rpc.testing.FakeCallContext; import com.google.bigtable.v2.BigtableGrpc; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; @@ -702,6 +705,27 @@ public void testPermanentFailure() { verifyAttributes(opLatency, expected); } + @Test + public void testRemainingDeadline() { +// stub.readRowsCallable().all().call(Query.create(TABLE), FakeCallContext.create()); + + MetricData remainingDeadline = getMetricData(metricReader, REMAINING_DEADLINE_NAME); + + Attributes attributes = + baseAttributes + .toBuilder() + .put(TABLE_ID_KEY, TABLE) + .put(CLUSTER_ID_KEY, CLUSTER) + .put(ZONE_ID_KEY, ZONE) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); + + long value = getAggregatedValue(remainingDeadline, attributes); + System.out.println("DEADLINE = "); + System.out.println(value); + } + private static class FakeService extends BigtableGrpc.BigtableImplBase { static List createFakeResponse() { From 67fe252a92ecd30f54fdadae6d7d1e069c7c2896 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 7 Oct 2024 11:12:59 -0400 Subject: [PATCH 06/41] Add remaining deadline to list of metrics in BigtableCloudMonitoringExporter --- .../data/v2/stub/metrics/BigtableCloudMonitoringExporter.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index fd54313e8d..27fdd75995 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -22,6 +22,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.FIRST_RESPONSE_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; @@ -115,7 +116,8 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { CLIENT_BLOCKING_LATENCIES_NAME, APPLICATION_BLOCKING_LATENCIES_NAME, RETRY_COUNT_NAME, - CONNECTIVITY_ERROR_COUNT_NAME) + CONNECTIVITY_ERROR_COUNT_NAME, + REMAINING_DEADLINE_NAME) .stream() .map(m -> METER_NAME + m) .collect(ImmutableList.toImmutableList()); From 64908fba12226ab84feb9a619cb7ce5c7a28066d Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 7 Oct 2024 11:42:14 -0400 Subject: [PATCH 07/41] Export remainingDeadline histogram --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 47bc5c90ac..40bb3b4ccf 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -321,6 +321,8 @@ private void recordOperationCompletion(@Nullable Throwable status) { long applicationLatencyNano = operationLatencyNano - totalServerLatencyNano.get(); applicationBlockingLatenciesHistogram.record(convertToMs(applicationLatencyNano), attributes); + remainingDeadlineHistogram.record(convertToMs(deadlineRemaining), attributes); + if (operationType == OperationType.ServerStreaming && spanName.getMethodName().equals("ReadRows")) { firstResponseLatenciesHistogram.record( From 804d2237a22d7eb6e15422fd2b534c04e3475a52 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Tue, 8 Oct 2024 12:29:08 -0400 Subject: [PATCH 08/41] Fix unit test --- .../stub/metrics/BuiltinMetricsConstants.java | 10 +++++++ .../v2/stub/metrics/BuiltinMetricsTracer.java | 2 +- .../metrics/BuiltinMetricsTracerTest.java | 30 ++++++++++++++----- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index a9c968de12..58a70f5254 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -215,6 +215,16 @@ public static Map getAllViews() { ImmutableSet.builder() .add(BIGTABLE_PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY) .build()); + defineView( + views, + REMAINING_DEADLINE_NAME, + AGGREGATION_WITH_MILLIS_HISTOGRAM, + InstrumentType.HISTOGRAM, + "ms", + ImmutableSet.builder() + .addAll(COMMON_ATTRIBUTES) + .add(STREAMING_KEY, STATUS_KEY) + .build()); return views.build(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 40bb3b4ccf..fa8a023ed3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -321,7 +321,7 @@ private void recordOperationCompletion(@Nullable Throwable status) { long applicationLatencyNano = operationLatencyNano - totalServerLatencyNano.get(); applicationBlockingLatenciesHistogram.record(convertToMs(applicationLatencyNano), attributes); - remainingDeadlineHistogram.record(convertToMs(deadlineRemaining), attributes); + remainingDeadlineHistogram.record(deadlineRemaining, attributes); if (operationType == OperationType.ServerStreaming && spanName.getMethodName().equals("ReadRows")) { 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 66b4019ada..db9650299d 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 @@ -42,13 +42,13 @@ import com.google.api.gax.batching.BatchingException; import com.google.api.gax.batching.BatchingSettings; import com.google.api.gax.batching.FlowControlSettings; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.NotFoundException; import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.StreamController; -import com.google.api.gax.rpc.testing.FakeCallContext; import com.google.bigtable.v2.BigtableGrpc; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; @@ -73,6 +73,15 @@ import com.google.protobuf.ByteString; import com.google.protobuf.BytesValue; import com.google.protobuf.StringValue; +<<<<<<< HEAD +======= +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ClientCall; +import io.grpc.ClientInterceptor; +import io.grpc.Deadline; +import io.grpc.ForwardingClientCall; +>>>>>>> 685c3d6b (Fix unit test) import io.grpc.ForwardingServerCall; import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; @@ -97,6 +106,7 @@ import java.io.IOException; import java.net.SocketAddress; import java.nio.charset.Charset; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; @@ -659,6 +669,7 @@ public void testQueuedOnChannelServerStreamLatencies() { @Test public void testQueuedOnChannelUnaryLatencies() { + System.out.print("HELLO WORLD2"); stub.mutateRowCallable().call(RowMutation.create(TABLE, "a-key").setCell("f", "q", "v")); MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); @@ -707,23 +718,26 @@ public void testPermanentFailure() { @Test public void testRemainingDeadline() { -// stub.readRowsCallable().all().call(Query.create(TABLE), FakeCallContext.create()); - - MetricData remainingDeadline = getMetricData(metricReader, REMAINING_DEADLINE_NAME); + GrpcCallContext context = GrpcCallContext.createDefault(); + stub.readRowsCallable().all().call(Query.create(TABLE), + context.withCallOptions(context.getCallOptions().withDeadlineAfter(1000, TimeUnit.MILLISECONDS))); + MetricData remainingDeadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); Attributes attributes = baseAttributes .toBuilder() + .put(STATUS_KEY, "OK") .put(TABLE_ID_KEY, TABLE) - .put(CLUSTER_ID_KEY, CLUSTER) .put(ZONE_ID_KEY, ZONE) + .put(CLUSTER_ID_KEY, CLUSTER) .put(METHOD_KEY, "Bigtable.ReadRows") + .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - long value = getAggregatedValue(remainingDeadline, attributes); - System.out.println("DEADLINE = "); - System.out.println(value); + long remainingDeadline = getAggregatedValue(remainingDeadlineMetric, attributes); + System.out.println("DEADLINE = " + remainingDeadline); + assertThat(remainingDeadline).isIn(Range.closed((long) 500, (long) 800)); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 585c64c34564feb6cfc00d96c7897e5bc3544d91 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Tue, 8 Oct 2024 12:36:18 -0400 Subject: [PATCH 09/41] Remove print statement --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java | 1 - 1 file changed, 1 deletion(-) 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 db9650299d..f2ee96736e 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 @@ -669,7 +669,6 @@ public void testQueuedOnChannelServerStreamLatencies() { @Test public void testQueuedOnChannelUnaryLatencies() { - System.out.print("HELLO WORLD2"); stub.mutateRowCallable().call(RowMutation.create(TABLE, "a-key").setCell("f", "q", "v")); MetricData clientLatency = getMetricData(metricReader, CLIENT_BLOCKING_LATENCIES_NAME); From ff7c311b9717895ff0e60b91d68e5499e110a0e6 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:18:23 -0400 Subject: [PATCH 10/41] Fix merge conflict --- .../data/v2/stub/metrics/BigtableGrpcStreamTracer.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 19f0247f3a..670befac29 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -36,14 +36,10 @@ public BigtableGrpcStreamTracer(BigtableTracer tracer, Deadline deadline) { @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { -<<<<<<< HEAD tracer.grpcMessageSent(); -======= - tracer.grpcChannelQueuedLatencies(stopwatch.elapsed(TimeUnit.NANOSECONDS)); if (deadline != null) { tracer.setRemainingDeadline(deadline.timeRemaining(TimeUnit.MILLISECONDS)); } ->>>>>>> 2651506d (Only record remaining deadline if it is not null) } static class Factory extends ClientStreamTracer.Factory { From a336fa4076f996fc54af04e1ccf59b191db727d4 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:28:08 -0400 Subject: [PATCH 11/41] Use deadline from callContext, not callOptions --- .../v2/stub/metrics/BigtableGrpcStreamTracer.java | 14 ++++++-------- .../data/v2/stub/metrics/BigtableTracer.java | 2 +- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 11 ++++------- .../data/v2/stub/metrics/CompositeTracer.java | 10 ++-------- .../cloud/bigtable/data/v2/stub/metrics/Util.java | 5 +++-- 5 files changed, 16 insertions(+), 26 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 670befac29..6278c7a9c3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -16,8 +16,8 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import io.grpc.ClientStreamTracer; -import io.grpc.Deadline; import io.grpc.Metadata; +import org.threeten.bp.Duration; /** * Records the time a request is enqueued in a grpc channel queue. This a bridge between gRPC stream @@ -27,9 +27,9 @@ class BigtableGrpcStreamTracer extends ClientStreamTracer { private final BigtableTracer tracer; - private final Deadline deadline; + private final Duration deadline; - public BigtableGrpcStreamTracer(BigtableTracer tracer, Deadline deadline) { + public BigtableGrpcStreamTracer(BigtableTracer tracer, Duration deadline) { this.tracer = tracer; this.deadline = deadline; } @@ -37,17 +37,15 @@ public BigtableGrpcStreamTracer(BigtableTracer tracer, Deadline deadline) { @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { tracer.grpcMessageSent(); - if (deadline != null) { - tracer.setRemainingDeadline(deadline.timeRemaining(TimeUnit.MILLISECONDS)); - } + tracer.setRemainingDeadline(deadline.toMillis()); } static class Factory extends ClientStreamTracer.Factory { private final BigtableTracer tracer; - private final Deadline deadline; + private final Duration deadline; - Factory(BigtableTracer tracer, Deadline deadline) { + Factory(BigtableTracer tracer, Duration deadline) { this.tracer = tracer; this.deadline = deadline; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index e10e758837..c6f6636619 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -98,7 +98,7 @@ public void grpcMessageSent() { * Set the remaining customer specified deadline so it can be exported in a metric. This will * be called in BuiltinMetricsTracer. */ - public void setRemainingDeadline(long deadlineRemaining) { + public void setRemainingDeadline(long deadline) { // noop } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index fa8a023ed3..d4b1ddd9e1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -86,8 +86,6 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); - private long deadlineRemaining; - // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them // to milliseconds and use DoubleHistogram. This should minimize the chance of a data @@ -98,6 +96,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -274,9 +273,9 @@ public void grpcMessageSent() { } @Override - public void setRemainingDeadline(long deadlineRemaining) { - // update remaining deadline variable - this.deadlineRemaining = deadlineRemaining; + public void setRemainingDeadline(long deadline) { + long timeElapsed = attemptTimer.elapsed(TimeUnit.MILLISECONDS); + long deadlineRemaining = deadline - timeElapsed; } @Override @@ -321,8 +320,6 @@ private void recordOperationCompletion(@Nullable Throwable status) { long applicationLatencyNano = operationLatencyNano - totalServerLatencyNano.get(); applicationBlockingLatenciesHistogram.record(convertToMs(applicationLatencyNano), attributes); - remainingDeadlineHistogram.record(deadlineRemaining, attributes); - if (operationType == OperationType.ServerStreaming && spanName.getMethodName().equals("ReadRows")) { firstResponseLatenciesHistogram.record( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index d0410989ea..e490a68360 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -227,15 +227,9 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { } @Override -<<<<<<< HEAD - public void grpcMessageSent() { + public void setRemainingDeadline(long deadline) { for (BigtableTracer tracer : bigtableTracers) { - tracer.grpcMessageSent(); -======= - public void setRemainingDeadline(long deadlineRemaining) { - for (BigtableTracer tracer : bigtableTracers) { - tracer.setRemainingDeadline(deadlineRemaining); ->>>>>>> 0ad9a399 (Add override to CompositeTracer) + tracer.setRemainingDeadline(deadline); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index abd15179cd..724dd4972c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -34,12 +34,13 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.InvalidProtocolBufferException; import io.grpc.CallOptions; -import io.grpc.Deadline; import io.grpc.Metadata; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opencensus.tags.TagValue; +import org.threeten.bp.Duration; + import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; @@ -223,7 +224,7 @@ static GrpcCallContext injectBigtableStreamTracer( if (context instanceof GrpcCallContext) { GrpcCallContext callContext = (GrpcCallContext) context; CallOptions callOptions = callContext.getCallOptions(); - Deadline deadline = callOptions.getDeadline(); + Duration deadline = callContext.getTimeout(); return responseMetadata.addHandlers( callContext.withCallOptions( callOptions.withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer, deadline)))); From 3f3d2736b4462bcde0af40343ce98b3893ca9c9e Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:38:34 -0400 Subject: [PATCH 12/41] Fixing merge conflicts --- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 1 - .../bigtable/data/v2/stub/metrics/CompositeTracer.java | 7 +++++++ .../google/cloud/bigtable/data/v2/stub/metrics/Util.java | 1 - 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index d4b1ddd9e1..48a5e2e563 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -96,7 +96,6 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index e490a68360..be1b83217b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -226,6 +226,13 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) { } } + @Override + public void grpcMessageSent() { + for (BigtableTracer tracer : bigtableTracers) { + tracer.grpcMessageSent(); + } + } + @Override public void setRemainingDeadline(long deadline) { for (BigtableTracer tracer : bigtableTracers) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 724dd4972c..0ff324f37f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -40,7 +40,6 @@ import io.grpc.StatusRuntimeException; import io.opencensus.tags.TagValue; import org.threeten.bp.Duration; - import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; From afb4d50dd0689fb6a91035b16e37ebab27dbd2bd Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:39:58 -0400 Subject: [PATCH 13/41] Fix more merge conflicts --- .../data/v2/stub/metrics/BuiltinMetricsTracerFactory.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index 0f4aaa224f..370c608efb 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -25,7 +25,6 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; - import com.google.api.core.InternalApi; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracerFactory; From bf789ab0ddd5ac5073a4a14a3afa36a8fca699d1 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:42:11 -0400 Subject: [PATCH 14/41] Fix merge conflicts... --- .../metrics/BuiltinMetricsTracerTest.java | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) 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 f2ee96736e..a27838b794 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 @@ -73,20 +73,15 @@ import com.google.protobuf.ByteString; import com.google.protobuf.BytesValue; import com.google.protobuf.StringValue; -<<<<<<< HEAD -======= import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; -import io.grpc.Deadline; import io.grpc.ForwardingClientCall; ->>>>>>> 685c3d6b (Fix unit test) import io.grpc.ForwardingServerCall; import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; -import io.grpc.ProxiedSocketAddress; -import io.grpc.ProxyDetector; +import io.grpc.MethodDescriptor; import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -103,8 +98,6 @@ import io.opentelemetry.sdk.metrics.View; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; -import java.io.IOException; -import java.net.SocketAddress; import java.nio.charset.Charset; import java.time.Duration; import java.util.ArrayList; @@ -115,7 +108,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import javax.annotation.Nullable; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -142,7 +134,7 @@ public class BuiltinMetricsTracerTest { private static final long SLEEP_VARIABILITY = 15; private static final String CLIENT_NAME = "java-bigtable/" + Version.VERSION; - private static final long CHANNEL_BLOCKING_LATENCY = 200; + private static final long CHANNEL_BLOCKING_LATENCY = 75; @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); @@ -208,6 +200,28 @@ public void sendHeaders(Metadata headers) { } }; + ClientInterceptor clientInterceptor = + new ClientInterceptor() { + @Override + public ClientCall interceptCall( + MethodDescriptor methodDescriptor, + CallOptions callOptions, + Channel channel) { + return new ForwardingClientCall.SimpleForwardingClientCall( + channel.newCall(methodDescriptor, callOptions)) { + @Override + public void sendMessage(ReqT message) { + try { + Thread.sleep(CHANNEL_BLOCKING_LATENCY); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + super.sendMessage(message); + } + }; + } + }; + server = FakeServiceBuilder.create(fakeService).intercept(trailersInterceptor).start(); BigtableDataSettings settings = @@ -215,7 +229,6 @@ public void sendHeaders(Metadata headers) { .setProjectId(PROJECT_ID) .setInstanceId(INSTANCE_ID) .setAppProfileId(APP_PROFILE_ID) - .setRefreshingChannel(false) .build(); EnhancedBigtableStubSettings.Builder stubSettingsBuilder = settings.getStubSettings().toBuilder(); @@ -255,7 +268,7 @@ public void sendHeaders(Metadata headers) { if (oldConfigurator != null) { builder = oldConfigurator.apply(builder); } - return builder.proxyDetector(new DelayProxyDetector()); + return builder.intercept(clientInterceptor); }); stubSettingsBuilder.setTransportChannelProvider(channelProvider.build()); @@ -683,8 +696,9 @@ public void testQueuedOnChannelUnaryLatencies() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); + long expected = CHANNEL_BLOCKING_LATENCY * 2 / 3; long actual = getAggregatedValue(clientLatency, attributes); - assertThat(actual).isAtLeast(CHANNEL_BLOCKING_LATENCY); + assertThat(actual).isAtLeast(expected); } @Test @@ -852,18 +866,4 @@ public AtomicInteger getResponseCounter() { return responseCounter; } } - - class DelayProxyDetector implements ProxyDetector { - - @Nullable - @Override - public ProxiedSocketAddress proxyFor(SocketAddress socketAddress) throws IOException { - try { - Thread.sleep(CHANNEL_BLOCKING_LATENCY); - } catch (InterruptedException e) { - - } - return null; - } - } } From 501e02913a778a6b32841b31220cc8530c4c7922 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:47:49 -0400 Subject: [PATCH 15/41] Fixing more merge conflicts --- .../metrics/BuiltinMetricsTracerFactory.java | 2 +- .../metrics/BuiltinMetricsTracerTest.java | 43 +++++++++---------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index 370c608efb..6f029733a9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -25,6 +25,7 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; + import com.google.api.core.InternalApi; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracerFactory; @@ -55,7 +56,6 @@ public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; 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 a27838b794..e704f2cf08 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 @@ -82,6 +82,8 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.ProxiedSocketAddress; +import io.grpc.ProxyDetector; import io.grpc.Server; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -98,6 +100,9 @@ import io.opentelemetry.sdk.metrics.View; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; + +import java.io.IOException; +import java.net.SocketAddress; import java.nio.charset.Charset; import java.time.Duration; import java.util.ArrayList; @@ -118,6 +123,8 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; +import javax.annotation.Nullable; + @RunWith(JUnit4.class) public class BuiltinMetricsTracerTest { private static final String PROJECT_ID = "fake-project"; @@ -200,28 +207,6 @@ public void sendHeaders(Metadata headers) { } }; - ClientInterceptor clientInterceptor = - new ClientInterceptor() { - @Override - public ClientCall interceptCall( - MethodDescriptor methodDescriptor, - CallOptions callOptions, - Channel channel) { - return new ForwardingClientCall.SimpleForwardingClientCall( - channel.newCall(methodDescriptor, callOptions)) { - @Override - public void sendMessage(ReqT message) { - try { - Thread.sleep(CHANNEL_BLOCKING_LATENCY); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - super.sendMessage(message); - } - }; - } - }; - server = FakeServiceBuilder.create(fakeService).intercept(trailersInterceptor).start(); BigtableDataSettings settings = @@ -866,4 +851,18 @@ public AtomicInteger getResponseCounter() { return responseCounter; } } + + class DelayProxyDetector implements ProxyDetector { + + @Nullable + @Override + public ProxiedSocketAddress proxyFor(SocketAddress socketAddress) throws IOException { + try { + Thread.sleep(CHANNEL_BLOCKING_LATENCY); + } catch (InterruptedException e) { + + } + return null; + } + } } From 42930a82438e9912b4a9dd737431c62e9fb78da3 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 16 Oct 2024 14:50:44 -0400 Subject: [PATCH 16/41] Fix more merge conflicts --- .../metrics/BuiltinMetricsTracerTest.java | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) 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 e704f2cf08..3855e0bb93 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 @@ -44,7 +44,6 @@ import com.google.api.gax.batching.FlowControlSettings; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; -import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.NotFoundException; import com.google.api.gax.rpc.ResponseObserver; @@ -73,15 +72,9 @@ import com.google.protobuf.ByteString; import com.google.protobuf.BytesValue; import com.google.protobuf.StringValue; -import io.grpc.CallOptions; -import io.grpc.Channel; -import io.grpc.ClientCall; -import io.grpc.ClientInterceptor; -import io.grpc.ForwardingClientCall; import io.grpc.ForwardingServerCall; import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; -import io.grpc.MethodDescriptor; import io.grpc.ProxiedSocketAddress; import io.grpc.ProxyDetector; import io.grpc.Server; @@ -100,11 +93,9 @@ import io.opentelemetry.sdk.metrics.View; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; - import java.io.IOException; import java.net.SocketAddress; import java.nio.charset.Charset; -import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; @@ -113,6 +104,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nullable; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -123,8 +115,6 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; -import javax.annotation.Nullable; - @RunWith(JUnit4.class) public class BuiltinMetricsTracerTest { private static final String PROJECT_ID = "fake-project"; @@ -141,7 +131,7 @@ public class BuiltinMetricsTracerTest { private static final long SLEEP_VARIABILITY = 15; private static final String CLIENT_NAME = "java-bigtable/" + Version.VERSION; - private static final long CHANNEL_BLOCKING_LATENCY = 75; + private static final long CHANNEL_BLOCKING_LATENCY = 200; @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); @@ -214,6 +204,7 @@ public void sendHeaders(Metadata headers) { .setProjectId(PROJECT_ID) .setInstanceId(INSTANCE_ID) .setAppProfileId(APP_PROFILE_ID) + .setRefreshingChannel(false) .build(); EnhancedBigtableStubSettings.Builder stubSettingsBuilder = settings.getStubSettings().toBuilder(); @@ -253,7 +244,7 @@ public void sendHeaders(Metadata headers) { if (oldConfigurator != null) { builder = oldConfigurator.apply(builder); } - return builder.intercept(clientInterceptor); + return builder.proxyDetector(new DelayProxyDetector()); }); stubSettingsBuilder.setTransportChannelProvider(channelProvider.build()); @@ -681,9 +672,8 @@ public void testQueuedOnChannelUnaryLatencies() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - long expected = CHANNEL_BLOCKING_LATENCY * 2 / 3; long actual = getAggregatedValue(clientLatency, attributes); - assertThat(actual).isAtLeast(expected); + assertThat(actual).isAtLeast(CHANNEL_BLOCKING_LATENCY); } @Test From 0191ab028e2bd5237149b696474dfbbaef537ee6 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Thu, 17 Oct 2024 12:31:26 -0400 Subject: [PATCH 17/41] Use operation timer --- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 8 ++++++-- .../data/v2/stub/metrics/BuiltinMetricsTracerTest.java | 10 +++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 48a5e2e563..1a7b5a1f61 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -86,6 +86,8 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); + private long deadlineRemaining = 0; + // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them // to milliseconds and use DoubleHistogram. This should minimize the chance of a data @@ -273,8 +275,8 @@ public void grpcMessageSent() { @Override public void setRemainingDeadline(long deadline) { - long timeElapsed = attemptTimer.elapsed(TimeUnit.MILLISECONDS); - long deadlineRemaining = deadline - timeElapsed; + long timeElapsed = operationTimer.elapsed(TimeUnit.MILLISECONDS); + deadlineRemaining = deadline - timeElapsed; } @Override @@ -315,6 +317,8 @@ private void recordOperationCompletion(@Nullable Throwable status) { operationLatenciesHistogram.record(convertToMs(operationLatencyNano), attributes); + remainingDeadlineHistogram.record(deadlineRemaining, attributes); + // serverLatencyTimer should already be stopped in recordAttemptCompletion long applicationLatencyNano = operationLatencyNano - totalServerLatencyNano.get(); applicationBlockingLatenciesHistogram.record(convertToMs(applicationLatencyNano), attributes); 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 3855e0bb93..c899d8260e 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 @@ -96,6 +96,7 @@ import java.io.IOException; import java.net.SocketAddress; import java.nio.charset.Charset; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; @@ -213,6 +214,8 @@ public void sendHeaders(Metadata headers) { .retrySettings() .setInitialRetryDelayDuration(java.time.Duration.ofMillis(200)); + stubSettingsBuilder.readRowsSettings().retrySettings().setTotalTimeoutDuration(Duration.ofMillis(3000)); + stubSettingsBuilder .bulkMutateRowsSettings() .setBatchingSettings( @@ -706,9 +709,7 @@ public void testPermanentFailure() { @Test public void testRemainingDeadline() { - GrpcCallContext context = GrpcCallContext.createDefault(); - stub.readRowsCallable().all().call(Query.create(TABLE), - context.withCallOptions(context.getCallOptions().withDeadlineAfter(1000, TimeUnit.MILLISECONDS))); + stub.readRowsCallable().all().call(Query.create(TABLE)); MetricData remainingDeadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); Attributes attributes = @@ -724,8 +725,7 @@ public void testRemainingDeadline() { .build(); long remainingDeadline = getAggregatedValue(remainingDeadlineMetric, attributes); - System.out.println("DEADLINE = " + remainingDeadline); - assertThat(remainingDeadline).isIn(Range.closed((long) 500, (long) 800)); + assertThat(remainingDeadline).isIn(Range.closed((long) 2000, (long) 2500)); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From ec13bd807455ab64e6327a21d48211ce3c49e02c Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 21 Oct 2024 13:59:00 -0400 Subject: [PATCH 18/41] Run auto formatter --- .../BigtableCloudMonitoringExporter.java | 2 +- .../data/v2/stub/metrics/BigtableTracer.java | 4 +-- .../stub/metrics/BuiltinMetricsConstants.java | 18 ++++++------- .../metrics/BuiltinMetricsTracerFactory.java | 11 ++++---- .../bigtable/data/v2/stub/metrics/Util.java | 5 ++-- .../metrics/BuiltinMetricsTracerTest.java | 26 ++++++++++--------- 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index 27fdd75995..8aa53fa198 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -22,8 +22,8 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.FIRST_RESPONSE_LATENCIES_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.METER_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.OPERATION_LATENCIES_NAME; -import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.PER_CONNECTION_ERROR_COUNT_NAME; +import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.REMAINING_DEADLINE_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.RETRY_COUNT_NAME; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.SERVER_LATENCIES_NAME; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index c6f6636619..5317a83314 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -95,8 +95,8 @@ public void grpcMessageSent() { } /** - * Set the remaining customer specified deadline so it can be exported in a metric. This will - * be called in BuiltinMetricsTracer. + * Set the remaining customer specified deadline so it can be exported in a metric. This will be + * called in BuiltinMetricsTracer. */ public void setRemainingDeadline(long deadline) { // noop diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java index 58a70f5254..62ac0f1153 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsConstants.java @@ -216,15 +216,15 @@ public static Map getAllViews() { .add(BIGTABLE_PROJECT_ID_KEY, INSTANCE_ID_KEY, APP_PROFILE_KEY, CLIENT_NAME_KEY) .build()); defineView( - views, - REMAINING_DEADLINE_NAME, - AGGREGATION_WITH_MILLIS_HISTOGRAM, - InstrumentType.HISTOGRAM, - "ms", - ImmutableSet.builder() - .addAll(COMMON_ATTRIBUTES) - .add(STREAMING_KEY, STATUS_KEY) - .build()); + views, + REMAINING_DEADLINE_NAME, + AGGREGATION_WITH_MILLIS_HISTOGRAM, + InstrumentType.HISTOGRAM, + "ms", + ImmutableSet.builder() + .addAll(COMMON_ATTRIBUTES) + .add(STREAMING_KEY, STATUS_KEY) + .build()); return views.build(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index 6f029733a9..e3c60799b8 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -111,12 +111,11 @@ public static BuiltinMetricsTracerFactory create( .setUnit(MILLISECOND) .build(); remainingDeadlineHistogram = - meter - .histogramBuilder(REMAINING_DEADLINE_NAME) - .setDescription( - "The remaining customer specified deadline at the end of the request.") - .setUnit(MILLISECOND) - .build(); + meter + .histogramBuilder(REMAINING_DEADLINE_NAME) + .setDescription("The remaining customer specified deadline at the end of the request.") + .setUnit(MILLISECOND) + .build(); connectivityErrorCounter = meter .counterBuilder(CONNECTIVITY_ERROR_COUNT_NAME) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 0ff324f37f..f7ccfad718 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -39,7 +39,6 @@ import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opencensus.tags.TagValue; -import org.threeten.bp.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; @@ -51,6 +50,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; +import org.threeten.bp.Duration; /** Utilities to help integrating with OpenCensus. */ @InternalApi("For internal use only") @@ -226,7 +226,8 @@ static GrpcCallContext injectBigtableStreamTracer( Duration deadline = callContext.getTimeout(); return responseMetadata.addHandlers( callContext.withCallOptions( - callOptions.withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer, deadline)))); + callOptions.withStreamTracerFactory( + new BigtableGrpcStreamTracer.Factory(tracer, deadline)))); } else { // context should always be an instance of GrpcCallContext. If not throw an exception // so we can see what class context is. 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 c899d8260e..b826b087e1 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 @@ -42,7 +42,6 @@ import com.google.api.gax.batching.BatchingException; import com.google.api.gax.batching.BatchingSettings; import com.google.api.gax.batching.FlowControlSettings; -import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.NotFoundException; @@ -214,7 +213,10 @@ public void sendHeaders(Metadata headers) { .retrySettings() .setInitialRetryDelayDuration(java.time.Duration.ofMillis(200)); - stubSettingsBuilder.readRowsSettings().retrySettings().setTotalTimeoutDuration(Duration.ofMillis(3000)); + stubSettingsBuilder + .readRowsSettings() + .retrySettings() + .setTotalTimeoutDuration(Duration.ofMillis(3000)); stubSettingsBuilder .bulkMutateRowsSettings() @@ -713,16 +715,16 @@ public void testRemainingDeadline() { MetricData remainingDeadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); Attributes attributes = - baseAttributes - .toBuilder() - .put(STATUS_KEY, "OK") - .put(TABLE_ID_KEY, TABLE) - .put(ZONE_ID_KEY, ZONE) - .put(CLUSTER_ID_KEY, CLUSTER) - .put(METHOD_KEY, "Bigtable.ReadRows") - .put(STREAMING_KEY, true) - .put(CLIENT_NAME_KEY, CLIENT_NAME) - .build(); + baseAttributes + .toBuilder() + .put(STATUS_KEY, "OK") + .put(TABLE_ID_KEY, TABLE) + .put(ZONE_ID_KEY, ZONE) + .put(CLUSTER_ID_KEY, CLUSTER) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(STREAMING_KEY, true) + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); long remainingDeadline = getAggregatedValue(remainingDeadlineMetric, attributes); assertThat(remainingDeadline).isIn(Range.closed((long) 2000, (long) 2500)); From 1322f44df43d1ba200b5cdeb34fa67675df24a07 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 21 Oct 2024 16:56:42 -0400 Subject: [PATCH 19/41] Address PR comment --- .../stub/metrics/BigtableGrpcStreamTracer.java | 3 ++- .../data/v2/stub/metrics/BigtableTracer.java | 2 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 17 ++++++++--------- .../metrics/BuiltinMetricsTracerFactory.java | 8 ++++---- .../data/v2/stub/metrics/CompositeTracer.java | 4 ++-- .../stub/metrics/BuiltinMetricsTracerTest.java | 8 +++++--- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 6278c7a9c3..9eeab65966 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -37,7 +37,8 @@ public BigtableGrpcStreamTracer(BigtableTracer tracer, Duration deadline) { @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { tracer.grpcMessageSent(); - tracer.setRemainingDeadline(deadline.toMillis()); + System.out.print("outboundMessageSent DEADLINE=" + deadline.toMillis() + "\n"); + tracer.setDeadline(deadline.toMillis()); } static class Factory extends ClientStreamTracer.Factory { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 5317a83314..9ef811d464 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -98,7 +98,7 @@ public void grpcMessageSent() { * Set the remaining customer specified deadline so it can be exported in a metric. This will be * called in BuiltinMetricsTracer. */ - public void setRemainingDeadline(long deadline) { + public void setDeadline(long deadline) { // noop } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 1a7b5a1f61..4445f0e1fe 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -86,7 +86,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); - private long deadlineRemaining = 0; + private long deadline = 0; // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -98,7 +98,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram remainingDeadlineHistogram; + private final DoubleHistogram deadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -112,7 +112,7 @@ class BuiltinMetricsTracer extends BigtableTracer { DoubleHistogram firstResponseLatenciesHistogram, DoubleHistogram clientBlockingLatenciesHistogram, DoubleHistogram applicationBlockingLatenciesHistogram, - DoubleHistogram remainingDeadlineHistogram, + DoubleHistogram deadlineHistogram, LongCounter connectivityErrorCounter, LongCounter retryCounter) { this.operationType = operationType; @@ -125,7 +125,7 @@ class BuiltinMetricsTracer extends BigtableTracer { this.firstResponseLatenciesHistogram = firstResponseLatenciesHistogram; this.clientBlockingLatenciesHistogram = clientBlockingLatenciesHistogram; this.applicationBlockingLatenciesHistogram = applicationBlockingLatenciesHistogram; - this.remainingDeadlineHistogram = remainingDeadlineHistogram; + this.deadlineHistogram = deadlineHistogram; this.connectivityErrorCounter = connectivityErrorCounter; this.retryCounter = retryCounter; } @@ -274,9 +274,8 @@ public void grpcMessageSent() { } @Override - public void setRemainingDeadline(long deadline) { - long timeElapsed = operationTimer.elapsed(TimeUnit.MILLISECONDS); - deadlineRemaining = deadline - timeElapsed; + public void setDeadline(long deadline) { + this.deadline = deadline; } @Override @@ -317,8 +316,6 @@ private void recordOperationCompletion(@Nullable Throwable status) { operationLatenciesHistogram.record(convertToMs(operationLatencyNano), attributes); - remainingDeadlineHistogram.record(deadlineRemaining, attributes); - // serverLatencyTimer should already be stopped in recordAttemptCompletion long applicationLatencyNano = operationLatencyNano - totalServerLatencyNano.get(); applicationBlockingLatenciesHistogram.record(convertToMs(applicationLatencyNano), attributes); @@ -371,6 +368,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); + deadlineHistogram.record(deadline, attributes); + if (serverLatencies != null) { serverLatenciesHistogram.record(serverLatencies, attributes); connectivityErrorCounter.add(0, attributes); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index e3c60799b8..b4e412503d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -56,7 +56,7 @@ public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram remainingDeadlineHistogram; + private final DoubleHistogram deadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -110,10 +110,10 @@ public static BuiltinMetricsTracerFactory create( "The latency of the client application consuming available response data.") .setUnit(MILLISECOND) .build(); - remainingDeadlineHistogram = + deadlineHistogram = meter .histogramBuilder(REMAINING_DEADLINE_NAME) - .setDescription("The remaining customer specified deadline at the end of the request.") + .setDescription("The customer specified operation deadline for the request.") .setUnit(MILLISECOND) .build(); connectivityErrorCounter = @@ -143,7 +143,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op firstResponseLatenciesHistogram, clientBlockingLatenciesHistogram, applicationBlockingLatenciesHistogram, - remainingDeadlineHistogram, + deadlineHistogram, connectivityErrorCounter, retryCounter); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index be1b83217b..a437f13af5 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -234,9 +234,9 @@ public void grpcMessageSent() { } @Override - public void setRemainingDeadline(long deadline) { + public void setDeadline(long deadline) { for (BigtableTracer tracer : bigtableTracers) { - tracer.setRemainingDeadline(deadline); + tracer.setDeadline(deadline); } } } 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 b826b087e1..ba91a33d25 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 @@ -711,8 +711,10 @@ public void testPermanentFailure() { @Test public void testRemainingDeadline() { + // The FakeService implements a retry attempt by default. By incrementing the attemptCounter, we skip that. + fakeService.attemptCounter.getAndIncrement(); stub.readRowsCallable().all().call(Query.create(TABLE)); - MetricData remainingDeadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); + MetricData deadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); Attributes attributes = baseAttributes @@ -726,8 +728,8 @@ public void testRemainingDeadline() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - long remainingDeadline = getAggregatedValue(remainingDeadlineMetric, attributes); - assertThat(remainingDeadline).isIn(Range.closed((long) 2000, (long) 2500)); + long deadline = getAggregatedValue(deadlineMetric, attributes); + assertThat(deadline).isEqualTo(3000); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 2235f2032700e128f681890cf6e0dd1cc9613fb5 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 21 Oct 2024 16:58:32 -0400 Subject: [PATCH 20/41] Remove print statement, fix fn coment --- .../bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java | 1 - .../cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 9eeab65966..06d0fd3656 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -37,7 +37,6 @@ public BigtableGrpcStreamTracer(BigtableTracer tracer, Duration deadline) { @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { tracer.grpcMessageSent(); - System.out.print("outboundMessageSent DEADLINE=" + deadline.toMillis() + "\n"); tracer.setDeadline(deadline.toMillis()); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 9ef811d464..97c45fc65b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -95,7 +95,7 @@ public void grpcMessageSent() { } /** - * Set the remaining customer specified deadline so it can be exported in a metric. This will be + * Set the customer specified deadline so that it can be exported in a metric. This will be * called in BuiltinMetricsTracer. */ public void setDeadline(long deadline) { From b8046a6d7aa10ba89a021fa02921572d5bc155ab Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 12:42:10 -0400 Subject: [PATCH 21/41] Trying to plumb operation timeout thru --- .../data/v2/stub/EnhancedBigtableStub.java | 25 +++++++++++-------- .../metrics/BigtableGrpcStreamTracer.java | 11 ++++---- .../data/v2/stub/metrics/BigtableTracer.java | 4 +-- .../BigtableTracerStreamingCallable.java | 6 +++-- .../metrics/BigtableTracerUnaryCallable.java | 6 +++-- .../v2/stub/metrics/BuiltinMetricsTracer.java | 8 ++++-- .../metrics/BuiltinMetricsTracerFactory.java | 2 +- .../data/v2/stub/metrics/CompositeTracer.java | 4 +-- .../bigtable/data/v2/stub/metrics/Util.java | 5 ++-- .../metrics/BuiltinMetricsTracerTest.java | 16 ++++++++---- 10 files changed, 54 insertions(+), 33 deletions(-) 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 91c63c2b85..97eb09daa6 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 @@ -185,6 +185,7 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final long FLOW_CONTROL_ADJUSTING_INTERVAL_MS = TimeUnit.SECONDS.toMillis(20); private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; + private final ApiCallContext.Key deadlineKey = ApiCallContext.Key.create("TEST123"); private final boolean closeClientContext; private final RequestContext requestContext; @@ -508,6 +509,7 @@ public EnhancedBigtableStub( @BetaApi("This surface is stable yet it might be removed in the future.") public ServerStreamingCallable createReadRowsRawCallable( RowAdapter rowAdapter) { + System.out.println("createReadRowsRawCallable"); return createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter) .withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -528,6 +530,7 @@ public ServerStreamingCallable createReadRowsRawCa */ public ServerStreamingCallable createReadRowsCallable( RowAdapter rowAdapter) { + System.out.println("createReadRowsCallable"); ServerStreamingCallable readRowsCallable = createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter); @@ -539,7 +542,9 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext(). + withOption(deadlineKey, (long) settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); +// withOption(ApiCallContext.Key.create("AFFINITY_TEST"), settings.readRowSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -647,7 +652,7 @@ public Map extract(ReadRowsRequest readRowsRequest) { Callables.watched(merging, innerSettings, clientContext); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched); + new BigtableTracerStreamingCallable<>(watched, deadlineKey); // Retry logic is split into 2 parts to workaround a rare edge case described in // ReadRowsRetryCompletedCallable @@ -740,7 +745,7 @@ public Map extract( withStatsHeaders = new StatsHeadersUnaryCallable<>(spoolable); UnaryCallable> - withBigtableTracer = new BigtableTracerUnaryCallable<>(withStatsHeaders); + withBigtableTracer = new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); UnaryCallable> retryable = withRetries(withBigtableTracer, settings.sampleRowKeysSettings()); @@ -830,7 +835,7 @@ public Map extract(MutateRowRequest mutateRowRequest) { new StatsHeadersUnaryCallable<>(base); UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders); + new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); UnaryCallable retrying = withRetries(withBigtableTracer, settings.mutateRowSettings()); @@ -897,7 +902,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { new ConvertExceptionCallable<>(callable); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(convertException); + new BigtableTracerStreamingCallable<>(convertException, deadlineKey); BasicResultRetryAlgorithm resultRetryAlgorithm; if (settings.getEnableRetryInfo()) { @@ -1087,7 +1092,7 @@ public Map extract( new StatsHeadersUnaryCallable<>(base); UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders); + new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); UnaryCallable retrying = withRetries(withBigtableTracer, settings.checkAndMutateRowSettings()); @@ -1133,7 +1138,7 @@ public Map extract(ReadModifyWriteRowRequest request) { String methodName = "ReadModifyWriteRow"; UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders); + new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); UnaryCallable retrying = withRetries(withBigtableTracer, settings.readModifyWriteRowSettings()); @@ -1213,7 +1218,7 @@ public Map extract( Callables.watched(convertException, innerSettings, clientContext); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched); + new BigtableTracerStreamingCallable<>(watched, deadlineKey); ServerStreamingCallable retrying = withRetries(withBigtableTracer, innerSettings); @@ -1288,7 +1293,7 @@ public Map extract( Callables.watched(merging, innerSettings, clientContext); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched); + new BigtableTracerStreamingCallable<>(watched, deadlineKey); ServerStreamingCallable readChangeStreamCallable = withRetries(withBigtableTracer, innerSettings); @@ -1361,7 +1366,7 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { new SqlRowMergingCallable(withMetadataObserver); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(merging); + new BigtableTracerStreamingCallable<>(merging, deadlineKey); ServerStreamingCallSettings retrySettings = ServerStreamingCallSettings.newBuilder() diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 06d0fd3656..4783c12fb6 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -27,25 +27,26 @@ class BigtableGrpcStreamTracer extends ClientStreamTracer { private final BigtableTracer tracer; - private final Duration deadline; + private final long deadline; - public BigtableGrpcStreamTracer(BigtableTracer tracer, Duration deadline) { + public BigtableGrpcStreamTracer(BigtableTracer tracer, long deadline) { this.tracer = tracer; this.deadline = deadline; } @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { + System.out.println("HERE " + deadline + "\n"); tracer.grpcMessageSent(); - tracer.setDeadline(deadline.toMillis()); + tracer.setRemainingDeadline(deadline); } static class Factory extends ClientStreamTracer.Factory { private final BigtableTracer tracer; - private final Duration deadline; + private final long deadline; - Factory(BigtableTracer tracer, Duration deadline) { + Factory(BigtableTracer tracer, long deadline) { this.tracer = tracer; this.deadline = deadline; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 97c45fc65b..4d59d17e9e 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -95,10 +95,10 @@ public void grpcMessageSent() { } /** - * Set the customer specified deadline so that it can be exported in a metric. This will be + * Record the deadline when the request is sent to the Bigtable server. This will be * called in BuiltinMetricsTracer. */ - public void setDeadline(long deadline) { + public void setRemainingDeadline(long deadline) { // noop } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 167cd0dc2e..dfef74c6e4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -47,10 +47,12 @@ public class BigtableTracerStreamingCallable extends ServerStreamingCallable { private final ServerStreamingCallable innerCallable; + private final ApiCallContext.Key deadlineKey; public BigtableTracerStreamingCallable( - @Nonnull ServerStreamingCallable callable) { + @Nonnull ServerStreamingCallable callable, ApiCallContext.Key deadlineKey) { this.innerCallable = Preconditions.checkNotNull(callable, "Inner callable must be set"); + this.deadlineKey = deadlineKey; } @Override @@ -66,7 +68,7 @@ public void call( request, innerObserver, Util.injectBigtableStreamTracer( - context, responseMetadata, (BigtableTracer) context.getTracer())); + context, responseMetadata, (BigtableTracer) context.getTracer(), deadlineKey)); } else { innerCallable.call(request, responseObserver, context); } 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 7dfca8b753..ee093840b3 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 @@ -45,9 +45,11 @@ public class BigtableTracerUnaryCallable extends UnaryCallable { private final UnaryCallable innerCallable; + private final ApiCallContext.Key deadlineKey; - public BigtableTracerUnaryCallable(@Nonnull UnaryCallable innerCallable) { + public BigtableTracerUnaryCallable(@Nonnull UnaryCallable innerCallable, ApiCallContext.Key deadlineKey) { this.innerCallable = Preconditions.checkNotNull(innerCallable, "Inner callable must be set"); + this.deadlineKey = deadlineKey; } @Override @@ -62,7 +64,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) innerCallable.futureCall( request, Util.injectBigtableStreamTracer( - context, responseMetadata, (BigtableTracer) context.getTracer())); + context, responseMetadata, (BigtableTracer) context.getTracer(), deadlineKey)); ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); return future; } else { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 4445f0e1fe..241ab89b66 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -273,9 +273,13 @@ public void grpcMessageSent() { grpcMessageSentDelay.set(attemptTimer.elapsed(TimeUnit.NANOSECONDS)); } + /* + In GrpcClientCalls we set the timeout in ApiContext on CallOptions. The timeout in ApiContext will be the attempt + timeout for the first few requests or the remaining operation timeout after retries and back offs. + */ @Override - public void setDeadline(long deadline) { - this.deadline = deadline; + public void setRemainingDeadline(long deadline) { + this.deadline = deadline - operationTimer.elapsed(TimeUnit.MILLISECONDS); } @Override diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index b4e412503d..dd00c32e2c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -113,7 +113,7 @@ public static BuiltinMetricsTracerFactory create( deadlineHistogram = meter .histogramBuilder(REMAINING_DEADLINE_NAME) - .setDescription("The customer specified operation deadline for the request.") + .setDescription("The remaining deadline when the request is sent to grpc. This will either be the attempt timeout for the first few retries, or the reamining deadline from operation timeout after retries and back offs.") .setUnit(MILLISECOND) .build(); connectivityErrorCounter = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index a437f13af5..be1b83217b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -234,9 +234,9 @@ public void grpcMessageSent() { } @Override - public void setDeadline(long deadline) { + public void setRemainingDeadline(long deadline) { for (BigtableTracer tracer : bigtableTracers) { - tracer.setDeadline(deadline); + tracer.setRemainingDeadline(deadline); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index f7ccfad718..092f3d3dee 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -22,6 +22,7 @@ import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.StatusCode; import com.google.api.gax.rpc.StatusCode.Code; +import com.google.api.gax.rpc.internal.ApiCallContextOptions; import com.google.bigtable.v2.AuthorizedViewName; import com.google.bigtable.v2.CheckAndMutateRowRequest; import com.google.bigtable.v2.MutateRowRequest; @@ -219,11 +220,11 @@ static void recordMetricsFromMetadata( * io.grpc.ClientStreamTracer} to the callContext. */ static GrpcCallContext injectBigtableStreamTracer( - ApiCallContext context, GrpcResponseMetadata responseMetadata, BigtableTracer tracer) { + ApiCallContext context, GrpcResponseMetadata responseMetadata, BigtableTracer tracer, ApiCallContext.Key deadlineKey) { if (context instanceof GrpcCallContext) { GrpcCallContext callContext = (GrpcCallContext) context; CallOptions callOptions = callContext.getCallOptions(); - Duration deadline = callContext.getTimeout(); + long deadline = callContext.getOption(deadlineKey); return responseMetadata.addHandlers( callContext.withCallOptions( callOptions.withStreamTracerFactory( 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 ba91a33d25..780d5a3a41 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 @@ -213,10 +213,18 @@ public void sendHeaders(Metadata headers) { .retrySettings() .setInitialRetryDelayDuration(java.time.Duration.ofMillis(200)); + java.time.Duration timeout = java.time.Duration.ofMillis(6000); + java.time.Duration backOffTime = java.time.Duration.ofMillis(10); stubSettingsBuilder .readRowsSettings() .retrySettings() - .setTotalTimeoutDuration(Duration.ofMillis(3000)); + .setTotalTimeoutDuration(Duration.ofMillis(9000)) + .setMaxRpcTimeout(org.threeten.bp.Duration.parse(timeout.toString())) + .setRpcTimeoutMultiplier(1) + .setInitialRpcTimeout(org.threeten.bp.Duration.parse(timeout.toString())) + .setInitialRetryDelay(org.threeten.bp.Duration.parse(backOffTime.toString())) + .setRetryDelayMultiplier(1) + .setMaxRetryDelay(org.threeten.bp.Duration.parse(backOffTime.toString())); stubSettingsBuilder .bulkMutateRowsSettings() @@ -711,8 +719,6 @@ public void testPermanentFailure() { @Test public void testRemainingDeadline() { - // The FakeService implements a retry attempt by default. By incrementing the attemptCounter, we skip that. - fakeService.attemptCounter.getAndIncrement(); stub.readRowsCallable().all().call(Query.create(TABLE)); MetricData deadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); @@ -728,8 +734,8 @@ public void testRemainingDeadline() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - long deadline = getAggregatedValue(deadlineMetric, attributes); - assertThat(deadline).isEqualTo(3000); + long remainingDeadline = getAggregatedValue(deadlineMetric, attributes); + assertThat(remainingDeadline).isIn(Range.closed((long) 8400, (long) 8600)); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 68d22165b2b4ef32f72500f1925183918a7e5386 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 13:07:06 -0400 Subject: [PATCH 22/41] Set remaining deadline in tracer callables --- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 1 + .../v2/stub/metrics/BigtableGrpcStreamTracer.java | 12 +++--------- .../metrics/BigtableTracerStreamingCallable.java | 7 +++++++ .../cloud/bigtable/data/v2/stub/metrics/Util.java | 2 +- .../v2/stub/metrics/BuiltinMetricsTracerTest.java | 1 + 5 files changed, 13 insertions(+), 10 deletions(-) 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 97eb09daa6..b26b0a447a 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 @@ -39,6 +39,7 @@ import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; +import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.Callables; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.RequestParamsExtractor; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 4783c12fb6..681d18c67c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -27,34 +27,28 @@ class BigtableGrpcStreamTracer extends ClientStreamTracer { private final BigtableTracer tracer; - private final long deadline; - public BigtableGrpcStreamTracer(BigtableTracer tracer, long deadline) { + public BigtableGrpcStreamTracer(BigtableTracer tracer) { this.tracer = tracer; - this.deadline = deadline; } @Override public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) { - System.out.println("HERE " + deadline + "\n"); tracer.grpcMessageSent(); - tracer.setRemainingDeadline(deadline); } static class Factory extends ClientStreamTracer.Factory { private final BigtableTracer tracer; - private final long deadline; - Factory(BigtableTracer tracer, long deadline) { + Factory(BigtableTracer tracer) { this.tracer = tracer; - this.deadline = deadline; } @Override public ClientStreamTracer newClientStreamTracer( ClientStreamTracer.StreamInfo info, Metadata headers) { - return new BigtableGrpcStreamTracer(tracer, deadline); + return new BigtableGrpcStreamTracer(tracer); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index dfef74c6e4..581865b2ab 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ResponseObserver; @@ -24,6 +25,8 @@ import com.google.cloud.bigtable.data.v2.stub.SafeResponseObserver; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; +import io.grpc.CallOptions; + import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; @@ -64,6 +67,10 @@ public void call( BigtableTracerResponseObserver innerObserver = new BigtableTracerResponseObserver<>( responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); + GrpcCallContext callContext = (GrpcCallContext) context; + long deadline = callContext.getOption(deadlineKey); + ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); + System.out.println("TracerStreamingCallable::call " + deadline + "\n"); innerCallable.call( request, innerObserver, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 092f3d3dee..23089670d1 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -228,7 +228,7 @@ static GrpcCallContext injectBigtableStreamTracer( return responseMetadata.addHandlers( callContext.withCallOptions( callOptions.withStreamTracerFactory( - new BigtableGrpcStreamTracer.Factory(tracer, deadline)))); + new BigtableGrpcStreamTracer.Factory(tracer)))); } else { // context should always be an instance of GrpcCallContext. If not throw an exception // so we can see what class context is. 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 780d5a3a41..9f6a946389 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 @@ -735,6 +735,7 @@ public void testRemainingDeadline() { .build(); long remainingDeadline = getAggregatedValue(deadlineMetric, attributes); + System.out.println("remaining deadline @ end of test: " + remainingDeadline + "\n"); assertThat(remainingDeadline).isIn(Range.closed((long) 8400, (long) 8600)); } From 235e73a2d4659e8a271a02431b29ae5c6997e371 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 13:08:17 -0400 Subject: [PATCH 23/41] Set remaining deadline in tracer unary callable as well --- .../data/v2/stub/metrics/BigtableTracerUnaryCallable.java | 4 ++++ 1 file changed, 4 insertions(+) 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 ee093840b3..f3f01a3f49 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 @@ -19,6 +19,7 @@ import com.google.api.core.ApiFutureCallback; import com.google.api.core.ApiFutures; import com.google.api.core.InternalApi; +import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcResponseMetadata; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.UnaryCallable; @@ -60,6 +61,9 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) BigtableTracerUnaryCallback callback = new BigtableTracerUnaryCallback( (BigtableTracer) context.getTracer(), responseMetadata); + GrpcCallContext callContext = (GrpcCallContext) context; + long deadline = callContext.getOption(deadlineKey); + ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); ApiFuture future = innerCallable.futureCall( request, From b878e0bda0616e697c9ea1e6d6d509fabef03aab Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 13:20:57 -0400 Subject: [PATCH 24/41] Remove unused lines --- .../google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 1 - .../bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java | 1 - 2 files changed, 2 deletions(-) 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 b26b0a447a..d5bb74b068 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 @@ -545,7 +545,6 @@ public ServerStreamingCallable createReadRowsCallable( return traced.withDefaultCallContext(clientContext.getDefaultCallContext(). withOption(deadlineKey, (long) settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); -// withOption(ApiCallContext.Key.create("AFFINITY_TEST"), settings.readRowSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java index 681d18c67c..80fcdd0419 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java @@ -17,7 +17,6 @@ import io.grpc.ClientStreamTracer; import io.grpc.Metadata; -import org.threeten.bp.Duration; /** * Records the time a request is enqueued in a grpc channel queue. This a bridge between gRPC stream From e9b4b12266378eb9d61fc2bfd65ea8063cb28106 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 14:41:53 -0400 Subject: [PATCH 25/41] Remove unused code --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 4 +--- .../v2/stub/metrics/BigtableTracerStreamingCallable.java | 5 +---- .../data/v2/stub/metrics/BigtableTracerUnaryCallable.java | 2 +- .../data/v2/stub/metrics/BuiltinMetricsTracerFactory.java | 6 +++--- .../google/cloud/bigtable/data/v2/stub/metrics/Util.java | 8 ++------ 5 files changed, 8 insertions(+), 17 deletions(-) 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 d5bb74b068..716d9eb8db 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 @@ -186,7 +186,7 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final long FLOW_CONTROL_ADJUSTING_INTERVAL_MS = TimeUnit.SECONDS.toMillis(20); private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; - private final ApiCallContext.Key deadlineKey = ApiCallContext.Key.create("TEST123"); + private final ApiCallContext.Key deadlineKey = ApiCallContext.Key.create("deadline"); private final boolean closeClientContext; private final RequestContext requestContext; @@ -510,7 +510,6 @@ public EnhancedBigtableStub( @BetaApi("This surface is stable yet it might be removed in the future.") public ServerStreamingCallable createReadRowsRawCallable( RowAdapter rowAdapter) { - System.out.println("createReadRowsRawCallable"); return createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter) .withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -531,7 +530,6 @@ public ServerStreamingCallable createReadRowsRawCa */ public ServerStreamingCallable createReadRowsCallable( RowAdapter rowAdapter) { - System.out.println("createReadRowsCallable"); ServerStreamingCallable readRowsCallable = createReadRowsBaseCallable(settings.readRowsSettings(), rowAdapter); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 581865b2ab..9bd1ed94e4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -25,8 +25,6 @@ import com.google.cloud.bigtable.data.v2.stub.SafeResponseObserver; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; -import io.grpc.CallOptions; - import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; @@ -70,12 +68,11 @@ public void call( GrpcCallContext callContext = (GrpcCallContext) context; long deadline = callContext.getOption(deadlineKey); ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); - System.out.println("TracerStreamingCallable::call " + deadline + "\n"); innerCallable.call( request, innerObserver, Util.injectBigtableStreamTracer( - context, responseMetadata, (BigtableTracer) context.getTracer(), deadlineKey)); + context, responseMetadata, (BigtableTracer) context.getTracer())); } else { innerCallable.call(request, responseObserver, context); } 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 f3f01a3f49..a26b7bdbc7 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 @@ -68,7 +68,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) innerCallable.futureCall( request, Util.injectBigtableStreamTracer( - context, responseMetadata, (BigtableTracer) context.getTracer(), deadlineKey)); + context, responseMetadata, (BigtableTracer) context.getTracer())); ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor()); return future; } else { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index dd00c32e2c..44f76ebae0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -56,7 +56,7 @@ public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram deadlineHistogram; + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -110,7 +110,7 @@ public static BuiltinMetricsTracerFactory create( "The latency of the client application consuming available response data.") .setUnit(MILLISECOND) .build(); - deadlineHistogram = + remainingDeadlineHistogram = meter .histogramBuilder(REMAINING_DEADLINE_NAME) .setDescription("The remaining deadline when the request is sent to grpc. This will either be the attempt timeout for the first few retries, or the reamining deadline from operation timeout after retries and back offs.") @@ -143,7 +143,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op firstResponseLatenciesHistogram, clientBlockingLatenciesHistogram, applicationBlockingLatenciesHistogram, - deadlineHistogram, + remainingDeadlineHistogram, connectivityErrorCounter, retryCounter); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 23089670d1..4c3fd7a42d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java @@ -22,7 +22,6 @@ import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.StatusCode; import com.google.api.gax.rpc.StatusCode.Code; -import com.google.api.gax.rpc.internal.ApiCallContextOptions; import com.google.bigtable.v2.AuthorizedViewName; import com.google.bigtable.v2.CheckAndMutateRowRequest; import com.google.bigtable.v2.MutateRowRequest; @@ -51,7 +50,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; -import org.threeten.bp.Duration; /** Utilities to help integrating with OpenCensus. */ @InternalApi("For internal use only") @@ -220,15 +218,13 @@ static void recordMetricsFromMetadata( * io.grpc.ClientStreamTracer} to the callContext. */ static GrpcCallContext injectBigtableStreamTracer( - ApiCallContext context, GrpcResponseMetadata responseMetadata, BigtableTracer tracer, ApiCallContext.Key deadlineKey) { + ApiCallContext context, GrpcResponseMetadata responseMetadata, BigtableTracer tracer) { if (context instanceof GrpcCallContext) { GrpcCallContext callContext = (GrpcCallContext) context; CallOptions callOptions = callContext.getCallOptions(); - long deadline = callContext.getOption(deadlineKey); return responseMetadata.addHandlers( callContext.withCallOptions( - callOptions.withStreamTracerFactory( - new BigtableGrpcStreamTracer.Factory(tracer)))); + callOptions.withStreamTracerFactory(new BigtableGrpcStreamTracer.Factory(tracer)))); } else { // context should always be an instance of GrpcCallContext. If not throw an exception // so we can see what class context is. From 74b9ae59d9747eccc9d0837025ce5c939f015667 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 15:11:56 -0400 Subject: [PATCH 26/41] Set the operation timeout on the call context for each callable --- .../data/v2/stub/EnhancedBigtableStub.java | 78 ++++++++++++++++--- .../data/v2/stub/metrics/BigtableTracer.java | 4 +- .../BigtableTracerStreamingCallable.java | 3 +- .../metrics/BigtableTracerUnaryCallable.java | 4 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 6 +- .../metrics/BuiltinMetricsTracerFactory.java | 5 +- 6 files changed, 81 insertions(+), 19 deletions(-) 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 716d9eb8db..4b5e315978 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 @@ -541,8 +541,13 @@ public ServerStreamingCallable createReadRowsCallable( new TracedServerStreamingCallable<>( readRowsUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext(). - withOption(deadlineKey, (long) settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -578,7 +583,13 @@ public UnaryCallable createReadRowCallable(RowAdapter new TracedUnaryCallable<>( firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -697,7 +708,13 @@ private UnaryCallable> createBulkReadRowsCallable( UnaryCallable> traced = new TracedUnaryCallable<>(tracedBatcher, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -956,7 +973,13 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { new TracedUnaryCallable<>( tracedBatcherUnaryCallable, clientContext.getTracerFactory(), spanName); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1225,7 +1248,13 @@ public Map extract( ServerStreamingCallable traced = new TracedServerStreamingCallable<>(retrying, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1305,7 +1334,13 @@ public Map extract( new TracedServerStreamingCallable<>( readChangeStreamUserCallable, clientContext.getTracerFactory(), span); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1391,7 +1426,18 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { new TracedServerStreamingCallable<>(retries, clientContext.getTracerFactory(), span); return new ExecuteQueryCallable( - traced.withDefaultCallContext(clientContext.getDefaultCallContext()), requestContext); + traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings + .readRowsSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis())), + requestContext); } /** @@ -1404,7 +1450,13 @@ private UnaryCallable createUserFacin UnaryCallable traced = new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } private UnaryCallable createPingAndWarmCallable() { @@ -1423,7 +1475,13 @@ public Map extract(PingAndWarmRequest request) { }) .build(), Collections.emptySet()); - return pingAndWarm.withDefaultCallContext(clientContext.getDefaultCallContext()); + return pingAndWarm.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + deadlineKey, + (long) + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } private UnaryCallable withRetries( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 4d59d17e9e..29e7ab57c4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -95,8 +95,8 @@ public void grpcMessageSent() { } /** - * Record the deadline when the request is sent to the Bigtable server. This will be - * called in BuiltinMetricsTracer. + * Record the deadline when the request is sent to the Bigtable server. This will be called in + * BuiltinMetricsTracer. */ public void setRemainingDeadline(long deadline) { // noop diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 9bd1ed94e4..38612a6ec2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -51,7 +51,8 @@ public class BigtableTracerStreamingCallable private final ApiCallContext.Key deadlineKey; public BigtableTracerStreamingCallable( - @Nonnull ServerStreamingCallable callable, ApiCallContext.Key deadlineKey) { + @Nonnull ServerStreamingCallable callable, + ApiCallContext.Key deadlineKey) { this.innerCallable = Preconditions.checkNotNull(callable, "Inner callable must be set"); this.deadlineKey = deadlineKey; } 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 a26b7bdbc7..b32158c65c 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 @@ -48,7 +48,9 @@ public class BigtableTracerUnaryCallable private final UnaryCallable innerCallable; private final ApiCallContext.Key deadlineKey; - public BigtableTracerUnaryCallable(@Nonnull UnaryCallable innerCallable, ApiCallContext.Key deadlineKey) { + public BigtableTracerUnaryCallable( + @Nonnull UnaryCallable innerCallable, + ApiCallContext.Key deadlineKey) { this.innerCallable = Preconditions.checkNotNull(innerCallable, "Inner callable must be set"); this.deadlineKey = deadlineKey; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 241ab89b66..2e36e14916 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -274,9 +274,9 @@ public void grpcMessageSent() { } /* - In GrpcClientCalls we set the timeout in ApiContext on CallOptions. The timeout in ApiContext will be the attempt - timeout for the first few requests or the remaining operation timeout after retries and back offs. - */ + In GrpcClientCalls we set the timeout in ApiContext on CallOptions. The timeout in ApiContext will be the attempt + timeout for the first few requests or the remaining operation timeout after retries and back offs. + */ @Override public void setRemainingDeadline(long deadline) { this.deadline = deadline - operationTimer.elapsed(TimeUnit.MILLISECONDS); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index 44f76ebae0..bf56bb4818 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -113,7 +113,8 @@ public static BuiltinMetricsTracerFactory create( remainingDeadlineHistogram = meter .histogramBuilder(REMAINING_DEADLINE_NAME) - .setDescription("The remaining deadline when the request is sent to grpc. This will either be the attempt timeout for the first few retries, or the reamining deadline from operation timeout after retries and back offs.") + .setDescription( + "The remaining deadline when the request is sent to grpc. This will either be the attempt timeout for the first few retries, or the reamining deadline from operation timeout after retries and back offs.") .setUnit(MILLISECOND) .build(); connectivityErrorCounter = @@ -143,7 +144,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op firstResponseLatenciesHistogram, clientBlockingLatenciesHistogram, applicationBlockingLatenciesHistogram, - remainingDeadlineHistogram, + remainingDeadlineHistogram, connectivityErrorCounter, retryCounter); } From 7e788574960ef57c62c0090d1aa62f07e409ff21 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 15:14:09 -0400 Subject: [PATCH 27/41] Remove unnecessary long conversion, format code --- .../data/v2/stub/EnhancedBigtableStub.java | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-) 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 4b5e315978..d9a4d1ff84 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 @@ -546,8 +546,7 @@ public ServerStreamingCallable createReadRowsCallable( .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -588,8 +587,7 @@ public UnaryCallable createReadRowCallable(RowAdapter .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -713,8 +711,7 @@ private UnaryCallable> createBulkReadRowsCallable( .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -978,8 +975,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1253,8 +1249,7 @@ public Map extract( .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1339,8 +1334,7 @@ public Map extract( .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1431,12 +1425,7 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings - .readRowsSettings() - .getRetrySettings() - .getTotalTimeout() - .toMillis())), + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())), requestContext); } @@ -1455,8 +1444,7 @@ private UnaryCallable createUserFacin .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } private UnaryCallable createPingAndWarmCallable() { @@ -1480,8 +1468,7 @@ public Map extract(PingAndWarmRequest request) { .getDefaultCallContext() .withOption( deadlineKey, - (long) - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } private UnaryCallable withRetries( From 1a66d9e795bc41420167331499babda0d1f59611 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 15:57:09 -0400 Subject: [PATCH 28/41] Make the deadline key a public member of Bigtable Tracer --- .../data/v2/stub/EnhancedBigtableStub.java | 39 +++++++++---------- .../data/v2/stub/metrics/BigtableTracer.java | 2 + .../BigtableTracerStreamingCallable.java | 7 +--- .../metrics/BigtableTracerUnaryCallable.java | 8 +--- .../metrics/BuiltinMetricsTracerTest.java | 2 +- 5 files changed, 26 insertions(+), 32 deletions(-) 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 d9a4d1ff84..f21f723a2a 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 @@ -39,7 +39,6 @@ import com.google.api.gax.retrying.RetryAlgorithm; import com.google.api.gax.retrying.RetryingExecutorWithContext; import com.google.api.gax.retrying.ScheduledRetryingExecutor; -import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.Callables; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.RequestParamsExtractor; @@ -105,6 +104,7 @@ 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.BigtableTracer; 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; @@ -186,7 +186,6 @@ public class EnhancedBigtableStub implements AutoCloseable { private static final long FLOW_CONTROL_ADJUSTING_INTERVAL_MS = TimeUnit.SECONDS.toMillis(20); private final EnhancedBigtableStubSettings settings; private final ClientContext clientContext; - private final ApiCallContext.Key deadlineKey = ApiCallContext.Key.create("deadline"); private final boolean closeClientContext; private final RequestContext requestContext; @@ -545,7 +544,7 @@ public ServerStreamingCallable createReadRowsCallable( clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -586,7 +585,7 @@ public UnaryCallable createReadRowCallable(RowAdapter clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -659,7 +658,7 @@ public Map extract(ReadRowsRequest readRowsRequest) { Callables.watched(merging, innerSettings, clientContext); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched, deadlineKey); + new BigtableTracerStreamingCallable<>(watched); // Retry logic is split into 2 parts to workaround a rare edge case described in // ReadRowsRetryCompletedCallable @@ -710,7 +709,7 @@ private UnaryCallable> createBulkReadRowsCallable( clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -757,7 +756,7 @@ public Map extract( withStatsHeaders = new StatsHeadersUnaryCallable<>(spoolable); UnaryCallable> - withBigtableTracer = new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); + withBigtableTracer = new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable> retryable = withRetries(withBigtableTracer, settings.sampleRowKeysSettings()); @@ -847,7 +846,7 @@ public Map extract(MutateRowRequest mutateRowRequest) { new StatsHeadersUnaryCallable<>(base); UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); + new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable retrying = withRetries(withBigtableTracer, settings.mutateRowSettings()); @@ -914,7 +913,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { new ConvertExceptionCallable<>(callable); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(convertException, deadlineKey); + new BigtableTracerStreamingCallable<>(convertException); BasicResultRetryAlgorithm resultRetryAlgorithm; if (settings.getEnableRetryInfo()) { @@ -974,7 +973,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -1109,7 +1108,7 @@ public Map extract( new StatsHeadersUnaryCallable<>(base); UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); + new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable retrying = withRetries(withBigtableTracer, settings.checkAndMutateRowSettings()); @@ -1155,7 +1154,7 @@ public Map extract(ReadModifyWriteRowRequest request) { String methodName = "ReadModifyWriteRow"; UnaryCallable withBigtableTracer = - new BigtableTracerUnaryCallable<>(withStatsHeaders, deadlineKey); + new BigtableTracerUnaryCallable<>(withStatsHeaders); UnaryCallable retrying = withRetries(withBigtableTracer, settings.readModifyWriteRowSettings()); @@ -1235,7 +1234,7 @@ public Map extract( Callables.watched(convertException, innerSettings, clientContext); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched, deadlineKey); + new BigtableTracerStreamingCallable<>(watched); ServerStreamingCallable retrying = withRetries(withBigtableTracer, innerSettings); @@ -1248,7 +1247,7 @@ public Map extract( clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -1315,7 +1314,7 @@ public Map extract( Callables.watched(merging, innerSettings, clientContext); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(watched, deadlineKey); + new BigtableTracerStreamingCallable<>(watched); ServerStreamingCallable readChangeStreamCallable = withRetries(withBigtableTracer, innerSettings); @@ -1333,7 +1332,7 @@ public Map extract( clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -1393,7 +1392,7 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { new SqlRowMergingCallable(withMetadataObserver); ServerStreamingCallable withBigtableTracer = - new BigtableTracerStreamingCallable<>(merging, deadlineKey); + new BigtableTracerStreamingCallable<>(merging); ServerStreamingCallSettings retrySettings = ServerStreamingCallSettings.newBuilder() @@ -1424,7 +1423,7 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())), requestContext); } @@ -1443,7 +1442,7 @@ private UnaryCallable createUserFacin clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } @@ -1467,7 +1466,7 @@ public Map extract(PingAndWarmRequest request) { clientContext .getDefaultCallContext() .withOption( - deadlineKey, + BigtableTracer.DEADLINE_KEY, settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index 29e7ab57c4..d28db4fa47 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -30,6 +30,8 @@ public class BigtableTracer extends BaseApiTracer { private volatile int attempt = 0; + public static final ApiCallContext.Key DEADLINE_KEY = ApiCallContext.Key.create("deadline"); + @Override public void attemptStarted(int attemptNumber) { this.attempt = attemptNumber; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 38612a6ec2..657e1f4ae7 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -48,13 +48,10 @@ public class BigtableTracerStreamingCallable extends ServerStreamingCallable { private final ServerStreamingCallable innerCallable; - private final ApiCallContext.Key deadlineKey; public BigtableTracerStreamingCallable( - @Nonnull ServerStreamingCallable callable, - ApiCallContext.Key deadlineKey) { + @Nonnull ServerStreamingCallable callable) { this.innerCallable = Preconditions.checkNotNull(callable, "Inner callable must be set"); - this.deadlineKey = deadlineKey; } @Override @@ -67,7 +64,7 @@ public void call( new BigtableTracerResponseObserver<>( responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); GrpcCallContext callContext = (GrpcCallContext) context; - long deadline = callContext.getOption(deadlineKey); + long deadline = callContext.getOption(BigtableTracer.DEADLINE_KEY); ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); innerCallable.call( request, 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 b32158c65c..b4edff578c 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 @@ -46,13 +46,9 @@ public class BigtableTracerUnaryCallable extends UnaryCallable { private final UnaryCallable innerCallable; - private final ApiCallContext.Key deadlineKey; - public BigtableTracerUnaryCallable( - @Nonnull UnaryCallable innerCallable, - ApiCallContext.Key deadlineKey) { + public BigtableTracerUnaryCallable(@Nonnull UnaryCallable innerCallable) { this.innerCallable = Preconditions.checkNotNull(innerCallable, "Inner callable must be set"); - this.deadlineKey = deadlineKey; } @Override @@ -64,7 +60,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) new BigtableTracerUnaryCallback( (BigtableTracer) context.getTracer(), responseMetadata); GrpcCallContext callContext = (GrpcCallContext) context; - long deadline = callContext.getOption(deadlineKey); + long deadline = callContext.getOption(BigtableTracer.DEADLINE_KEY); ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); ApiFuture future = innerCallable.futureCall( 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 9f6a946389..efc23f40c2 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 @@ -736,7 +736,7 @@ public void testRemainingDeadline() { long remainingDeadline = getAggregatedValue(deadlineMetric, attributes); System.out.println("remaining deadline @ end of test: " + remainingDeadline + "\n"); - assertThat(remainingDeadline).isIn(Range.closed((long) 8400, (long) 8600)); + assertThat(remainingDeadline).isIn(Range.closed((long) 8400, (long) 8700)); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 439e081026444e33e98ffe9121cda2e6efc52685 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 16:43:53 -0400 Subject: [PATCH 29/41] Fix settings --- .../data/v2/stub/EnhancedBigtableStub.java | 96 +++++++++++++++---- 1 file changed, 79 insertions(+), 17 deletions(-) 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 f21f723a2a..5302973139 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 @@ -586,7 +586,7 @@ public UnaryCallable createReadRowCallable(RowAdapter .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.readRowSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -710,7 +710,7 @@ private UnaryCallable> createBulkReadRowsCallable( .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.bulkReadRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -782,7 +782,18 @@ private UnaryCallable> createSampleRowKeysCallable() { UnaryCallable> baseCallable = createSampleRowKeysBaseCallable(); return createUserFacingUnaryCallable( - methodName, new SampleRowKeysCallable(baseCallable, requestContext)); + methodName, + new SampleRowKeysCallable(baseCallable, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.DEADLINE_KEY, + settings + .sampleRowKeysSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis()))); } /** @@ -805,7 +816,18 @@ private UnaryCallable> createSampleRowKeysCallable() { UnaryCallable> baseCallable = createSampleRowKeysBaseCallable(); return createUserFacingUnaryCallable( - methodName, new SampleRowKeysCallableWithRequest(baseCallable, requestContext)); + methodName, + new SampleRowKeysCallableWithRequest(baseCallable, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.DEADLINE_KEY, + settings + .sampleRowKeysSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis()))); } /** @@ -852,7 +874,18 @@ public Map extract(MutateRowRequest mutateRowRequest) { withRetries(withBigtableTracer, settings.mutateRowSettings()); return createUserFacingUnaryCallable( - methodName, new MutateRowCallable(retrying, requestContext)); + methodName, + new MutateRowCallable(retrying, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.DEADLINE_KEY, + settings + .mutateRowSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis()))); } /** @@ -974,7 +1007,7 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.bulkMutateRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); } /** @@ -1114,7 +1147,18 @@ public Map extract( withRetries(withBigtableTracer, settings.checkAndMutateRowSettings()); return createUserFacingUnaryCallable( - methodName, new CheckAndMutateRowCallable(retrying, requestContext)); + methodName, + new CheckAndMutateRowCallable(retrying, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.DEADLINE_KEY, + settings + .checkAndMutateRowSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis()))); } /** @@ -1160,7 +1204,18 @@ public Map extract(ReadModifyWriteRowRequest request) { withRetries(withBigtableTracer, settings.readModifyWriteRowSettings()); return createUserFacingUnaryCallable( - methodName, new ReadModifyWriteRowCallable(retrying, requestContext)); + methodName, + new ReadModifyWriteRowCallable(retrying, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.DEADLINE_KEY, + settings + .readModifyWriteRowSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis()))); } /** @@ -1248,7 +1303,11 @@ public Map extract( .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings + .generateInitialChangeStreamPartitionsSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis())); } /** @@ -1333,7 +1392,11 @@ public Map extract( .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings + .readChangeStreamSettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis())); } /** @@ -1424,7 +1487,11 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())), + settings + .executeQuerySettings() + .getRetrySettings() + .getTotalTimeout() + .toMillis())), requestContext); } @@ -1438,12 +1505,7 @@ private UnaryCallable createUserFacin UnaryCallable traced = new TracedUnaryCallable<>(inner, clientContext.getTracerFactory(), getSpanName(methodName)); - return traced.withDefaultCallContext( - clientContext - .getDefaultCallContext() - .withOption( - BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + return traced.withDefaultCallContext(clientContext.getDefaultCallContext()); } private UnaryCallable createPingAndWarmCallable() { From 18ffd5932fe27d6ffce231ae22b9b83e2ca24de2 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Fri, 25 Oct 2024 16:45:31 -0400 Subject: [PATCH 30/41] Missed one setting --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5302973139..e708635d95 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 @@ -1529,7 +1529,7 @@ public Map extract(PingAndWarmRequest request) { .getDefaultCallContext() .withOption( BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + settings.pingAndWarmSettings().getRetrySettings().getTotalTimeout().toMillis())); } private UnaryCallable withRetries( From b04b8ca92e7cb5ed88d7deb731afc0fa48179889 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 28 Oct 2024 12:07:29 -0400 Subject: [PATCH 31/41] Address PR comments --- .../data/v2/stub/EnhancedBigtableStub.java | 75 +++++++------------ .../data/v2/stub/metrics/BigtableTracer.java | 12 ++- .../BigtableTracerStreamingCallable.java | 5 +- .../metrics/BigtableTracerUnaryCallable.java | 5 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 9 ++- .../metrics/BuiltinMetricsTracerFactory.java | 2 +- .../data/v2/stub/metrics/CompositeTracer.java | 4 +- 7 files changed, 48 insertions(+), 64 deletions(-) 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 e708635d95..9144a0327d 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 @@ -544,8 +544,8 @@ public ServerStreamingCallable createReadRowsCallable( clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings.readRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowsSettings().getRetrySettings().getTotalTimeout())); } /** @@ -585,8 +585,8 @@ public UnaryCallable createReadRowCallable(RowAdapter clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout().toMillis())); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowSettings().getRetrySettings().getTotalTimeout())); } /** @@ -709,8 +709,8 @@ private UnaryCallable> createBulkReadRowsCallable( clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings.bulkReadRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.bulkReadRowsSettings().getRetrySettings().getTotalTimeout())); } /** @@ -788,12 +788,8 @@ private UnaryCallable> createSampleRowKeysCallable() { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings - .sampleRowKeysSettings() - .getRetrySettings() - .getTotalTimeout() - .toMillis()))); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout()))); } /** @@ -822,12 +818,8 @@ private UnaryCallable> createSampleRowKeysCallable() { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings - .sampleRowKeysSettings() - .getRetrySettings() - .getTotalTimeout() - .toMillis()))); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout()))); } /** @@ -880,12 +872,8 @@ public Map extract(MutateRowRequest mutateRowRequest) { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings - .mutateRowSettings() - .getRetrySettings() - .getTotalTimeout() - .toMillis()))); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.mutateRowSettings().getRetrySettings().getTotalTimeout()))); } /** @@ -1006,8 +994,8 @@ public Map extract(MutateRowsRequest mutateRowsRequest) { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings.bulkMutateRowsSettings().getRetrySettings().getTotalTimeout().toMillis())); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.bulkMutateRowsSettings().getRetrySettings().getTotalTimeout())); } /** @@ -1153,12 +1141,11 @@ public Map extract( clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, + BigtableTracer.OPERATION_TIMEOUT_KEY, settings .checkAndMutateRowSettings() .getRetrySettings() - .getTotalTimeout() - .toMillis()))); + .getTotalTimeout()))); } /** @@ -1210,12 +1197,11 @@ public Map extract(ReadModifyWriteRowRequest request) { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, + BigtableTracer.OPERATION_TIMEOUT_KEY, settings .readModifyWriteRowSettings() .getRetrySettings() - .getTotalTimeout() - .toMillis()))); + .getTotalTimeout()))); } /** @@ -1302,12 +1288,11 @@ public Map extract( clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, + BigtableTracer.OPERATION_TIMEOUT_KEY, settings .generateInitialChangeStreamPartitionsSettings() .getRetrySettings() - .getTotalTimeout() - .toMillis())); + .getTotalTimeout())); } /** @@ -1391,12 +1376,8 @@ public Map extract( clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings - .readChangeStreamSettings() - .getRetrySettings() - .getTotalTimeout() - .toMillis())); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readChangeStreamSettings().getRetrySettings().getTotalTimeout())); } /** @@ -1486,12 +1467,8 @@ public Map extract(ExecuteQueryRequest executeQueryRequest) { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings - .executeQuerySettings() - .getRetrySettings() - .getTotalTimeout() - .toMillis())), + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.executeQuerySettings().getRetrySettings().getTotalTimeout())), requestContext); } @@ -1528,8 +1505,8 @@ public Map extract(PingAndWarmRequest request) { clientContext .getDefaultCallContext() .withOption( - BigtableTracer.DEADLINE_KEY, - settings.pingAndWarmSettings().getRetrySettings().getTotalTimeout().toMillis())); + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.pingAndWarmSettings().getRetrySettings().getTotalTimeout())); } private UnaryCallable withRetries( diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java index d28db4fa47..ad77b207b3 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java @@ -16,10 +16,12 @@ package com.google.cloud.bigtable.data.v2.stub.metrics; import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.BaseApiTracer; import javax.annotation.Nullable; +import org.threeten.bp.Duration; /** * A Bigtable specific {@link ApiTracer} that includes additional contexts. This class is a base @@ -30,7 +32,9 @@ public class BigtableTracer extends BaseApiTracer { private volatile int attempt = 0; - public static final ApiCallContext.Key DEADLINE_KEY = ApiCallContext.Key.create("deadline"); + @InternalApi("for internal use only") + public static final ApiCallContext.Key OPERATION_TIMEOUT_KEY = + ApiCallContext.Key.create("OPERATION_TIMEOUT"); @Override public void attemptStarted(int attemptNumber) { @@ -97,10 +101,10 @@ public void grpcMessageSent() { } /** - * Record the deadline when the request is sent to the Bigtable server. This will be called in - * BuiltinMetricsTracer. + * Record the operation timeout from user settings for calculating remaining deadline. This will + * be called in BuiltinMetricsTracer. */ - public void setRemainingDeadline(long deadline) { + public void setOperationTimeout(Duration operationTimeout) { // noop } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index 657e1f4ae7..e16718f7ce 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -27,6 +27,7 @@ import com.google.common.base.Stopwatch; import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; +import org.threeten.bp.Duration; /** * This callable will @@ -64,8 +65,8 @@ public void call( new BigtableTracerResponseObserver<>( responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); GrpcCallContext callContext = (GrpcCallContext) context; - long deadline = callContext.getOption(BigtableTracer.DEADLINE_KEY); - ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); + Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); + ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); innerCallable.call( request, innerObserver, 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 b4edff578c..2dc612d9d4 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 @@ -26,6 +26,7 @@ import com.google.common.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; import javax.annotation.Nonnull; +import org.threeten.bp.Duration; /** * This callable will: @@ -60,8 +61,8 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) new BigtableTracerUnaryCallback( (BigtableTracer) context.getTracer(), responseMetadata); GrpcCallContext callContext = (GrpcCallContext) context; - long deadline = callContext.getOption(BigtableTracer.DEADLINE_KEY); - ((BigtableTracer) context.getTracer()).setRemainingDeadline(deadline); + Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); + ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); ApiFuture future = innerCallable.futureCall( request, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 2e36e14916..e9fafd33e9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -86,7 +86,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private Long serverLatencies = null; private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); - private long deadline = 0; + private Duration operationTimeout = Duration.ofMillis(0); // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -278,8 +278,8 @@ public void grpcMessageSent() { timeout for the first few requests or the remaining operation timeout after retries and back offs. */ @Override - public void setRemainingDeadline(long deadline) { - this.deadline = deadline - operationTimer.elapsed(TimeUnit.MILLISECONDS); + public void setOperationTimeout(Duration operationTimeout) { + this.operationTimeout = operationTimeout; } @Override @@ -372,7 +372,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - deadlineHistogram.record(deadline, attributes); + deadlineHistogram.record( + operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); if (serverLatencies != null) { serverLatenciesHistogram.record(serverLatencies, attributes); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java index bf56bb4818..18d3a3ace9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -114,7 +114,7 @@ public static BuiltinMetricsTracerFactory create( meter .histogramBuilder(REMAINING_DEADLINE_NAME) .setDescription( - "The remaining deadline when the request is sent to grpc. This will either be the attempt timeout for the first few retries, or the reamining deadline from operation timeout after retries and back offs.") + "The remaining deadline when the request is sent to grpc. This will either be the operation timeout, or the remaining deadline from operation timeout after retries and back offs.") .setUnit(MILLISECOND) .build(); connectivityErrorCounter = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java index be1b83217b..6135b2347f 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java @@ -234,9 +234,9 @@ public void grpcMessageSent() { } @Override - public void setRemainingDeadline(long deadline) { + public void setOperationTimeout(Duration operationTimeout) { for (BigtableTracer tracer : bigtableTracers) { - tracer.setRemainingDeadline(deadline); + tracer.setOperationTimeout(operationTimeout); } } } From d63d640bd80cae18996586619cda24604d1b07f4 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 28 Oct 2024 12:10:22 -0400 Subject: [PATCH 32/41] Rename variable to make consistent --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index e9fafd33e9..c6d1ac3f6d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -98,7 +98,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final DoubleHistogram firstResponseLatenciesHistogram; private final DoubleHistogram clientBlockingLatenciesHistogram; private final DoubleHistogram applicationBlockingLatenciesHistogram; - private final DoubleHistogram deadlineHistogram; + private final DoubleHistogram remainingDeadlineHistogram; private final LongCounter connectivityErrorCounter; private final LongCounter retryCounter; @@ -125,7 +125,7 @@ class BuiltinMetricsTracer extends BigtableTracer { this.firstResponseLatenciesHistogram = firstResponseLatenciesHistogram; this.clientBlockingLatenciesHistogram = clientBlockingLatenciesHistogram; this.applicationBlockingLatenciesHistogram = applicationBlockingLatenciesHistogram; - this.deadlineHistogram = deadlineHistogram; + this.remainingDeadlineHistogram = deadlineHistogram; this.connectivityErrorCounter = connectivityErrorCounter; this.retryCounter = retryCounter; } @@ -372,7 +372,7 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - deadlineHistogram.record( + remainingDeadlineHistogram.record( operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); if (serverLatencies != null) { From f6b6c70c092f5e013cb2e3bbec18ce35d28b5bd8 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 28 Oct 2024 15:35:08 -0400 Subject: [PATCH 33/41] Add nonzero check for remaining deadline --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 15 +++++--- .../metrics/BuiltinMetricsTracerTest.java | 36 ++++++++++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index c6d1ac3f6d..9cf35b1ea4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -37,6 +37,8 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -46,6 +48,8 @@ */ class BuiltinMetricsTracer extends BigtableTracer { + private static final Logger logger = Logger.getLogger(BuiltinMetricsTracer.class.getName()); + private static final String NAME = "java-bigtable/" + Version.VERSION; private final OperationType operationType; private final SpanName spanName; @@ -274,8 +278,7 @@ public void grpcMessageSent() { } /* - In GrpcClientCalls we set the timeout in ApiContext on CallOptions. The timeout in ApiContext will be the attempt - timeout for the first few requests or the remaining operation timeout after retries and back offs. + This is called by BigtableTracerCallables that sets operation timeout from user settings. */ @Override public void setOperationTimeout(Duration operationTimeout) { @@ -372,8 +375,12 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - remainingDeadlineHistogram.record( - operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS), attributes); + long remainingDeadline = operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); + if (remainingDeadline >= 0) { + remainingDeadlineHistogram.record(remainingDeadline); + } else { + logger.log(Level.WARNING, "The remaining deadline was less than 0: " + remainingDeadline); + } if (serverLatencies != null) { serverLatenciesHistogram.record(serverLatencies, attributes); 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 efc23f40c2..20fe5dea12 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 @@ -90,6 +90,7 @@ import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import io.opentelemetry.sdk.metrics.View; +import io.opentelemetry.sdk.metrics.data.HistogramPointData; import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import java.io.IOException; @@ -104,6 +105,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.junit.After; import org.junit.Assert; @@ -722,7 +724,28 @@ public void testRemainingDeadline() { stub.readRowsCallable().all().call(Query.create(TABLE)); MetricData deadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); - Attributes attributes = + Attributes retryAttributes = + baseAttributes + .toBuilder() + .put(STATUS_KEY, "UNAVAILABLE") + .put(TABLE_ID_KEY, TABLE) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(ZONE_ID_KEY, "global") + .put(CLUSTER_ID_KEY, "unspecified") + .put(STREAMING_KEY, true) + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); + + HistogramPointData retryHistogramPointData = + deadlineMetric.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(retryAttributes)) + .collect(Collectors.toList()) + .get(0); + + double retryRemainingDeadline = retryHistogramPointData.getSum(); + assertThat(retryRemainingDeadline).isWithin(50).of(8500); + + Attributes okAttributes = baseAttributes .toBuilder() .put(STATUS_KEY, "OK") @@ -734,9 +757,14 @@ public void testRemainingDeadline() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - long remainingDeadline = getAggregatedValue(deadlineMetric, attributes); - System.out.println("remaining deadline @ end of test: " + remainingDeadline + "\n"); - assertThat(remainingDeadline).isIn(Range.closed((long) 8400, (long) 8700)); + HistogramPointData okHistogramPointData = + deadlineMetric.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(okAttributes)) + .collect(Collectors.toList()) + .get(0); + double okRemainingDeadline = okHistogramPointData.getSum(); + assertThat(okRemainingDeadline).isLessThan(retryRemainingDeadline); + assertThat(okRemainingDeadline).isWithin(50).of(8375); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 0fef2b8e99fbf8c1cebe133c4fe8e395cb7fea5d Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 30 Oct 2024 11:09:34 -0400 Subject: [PATCH 34/41] Set remaining operation timeout in attemptStart --- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 13 +++++++++---- .../v2/stub/metrics/BuiltinMetricsTracerTest.java | 9 ++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 9cf35b1ea4..2ee3cc5b18 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -91,6 +91,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final AtomicLong grpcMessageSentDelay = new AtomicLong(0); private Duration operationTimeout = Duration.ofMillis(0); + private long remainingOperationTimeout = 0; // OpenCensus (and server) histogram buckets use [start, end), however OpenTelemetry uses (start, // end]. To work around this, we measure all the latencies in nanoseconds and convert them @@ -178,6 +179,9 @@ public void attemptStarted(Object request, int attemptNumber) { } } } + if (attemptCount > 1) { + remainingOperationTimeout = operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); + } } @Override @@ -375,11 +379,12 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - long remainingDeadline = operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); - if (remainingDeadline >= 0) { - remainingDeadlineHistogram.record(remainingDeadline); + if (attemptCount == 1) { + remainingDeadlineHistogram.record(operationTimeout.toMillis(), attributes); + } else if (remainingOperationTimeout >= 0){ + remainingDeadlineHistogram.record(remainingOperationTimeout, attributes); } else { - logger.log(Level.WARNING, "The remaining deadline was less than 0: " + remainingDeadline); + logger.log(Level.WARNING, "The remaining deadline was less than 0: " + remainingOperationTimeout); } if (serverLatencies != null) { 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 20fe5dea12..9589a758e8 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 @@ -735,7 +735,6 @@ public void testRemainingDeadline() { .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - HistogramPointData retryHistogramPointData = deadlineMetric.getHistogramData().getPoints().stream() .filter(pd -> pd.getAttributes().equals(retryAttributes)) @@ -743,7 +742,8 @@ public void testRemainingDeadline() { .get(0); double retryRemainingDeadline = retryHistogramPointData.getSum(); - assertThat(retryRemainingDeadline).isWithin(50).of(8500); + // The retry remaining deadline should be equivalent to the original timeout. + assertThat(retryRemainingDeadline).isEqualTo(9000); Attributes okAttributes = baseAttributes @@ -756,15 +756,14 @@ public void testRemainingDeadline() { .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); - HistogramPointData okHistogramPointData = deadlineMetric.getHistogramData().getPoints().stream() .filter(pd -> pd.getAttributes().equals(okAttributes)) .collect(Collectors.toList()) .get(0); + double okRemainingDeadline = okHistogramPointData.getSum(); - assertThat(okRemainingDeadline).isLessThan(retryRemainingDeadline); - assertThat(okRemainingDeadline).isWithin(50).of(8375); + assertThat(okRemainingDeadline).isWithin(50).of(8500); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 4a9ef83fad604e79ca345f9aad27eb614884a302 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 30 Oct 2024 11:11:37 -0400 Subject: [PATCH 35/41] Add clarifying comment --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 2ee3cc5b18..232bbfcf54 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -179,6 +179,7 @@ public void attemptStarted(Object request, int attemptNumber) { } } } + // OperationTimeout is only set after the first attempt. if (attemptCount > 1) { remainingOperationTimeout = operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); } From f409ea7d0ceb0e16597bba1a2f1e59422f7fa761 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 30 Oct 2024 11:12:21 -0400 Subject: [PATCH 36/41] Format code --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 8 +++-- .../metrics/BuiltinMetricsTracerTest.java | 36 +++++++++---------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 232bbfcf54..e9a0692568 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -181,7 +181,8 @@ public void attemptStarted(Object request, int attemptNumber) { } // OperationTimeout is only set after the first attempt. if (attemptCount > 1) { - remainingOperationTimeout = operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); + remainingOperationTimeout = + operationTimeout.toMillis() - operationTimer.elapsed(TimeUnit.MILLISECONDS); } } @@ -382,10 +383,11 @@ private void recordAttemptCompletion(@Nullable Throwable status) { if (attemptCount == 1) { remainingDeadlineHistogram.record(operationTimeout.toMillis(), attributes); - } else if (remainingOperationTimeout >= 0){ + } else if (remainingOperationTimeout >= 0) { remainingDeadlineHistogram.record(remainingOperationTimeout, attributes); } else { - logger.log(Level.WARNING, "The remaining deadline was less than 0: " + remainingOperationTimeout); + logger.log( + Level.WARNING, "The remaining deadline was less than 0: " + remainingOperationTimeout); } if (serverLatencies != null) { 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 9589a758e8..a5affdf1d7 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 @@ -725,21 +725,21 @@ public void testRemainingDeadline() { MetricData deadlineMetric = getMetricData(metricReader, REMAINING_DEADLINE_NAME); Attributes retryAttributes = - baseAttributes - .toBuilder() - .put(STATUS_KEY, "UNAVAILABLE") - .put(TABLE_ID_KEY, TABLE) - .put(METHOD_KEY, "Bigtable.ReadRows") - .put(ZONE_ID_KEY, "global") - .put(CLUSTER_ID_KEY, "unspecified") - .put(STREAMING_KEY, true) - .put(CLIENT_NAME_KEY, CLIENT_NAME) - .build(); + baseAttributes + .toBuilder() + .put(STATUS_KEY, "UNAVAILABLE") + .put(TABLE_ID_KEY, TABLE) + .put(METHOD_KEY, "Bigtable.ReadRows") + .put(ZONE_ID_KEY, "global") + .put(CLUSTER_ID_KEY, "unspecified") + .put(STREAMING_KEY, true) + .put(CLIENT_NAME_KEY, CLIENT_NAME) + .build(); HistogramPointData retryHistogramPointData = - deadlineMetric.getHistogramData().getPoints().stream() - .filter(pd -> pd.getAttributes().equals(retryAttributes)) - .collect(Collectors.toList()) - .get(0); + deadlineMetric.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(retryAttributes)) + .collect(Collectors.toList()) + .get(0); double retryRemainingDeadline = retryHistogramPointData.getSum(); // The retry remaining deadline should be equivalent to the original timeout. @@ -757,10 +757,10 @@ public void testRemainingDeadline() { .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); HistogramPointData okHistogramPointData = - deadlineMetric.getHistogramData().getPoints().stream() - .filter(pd -> pd.getAttributes().equals(okAttributes)) - .collect(Collectors.toList()) - .get(0); + deadlineMetric.getHistogramData().getPoints().stream() + .filter(pd -> pd.getAttributes().equals(okAttributes)) + .collect(Collectors.toList()) + .get(0); double okRemainingDeadline = okHistogramPointData.getSum(); assertThat(okRemainingDeadline).isWithin(50).of(8500); From 83f0df18fdb0a3ce9851fa929eb385d0404912ee Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 4 Nov 2024 11:55:02 -0500 Subject: [PATCH 37/41] Use only java.time.Duration in BuiltinMetricsTracerTest --- .../data/v2/stub/metrics/BuiltinMetricsTracerTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) 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 a5affdf1d7..0494295791 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 @@ -215,18 +215,16 @@ public void sendHeaders(Metadata headers) { .retrySettings() .setInitialRetryDelayDuration(java.time.Duration.ofMillis(200)); - java.time.Duration timeout = java.time.Duration.ofMillis(6000); - java.time.Duration backOffTime = java.time.Duration.ofMillis(10); stubSettingsBuilder .readRowsSettings() .retrySettings() .setTotalTimeoutDuration(Duration.ofMillis(9000)) - .setMaxRpcTimeout(org.threeten.bp.Duration.parse(timeout.toString())) + .setMaxRpcTimeoutDuration(Duration.ofMillis(6000)) .setRpcTimeoutMultiplier(1) - .setInitialRpcTimeout(org.threeten.bp.Duration.parse(timeout.toString())) - .setInitialRetryDelay(org.threeten.bp.Duration.parse(backOffTime.toString())) + .setInitialRpcTimeoutDuration(Duration.ofMillis(6000)) + .setInitialRetryDelayDuration(Duration.ofMillis(10)) .setRetryDelayMultiplier(1) - .setMaxRetryDelay(org.threeten.bp.Duration.parse(backOffTime.toString())); + .setMaxRetryDelayDuration(Duration.ofMillis(10)); stubSettingsBuilder .bulkMutateRowsSettings() From cc882938c8689737127c2e03b84697af9dc3c96e Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Mon, 4 Nov 2024 13:50:39 -0500 Subject: [PATCH 38/41] Loosen test assertions --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0494295791..10f6d98150 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 @@ -761,7 +761,7 @@ public void testRemainingDeadline() { .get(0); double okRemainingDeadline = okHistogramPointData.getSum(); - assertThat(okRemainingDeadline).isWithin(50).of(8500); + assertThat(okRemainingDeadline).isWithin(200).of(8500); } private static class FakeService extends BigtableGrpc.BigtableImplBase { From 549fb9aae0cd2d0862026276246236ac9b64ad3b Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 6 Nov 2024 11:51:43 -0500 Subject: [PATCH 39/41] Use callSettings in UnaryCallable --- .../cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 c930186d8b..8113fb4a5f 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 @@ -1329,7 +1329,7 @@ private Map composeRequestParams( return ImmutableMap.of("table_name", tableName, "app_profile_id", appProfileId); } - private UnaryCallable + private UnaryCallable createUnaryCallable ( MethodDescriptor methodDescriptor, RequestParamsExtractor headerParamsFn, @@ -1386,7 +1386,7 @@ public ApiFuture futureCall(ReqT reqT, ApiCallContext apiCallContext) { return traced.withDefaultCallContext(clientContext.getDefaultCallContext().withOption( BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout())); + callSettings.getRetrySettings().getTotalTimeout())); } private UnaryCallable createUnaryCallableNew( @@ -1417,7 +1417,7 @@ private UnaryCallable createUnar transformed, clientContext.getDefaultCallContext().withOption( BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout()), + callSettings.getRetrySettings().getTotalTimeout()), clientContext.getTracerFactory(), getSpanName(methodDescriptor.getBareMethodName()), /* allowNoResponse= */ false); From 4e66bc2bf5e116d00497a13a54154c74e5c41f45 Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 6 Nov 2024 11:52:54 -0500 Subject: [PATCH 40/41] Format code --- .../data/v2/stub/EnhancedBigtableStub.java | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) 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 8113fb4a5f..c15d989f49 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 @@ -585,9 +585,12 @@ public UnaryCallable createReadRowCallable(RowAdapter UnaryCallable traced = new TracedUnaryCallable<>( firstRow, clientContext.getTracerFactory(), getSpanName("ReadRow")); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext().withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout())); + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowSettings().getRetrySettings().getTotalTimeout())); } else { ServerStreamingCallable readRowsCallable = createReadRowsBaseCallable( @@ -607,9 +610,11 @@ public UnaryCallable createReadRowCallable(RowAdapter return new BigtableUnaryOperationCallable<>( readRowCallable, - clientContext.getDefaultCallContext().withOption( - BigtableTracer.OPERATION_TIMEOUT_KEY, - settings.readRowSettings().getRetrySettings().getTotalTimeout()), + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.readRowSettings().getRetrySettings().getTotalTimeout()), clientContext.getTracerFactory(), getSpanName("ReadRow"), /*allowNoResponses=*/ true); @@ -1329,8 +1334,7 @@ private Map composeRequestParams( return ImmutableMap.of("table_name", tableName, "app_profile_id", appProfileId); } - private UnaryCallable createUnaryCallable - ( + private UnaryCallable createUnaryCallable( MethodDescriptor methodDescriptor, RequestParamsExtractor headerParamsFn, UnaryCallSettings callSettings, @@ -1384,7 +1388,10 @@ public ApiFuture futureCall(ReqT reqT, ApiCallContext apiCallContext) { clientContext.getTracerFactory(), getSpanName(methodDescriptor.getBareMethodName())); - return traced.withDefaultCallContext(clientContext.getDefaultCallContext().withOption( + return traced.withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( BigtableTracer.OPERATION_TIMEOUT_KEY, callSettings.getRetrySettings().getTotalTimeout())); } @@ -1415,7 +1422,9 @@ private UnaryCallable createUnar return new BigtableUnaryOperationCallable<>( transformed, - clientContext.getDefaultCallContext().withOption( + clientContext + .getDefaultCallContext() + .withOption( BigtableTracer.OPERATION_TIMEOUT_KEY, callSettings.getRetrySettings().getTotalTimeout()), clientContext.getTracerFactory(), From bc1dde2027aa602fdcdfdee451ec89faa049fadf Mon Sep 17 00:00:00 2001 From: Derek Yau Date: Wed, 6 Nov 2024 14:42:10 -0500 Subject: [PATCH 41/41] Address PR comments --- .../bigtable/data/v2/stub/EnhancedBigtableStub.java | 9 ++++++++- .../v2/stub/metrics/BigtableTracerStreamingCallable.java | 4 +++- .../v2/stub/metrics/BigtableTracerUnaryCallable.java | 4 +++- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 6 ++++-- 4 files changed, 18 insertions(+), 5 deletions(-) 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 c15d989f49..53ebca2919 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 @@ -800,7 +800,14 @@ public ApiFuture> futureCall(String s, ApiCallContext apiCallCon retryable = withRetries(withBigtableTracer, settings.sampleRowKeysSettings()); return createUserFacingUnaryCallable( - methodName, new SampleRowKeysCallableWithRequest(retryable, requestContext)); + methodName, + new SampleRowKeysCallableWithRequest(retryable, requestContext) + .withDefaultCallContext( + clientContext + .getDefaultCallContext() + .withOption( + BigtableTracer.OPERATION_TIMEOUT_KEY, + settings.sampleRowKeysSettings().getRetrySettings().getTotalTimeout()))); } /** diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java index e16718f7ce..b977a0a2c7 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracerStreamingCallable.java @@ -66,7 +66,9 @@ public void call( responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); GrpcCallContext callContext = (GrpcCallContext) context; Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); - ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + if (deadline != null) { + ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + } innerCallable.call( request, innerObserver, 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 2dc612d9d4..1f000c4639 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 @@ -62,7 +62,9 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) (BigtableTracer) context.getTracer(), responseMetadata); GrpcCallContext callContext = (GrpcCallContext) context; Duration deadline = callContext.getOption(BigtableTracer.OPERATION_TIMEOUT_KEY); - ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + if (deadline != null) { + ((BigtableTracer) context.getTracer()).setOperationTimeout(deadline); + } ApiFuture future = innerCallable.futureCall( request, diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java index 5744c370d5..07583bde52 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -378,11 +378,13 @@ private void recordAttemptCompletion(@Nullable Throwable status) { attemptLatenciesHistogram.record( convertToMs(attemptTimer.elapsed(TimeUnit.NANOSECONDS)), attributes); - if (attemptCount == 1) { + if (attemptCount <= 1) { remainingDeadlineHistogram.record(operationTimeout.toMillis(), attributes); } else if (remainingOperationTimeout >= 0) { remainingDeadlineHistogram.record(remainingOperationTimeout, attributes); - } else { + } else if (operationTimeout.toMillis() != 0) { + // If the operationTimeout is set but remaining deadline is < 0, log a warning. This should + // never happen. logger.log( Level.WARNING, "The remaining deadline was less than 0: " + remainingOperationTimeout); }