diff --git a/google-cloud-bigtable-stats/clirr-ignored-differences.xml b/google-cloud-bigtable-stats/clirr-ignored-differences.xml index ff42f58da4..a920751495 100644 --- a/google-cloud-bigtable-stats/clirr-ignored-differences.xml +++ b/google-cloud-bigtable-stats/clirr-ignored-differences.xml @@ -13,4 +13,10 @@ com/google/cloud/bigtable/stats/StatsRecorderWrapper void record(java.lang.String, java.lang.String, java.lang.String, java.lang.String) + + + 7002 + com/google/cloud/bigtable/stats/StatsRecorderWrapper + void putBatchRequestThrottled(long) + diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java index aafb75c6fb..88eab077c3 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java @@ -115,12 +115,8 @@ public void putGfeMissingHeaders(long connectivityErrors) { attemptMeasureMap.put(BuiltinMeasureConstants.CONNECTIVITY_ERROR_COUNT, connectivityErrors); } - public void putBatchRequestThrottled(long throttledTimeMs) { - operationMeasureMap.put(BuiltinMeasureConstants.THROTTLING_LATENCIES, throttledTimeMs); - } - - public void putGrpcChannelQueuedLatencies(long blockedTime) { - attemptMeasureMap.put(BuiltinMeasureConstants.THROTTLING_LATENCIES, blockedTime); + public void putClientBlockingLatencies(long clientBlockingLatency) { + operationMeasureMap.put(BuiltinMeasureConstants.THROTTLING_LATENCIES, clientBlockingLatency); } private TagContextBuilder newTagContextBuilder(String tableId, String zone, String cluster) { diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java index 80d4eaefa0..b68e4f1a1b 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java @@ -91,8 +91,7 @@ public void testStreamingOperation() throws InterruptedException { recorderWrapper.putGfeLatencies(serverLatency); recorderWrapper.putGfeMissingHeaders(connectivityErrorCount); recorderWrapper.putFirstResponseLatencies(firstResponseLatency); - recorderWrapper.putBatchRequestThrottled(throttlingLatency); - recorderWrapper.putGrpcChannelQueuedLatencies(throttlingLatency); + recorderWrapper.putClientBlockingLatencies(throttlingLatency); recorderWrapper.recordOperation("OK", TABLE_ID, ZONE, CLUSTER); recorderWrapper.recordAttempt("OK", TABLE_ID, ZONE, CLUSTER); @@ -291,8 +290,7 @@ public void testUnaryOperations() throws InterruptedException { recorderWrapper.putGfeLatencies(serverLatency); recorderWrapper.putGfeMissingHeaders(connectivityErrorCount); recorderWrapper.putFirstResponseLatencies(firstResponseLatency); - recorderWrapper.putBatchRequestThrottled(throttlingLatency); - recorderWrapper.putGrpcChannelQueuedLatencies(throttlingLatency); + recorderWrapper.putClientBlockingLatencies(throttlingLatency); recorderWrapper.recordOperation("UNAVAILABLE", TABLE_ID, ZONE, CLUSTER); recorderWrapper.recordAttempt("UNAVAILABLE", TABLE_ID, ZONE, CLUSTER); 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 de048b5ab8..a8b8148d3e 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 @@ -71,6 +71,8 @@ class BuiltinMetricsTracer extends BigtableTracer { private String zone = "global"; private String cluster = "unspecified"; + private AtomicLong totalClientBlockingTime = new AtomicLong(0); + @VisibleForTesting BuiltinMetricsTracer( OperationType operationType, SpanName spanName, StatsRecorderWrapper recorder) { @@ -219,12 +221,12 @@ public void setLocations(String zone, String cluster) { @Override public void batchRequestThrottled(long throttledTimeMs) { - recorder.putBatchRequestThrottled(throttledTimeMs); + totalClientBlockingTime.addAndGet(throttledTimeMs); } @Override public void grpcChannelQueuedLatencies(long queuedTimeMs) { - recorder.putGrpcChannelQueuedLatencies(queuedTimeMs); + totalClientBlockingTime.addAndGet(queuedTimeMs); } @Override @@ -271,6 +273,8 @@ private void recordAttemptCompletion(@Nullable Throwable status) { } } + recorder.putClientBlockingLatencies(totalClientBlockingTime.get()); + // Patch the status until it's fixed in gax. When an attempt failed, // it'll throw a ServerStreamingAttemptException. Unwrap the exception // so it could get processed by extractStatus 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 bd48b463bd..3bc283a7f7 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 @@ -480,8 +480,8 @@ public void testBatchBlockingLatencies() throws InterruptedException { int expectedNumRequests = 6 / batchElementCount; ArgumentCaptor throttledTime = ArgumentCaptor.forClass(Long.class); - verify(statsRecorderWrapper, times(expectedNumRequests)) - .putBatchRequestThrottled(throttledTime.capture()); + verify(statsRecorderWrapper, timeout(1000).times(expectedNumRequests)) + .putClientBlockingLatencies(throttledTime.capture()); // Adding the first 2 elements should not get throttled since the batch is empty assertThat(throttledTime.getAllValues().get(0)).isEqualTo(0); @@ -493,7 +493,7 @@ public void testBatchBlockingLatencies() throws InterruptedException { } @Test - public void testBlockedOnChannelServerStreamLatencies() throws InterruptedException { + public void testQueuedOnChannelServerStreamLatencies() throws InterruptedException { when(mockFactory.newTracer(any(), any(), any())) .thenReturn( new BuiltinMetricsTracer( @@ -505,14 +505,14 @@ public void testBlockedOnChannelServerStreamLatencies() throws InterruptedExcept ArgumentCaptor blockedTime = ArgumentCaptor.forClass(Long.class); - verify(statsRecorderWrapper, times(fakeService.attemptCounter.get())) - .putGrpcChannelQueuedLatencies(blockedTime.capture()); + verify(statsRecorderWrapper, timeout(1000).times(fakeService.attemptCounter.get())) + .putClientBlockingLatencies(blockedTime.capture()); assertThat(blockedTime.getAllValues().get(1)).isAtLeast(CHANNEL_BLOCKING_LATENCY); } @Test - public void testBlockedOnChannelUnaryLatencies() throws InterruptedException { + public void testQueuedOnChannelUnaryLatencies() throws InterruptedException { when(mockFactory.newTracer(any(), any(), any())) .thenReturn( new BuiltinMetricsTracer( @@ -521,8 +521,8 @@ public void testBlockedOnChannelUnaryLatencies() throws InterruptedException { ArgumentCaptor blockedTime = ArgumentCaptor.forClass(Long.class); - verify(statsRecorderWrapper, times(fakeService.attemptCounter.get())) - .putGrpcChannelQueuedLatencies(blockedTime.capture()); + verify(statsRecorderWrapper, timeout(1000).times(fakeService.attemptCounter.get())) + .putClientBlockingLatencies(blockedTime.capture()); assertThat(blockedTime.getAllValues().get(1)).isAtLeast(CHANNEL_BLOCKING_LATENCY); assertThat(blockedTime.getAllValues().get(2)).isAtLeast(CHANNEL_BLOCKING_LATENCY);