fix: fix client blocking latency

googleapis · Sep 25, 2024 · 1efb3ab · 1efb3ab
1 parent 0330d77
commit 1efb3ab
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 14 deletions.
diff --git a/...rc/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java b/...rc/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableGrpcStreamTracer.java
@@ -15,11 +15,8 @@
  */
 package com.google.cloud.bigtable.data.v2.stub.metrics;
 
-import com.google.common.base.Stopwatch;
-import io.grpc.Attributes;
 import io.grpc.ClientStreamTracer;
 import io.grpc.Metadata;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Records the time a request is enqueued in a grpc channel queue. This a bridge between gRPC stream
@@ -28,21 +25,15 @@
  */
 class BigtableGrpcStreamTracer extends ClientStreamTracer {
 
-  private final Stopwatch stopwatch = Stopwatch.createUnstarted();
   private final BigtableTracer tracer;
 
   public BigtableGrpcStreamTracer(BigtableTracer tracer) {
     this.tracer = tracer;
   }
 
-  @Override
-  public void streamCreated(Attributes transportAttrs, Metadata headers) {
-    stopwatch.start();
-  }
-
   @Override
   public void outboundMessageSent(int seqNo, long optionalWireSize, long optionalUncompressedSize) {
-    tracer.grpcChannelQueuedLatencies(stopwatch.elapsed(TimeUnit.NANOSECONDS));
+    tracer.grpcMessageSent();
   }
 
   static class Factory extends ClientStreamTracer.Factory {

diff --git a/...bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java b/...bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableTracer.java
@@ -83,7 +83,13 @@ public void setLocations(String zone, String cluster) {
     // noop
   }
 
+  /** @deprecated {@link #grpcMessageSent()} is called instead. */
   public void grpcChannelQueuedLatencies(long queuedTimeMs) {
     // noop
   }
+
+  /** Called when the message is sent on a grpc channel. */
+  public void grpcMessageSent() {
+    // noop
+  }
 }
diff --git a/...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java b/...le/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java
@@ -263,8 +263,8 @@ public void batchRequestThrottled(long throttledTimeNanos) {
   }
 
   @Override
-  public void grpcChannelQueuedLatencies(long queuedTimeNanos) {
-    totalClientBlockingTime.addAndGet(queuedTimeNanos);
+  public void grpcMessageSent() {
+    totalClientBlockingTime.addAndGet(attemptTimer.elapsed(TimeUnit.NANOSECONDS));
   }
 
   @Override

diff --git a/...igtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java b/...igtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracer.java
@@ -225,4 +225,11 @@ public void grpcChannelQueuedLatencies(long queuedTimeMs) {
       tracer.grpcChannelQueuedLatencies(queuedTimeMs);
     }
   }
+
+  @Override
+  public void grpcMessageSent() {
+    for (BigtableTracer tracer : bigtableTracers) {
+      tracer.grpcMessageSent();
+    }
+  }
 }
diff --git a/...rc/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java b/...rc/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java
@@ -692,9 +692,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

diff --git a/...ble/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java b/...ble/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java
@@ -258,4 +258,11 @@ public void testRequestBlockedOnChannel() {
     verify(child3, times(1)).grpcChannelQueuedLatencies(5L);
     verify(child4, times(1)).grpcChannelQueuedLatencies(5L);
   }
+
+  @Test
+  public void testGrpcMessageSent() {
+    compositeTracer.grpcMessageSent();
+    verify(child3, times(1)).grpcMessageSent();
+    verify(child4, times(1)).grpcMessageSent();
+  }
 }