From a1b64c7ad0bae21f86b6716dff737992f012d22e Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 10 May 2022 10:11:16 -0400 Subject: [PATCH 01/15] feat: add built in metrics measure and views --- .../stats/BuiltinMetricsRecorder.java | 170 +++++++ .../stats/BuiltinMetricsRecorderTest.java | 433 ++++++++++++++++++ 2 files changed, 603 insertions(+) create mode 100644 google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java create mode 100644 google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java new file mode 100644 index 0000000000..51a0feb5cb --- /dev/null +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java @@ -0,0 +1,170 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.stats; + +import com.google.api.core.InternalApi; +import com.google.api.gax.tracing.ApiTracerFactory.OperationType; +import com.google.api.gax.tracing.SpanName; +import io.opencensus.stats.MeasureMap; +import io.opencensus.stats.StatsRecorder; +import io.opencensus.tags.TagContext; +import io.opencensus.tags.TagContextBuilder; +import io.opencensus.tags.TagKey; +import io.opencensus.tags.TagValue; +import io.opencensus.tags.Tagger; +import io.opencensus.tags.Tags; +import java.util.Map; + +/** Add built-in metrics to the measure map * */ +@InternalApi("For internal use only") +public class BuiltinMetricsRecorder { + + private final OperationType operationType; + + private final Tagger tagger; + private final StatsRecorder statsRecorder; + private final TagContext parentContext; + private final SpanName spanName; + private final Map statsAttributes; + + private MeasureMap attemptLevelNoStreaming; + private MeasureMap attemptLevelWithStreaming; + private MeasureMap operationLevelNoStreaming; + private MeasureMap operationLevelWithStreaming; + + public BuiltinMetricsRecorder( + OperationType operationType, + SpanName spanName, + Map statsAttributes, + StatsWrapper builtinMetricsWrapper) { + this.operationType = operationType; + this.tagger = Tags.getTagger(); + this.statsRecorder = builtinMetricsWrapper.getStatsRecorder(); + this.spanName = spanName; + this.parentContext = tagger.getCurrentTagContext(); + this.statsAttributes = statsAttributes; + + this.attemptLevelNoStreaming = statsRecorder.newMeasureMap(); + this.attemptLevelWithStreaming = statsRecorder.newMeasureMap(); + this.operationLevelNoStreaming = statsRecorder.newMeasureMap(); + this.operationLevelWithStreaming = statsRecorder.newMeasureMap(); + } + + public void recordAttemptLevelWithoutStreaming( + String status, String tableId, String zone, String cluster) { + TagContextBuilder tagCtx = + newTagContextBuilder(tableId, zone, cluster) + .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); + + attemptLevelNoStreaming.record(tagCtx.build()); + } + + public void recordAttemptLevelWithStreaming( + String status, String tableId, String zone, String cluster) { + TagContextBuilder tagCtx = + newTagContextBuilder(tableId, zone, cluster) + .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); + + if (operationType == OperationType.ServerStreaming + && spanName.getMethodName().equals("ReadRows")) { + tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("true")); + } else { + tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("false")); + } + + attemptLevelWithStreaming.record(tagCtx.build()); + } + + public void recordOperationLevelWithoutStreaming( + String status, String tableId, String zone, String cluster) { + TagContextBuilder tagCtx = + newTagContextBuilder(tableId, zone, cluster) + .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); + + operationLevelNoStreaming.record(tagCtx.build()); + } + + public void recordOperationLevelWithStreaming( + String status, String tableId, String zone, String cluster) { + TagContextBuilder tagCtx = + newTagContextBuilder(tableId, zone, cluster) + .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); + + if (operationType == OperationType.ServerStreaming + && spanName.getMethodName().equals("ReadRows")) { + tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("true")); + } else { + tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("false")); + } + + operationLevelWithStreaming.record(tagCtx.build()); + } + + public void recordOperationLatencies(long operationLatency) { + operationLevelWithStreaming.put(BuiltinMeasureConstants.OPERATION_LATENCIES, operationLatency); + } + + public void recordAttemptLatency(long attemptLatency) { + attemptLevelWithStreaming.put(BuiltinMeasureConstants.ATTEMPT_LATENCIES, attemptLatency); + } + + public void recordRetryCount(int attemptCount) { + operationLevelNoStreaming.put(BuiltinMeasureConstants.RETRY_COUNT, attemptCount); + } + + public void recordApplicationLatency(long applicationLatency) { + operationLevelWithStreaming.put( + BuiltinMeasureConstants.APPLICATION_LATENCIES, applicationLatency); + } + + public void recordFirstResponseLatency(long firstResponseLatency) { + operationLevelNoStreaming.put( + BuiltinMeasureConstants.FIRST_RESPONSE_LATENCIES, firstResponseLatency); + } + + public void recordGfeLatencies(long serverLatency) { + attemptLevelWithStreaming.put(BuiltinMeasureConstants.SERVER_LATENCIES, serverLatency); + } + + public void recordGfeMissingHeaders(long connectivityErrors) { + attemptLevelNoStreaming.put( + BuiltinMeasureConstants.CONNECTIVITY_ERROR_COUNT, connectivityErrors); + } + + public void recordBatchRequestThrottled( + long throttledTimeMs, String tableId, String zone, String cluster) { + MeasureMap measures = + statsRecorder + .newMeasureMap() + .put(BuiltinMeasureConstants.THROTTLING_LATENCIES, throttledTimeMs); + measures.record(newTagContextBuilder(tableId, zone, cluster).build()); + } + + private TagContextBuilder newTagContextBuilder(String tableId, String zone, String cluster) { + TagContextBuilder tagContextBuilder = + tagger + .toBuilder(parentContext) + .putLocal(BuiltinMeasureConstants.CLIENT_NAME, TagValue.create("bigtable-java")) + .putLocal(BuiltinMeasureConstants.METHOD, TagValue.create(spanName.toString())) + .putLocal(BuiltinMeasureConstants.TABLE, TagValue.create(tableId)) + .putLocal(BuiltinMeasureConstants.ZONE, TagValue.create(zone)) + .putLocal(BuiltinMeasureConstants.CLUSTER, TagValue.create(cluster)); + for (Map.Entry entry : statsAttributes.entrySet()) { + tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); + } + return tagContextBuilder; + } +} diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java new file mode 100644 index 0000000000..8f0c2688bf --- /dev/null +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java @@ -0,0 +1,433 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.stats; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.SpanName; +import com.google.common.collect.ImmutableMap; +import io.grpc.Status; +import org.junit.Before; +import org.junit.Test; + +public class BuiltinMetricsRecorderTest { + + private final String PROJECT_ID = "fake-project"; + private final String INSTANCE_ID = "fake-instance"; + private final String APP_PROFILE_ID = "fake-app-profile"; + + private final String TABLE_ID = "fake-table-id"; + private final String ZONE = "fake-zone"; + private final String CLUSTER = "fake-cluster"; + + private StatsWrapper wrapper; + + @Before + public void setup() { + this.wrapper = new StatsWrapper(true); + BuiltinViews views = new BuiltinViews(wrapper); + views.registerBigtableBuiltinViews(); + } + + @Test + public void testStreamingOperation() throws InterruptedException { + BuiltinMetricsRecorder tracer = + new BuiltinMetricsRecorder( + ApiTracerFactory.OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + ImmutableMap.of( + BuiltinMeasureConstants.PROJECT_ID.getName(), PROJECT_ID, + BuiltinMeasureConstants.INSTANCE_ID.getName(), INSTANCE_ID, + BuiltinMeasureConstants.APP_PROFILE.getName(), APP_PROFILE_ID), + wrapper); + + long operationLatency = 1234; + int attemptCount = 2; + long attemptLatency = 56; + long serverLatency = 78; + long applicationLatency = 901; + long connectivityErrorCount = 15; + long throttlingLatency = 50; + long firstResponseLatency = 90; + + tracer.recordOperationLatencies(operationLatency); + tracer.recordRetryCount(attemptCount); + tracer.recordAttemptLatency(attemptLatency); + tracer.recordApplicationLatency(applicationLatency); + tracer.recordGfeLatencies(serverLatency); + tracer.recordGfeMissingHeaders(connectivityErrorCount); + tracer.recordFirstResponseLatency(firstResponseLatency); + tracer.recordBatchRequestThrottled(throttlingLatency, TABLE_ID, ZONE, CLUSTER); + + tracer.recordAttemptLevelWithoutStreaming( + Status.UNAVAILABLE.toString(), TABLE_ID, ZONE, CLUSTER); + tracer.recordAttemptLevelWithStreaming(Status.ABORTED.toString(), TABLE_ID, ZONE, CLUSTER); + tracer.recordOperationLevelWithoutStreaming("OK", TABLE_ID, ZONE, CLUSTER); + tracer.recordOperationLevelWithStreaming("OK", TABLE_ID, ZONE, CLUSTER); + + Thread.sleep(100); + + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.OPERATION_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, "Bigtable.ReadRows", + BuiltinMeasureConstants.STATUS, "OK", + BuiltinMeasureConstants.TABLE, TABLE_ID, + BuiltinMeasureConstants.ZONE, ZONE, + BuiltinMeasureConstants.CLUSTER, CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java", + BuiltinMeasureConstants.STREAMING, "true"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(operationLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.ATTEMPT_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.ReadRows", + BuiltinMeasureConstants.STATUS, + Status.ABORTED.toString(), + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.STREAMING, + "true"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(attemptLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.RETRY_COUNT_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.ReadRows", + BuiltinMeasureConstants.STATUS, + "OK", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(attemptCount); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.SERVER_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.ReadRows", + BuiltinMeasureConstants.STATUS, + Status.ABORTED.toString(), + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.STREAMING, + "true", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(serverLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.APPLICATION_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.ReadRows", + BuiltinMeasureConstants.STATUS, + "OK", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.STREAMING, + "true"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(applicationLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.CONNECTIVITY_ERROR_COUNT_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.ReadRows", + BuiltinMeasureConstants.STATUS, + Status.UNAVAILABLE.toString(), + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(connectivityErrorCount); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.THROTTLING_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, "Bigtable.ReadRows", + BuiltinMeasureConstants.TABLE, TABLE_ID, + BuiltinMeasureConstants.ZONE, ZONE, + BuiltinMeasureConstants.CLUSTER, CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(throttlingLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.FIRST_RESPONSE_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.ReadRows", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.STATUS, + "OK", + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(firstResponseLatency); + } + + @Test + public void testUnaryOperations() throws InterruptedException { + BuiltinMetricsRecorder tracer = + new BuiltinMetricsRecorder( + ApiTracerFactory.OperationType.ServerStreaming, + SpanName.of("Bigtable", "MutateRow"), + ImmutableMap.of( + BuiltinMeasureConstants.PROJECT_ID.getName(), PROJECT_ID, + BuiltinMeasureConstants.INSTANCE_ID.getName(), INSTANCE_ID, + BuiltinMeasureConstants.APP_PROFILE.getName(), APP_PROFILE_ID), + wrapper); + + long operationLatency = 1234; + int attemptCount = 2; + long attemptLatency = 56; + long serverLatency = 78; + long applicationLatency = 901; + long connectivityErrorCount = 15; + long throttlingLatency = 50; + long firstResponseLatency = 90; + + tracer.recordOperationLatencies(operationLatency); + tracer.recordRetryCount(attemptCount); + tracer.recordAttemptLatency(attemptLatency); + tracer.recordApplicationLatency(applicationLatency); + tracer.recordGfeLatencies(serverLatency); + tracer.recordGfeMissingHeaders(connectivityErrorCount); + tracer.recordFirstResponseLatency(firstResponseLatency); + tracer.recordBatchRequestThrottled(throttlingLatency, TABLE_ID, ZONE, CLUSTER); + + tracer.recordOperationLevelWithStreaming("OK", TABLE_ID, ZONE, CLUSTER); + tracer.recordOperationLevelWithoutStreaming("OK", TABLE_ID, ZONE, CLUSTER); + tracer.recordAttemptLevelWithoutStreaming( + Status.UNAVAILABLE.toString(), TABLE_ID, ZONE, CLUSTER); + tracer.recordAttemptLevelWithStreaming(Status.ABORTED.toString(), TABLE_ID, ZONE, CLUSTER); + + Thread.sleep(100); + + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.OPERATION_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, "Bigtable.MutateRow", + BuiltinMeasureConstants.STATUS, "OK", + BuiltinMeasureConstants.TABLE, TABLE_ID, + BuiltinMeasureConstants.ZONE, ZONE, + BuiltinMeasureConstants.CLUSTER, CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java", + BuiltinMeasureConstants.STREAMING, "false"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(operationLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.ATTEMPT_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.MutateRow", + BuiltinMeasureConstants.STATUS, + Status.ABORTED.toString(), + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.STREAMING, + "false"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(attemptLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.RETRY_COUNT_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.MutateRow", + BuiltinMeasureConstants.STATUS, + "OK", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(attemptCount); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.SERVER_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.MutateRow", + BuiltinMeasureConstants.STATUS, + Status.ABORTED.toString(), + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.STREAMING, + "false", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(serverLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.APPLICATION_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.MutateRow", + BuiltinMeasureConstants.STATUS, + "OK", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.STREAMING, + "false"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(applicationLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.CONNECTIVITY_ERROR_COUNT_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.MutateRow", + BuiltinMeasureConstants.STATUS, + Status.UNAVAILABLE.toString(), + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(connectivityErrorCount); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.THROTTLING_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, "Bigtable.MutateRow", + BuiltinMeasureConstants.TABLE, TABLE_ID, + BuiltinMeasureConstants.ZONE, ZONE, + BuiltinMeasureConstants.CLUSTER, CLUSTER, + BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(throttlingLatency); + assertThat( + wrapper.getAggregationValueAsLong( + BuiltinViewConstants.FIRST_RESPONSE_LATENCIES_VIEW, + ImmutableMap.of( + BuiltinMeasureConstants.METHOD, + "Bigtable.MutateRow", + BuiltinMeasureConstants.TABLE, + TABLE_ID, + BuiltinMeasureConstants.ZONE, + ZONE, + BuiltinMeasureConstants.CLUSTER, + CLUSTER, + BuiltinMeasureConstants.STATUS, + "OK", + BuiltinMeasureConstants.CLIENT_NAME, + "bigtable-java"), + PROJECT_ID, + INSTANCE_ID, + APP_PROFILE_ID)) + .isEqualTo(firstResponseLatency); + } +} From 2f8a90342b4cc9cb0fb24cf997b4c6422c0ce702 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 11 May 2022 14:20:34 -0400 Subject: [PATCH 02/15] remove status from application latency --- .../stats/BuiltinMetricsRecorder.java | 19 ++++++++++++++++--- .../stats/BuiltinMetricsRecorderTest.java | 4 ++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java index 51a0feb5cb..a66da11d0b 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java @@ -125,9 +125,22 @@ public void recordRetryCount(int attemptCount) { operationLevelNoStreaming.put(BuiltinMeasureConstants.RETRY_COUNT, attemptCount); } - public void recordApplicationLatency(long applicationLatency) { - operationLevelWithStreaming.put( - BuiltinMeasureConstants.APPLICATION_LATENCIES, applicationLatency); + public void recordApplicationLatency( + long applicationLatency, String tableId, String zone, String cluster) { + MeasureMap measures = + statsRecorder + .newMeasureMap() + .put(BuiltinMeasureConstants.APPLICATION_LATENCIES, applicationLatency); + + TagContextBuilder tagCtx = newTagContextBuilder(tableId, zone, cluster); + if (operationType == OperationType.ServerStreaming + && spanName.getMethodName().equals("ReadRows")) { + tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("true")); + } else { + tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("false")); + } + + measures.record(tagCtx.build()); } public void recordFirstResponseLatency(long firstResponseLatency) { diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java index 8f0c2688bf..ce0cb0e011 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java @@ -67,7 +67,7 @@ public void testStreamingOperation() throws InterruptedException { tracer.recordOperationLatencies(operationLatency); tracer.recordRetryCount(attemptCount); tracer.recordAttemptLatency(attemptLatency); - tracer.recordApplicationLatency(applicationLatency); + tracer.recordApplicationLatency(applicationLatency, TABLE_ID, ZONE, CLUSTER); tracer.recordGfeLatencies(serverLatency); tracer.recordGfeMissingHeaders(connectivityErrorCount); tracer.recordFirstResponseLatency(firstResponseLatency); @@ -261,7 +261,7 @@ public void testUnaryOperations() throws InterruptedException { tracer.recordOperationLatencies(operationLatency); tracer.recordRetryCount(attemptCount); tracer.recordAttemptLatency(attemptLatency); - tracer.recordApplicationLatency(applicationLatency); + tracer.recordApplicationLatency(applicationLatency, TABLE_ID, ZONE, CLUSTER); tracer.recordGfeLatencies(serverLatency); tracer.recordGfeMissingHeaders(connectivityErrorCount); tracer.recordFirstResponseLatency(firstResponseLatency); From 20511cfed0b9457af37165acd83463e870022e51 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 7 Jun 2022 15:56:56 -0400 Subject: [PATCH 03/15] Rename methods and add comments --- .../stats/BuiltinMetricsRecorder.java | 183 -------- .../stats/BuiltinMetricsRecorderTest.java | 433 ------------------ 2 files changed, 616 deletions(-) delete mode 100644 google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java delete mode 100644 google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java deleted file mode 100644 index a66da11d0b..0000000000 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorder.java +++ /dev/null @@ -1,183 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.stats; - -import com.google.api.core.InternalApi; -import com.google.api.gax.tracing.ApiTracerFactory.OperationType; -import com.google.api.gax.tracing.SpanName; -import io.opencensus.stats.MeasureMap; -import io.opencensus.stats.StatsRecorder; -import io.opencensus.tags.TagContext; -import io.opencensus.tags.TagContextBuilder; -import io.opencensus.tags.TagKey; -import io.opencensus.tags.TagValue; -import io.opencensus.tags.Tagger; -import io.opencensus.tags.Tags; -import java.util.Map; - -/** Add built-in metrics to the measure map * */ -@InternalApi("For internal use only") -public class BuiltinMetricsRecorder { - - private final OperationType operationType; - - private final Tagger tagger; - private final StatsRecorder statsRecorder; - private final TagContext parentContext; - private final SpanName spanName; - private final Map statsAttributes; - - private MeasureMap attemptLevelNoStreaming; - private MeasureMap attemptLevelWithStreaming; - private MeasureMap operationLevelNoStreaming; - private MeasureMap operationLevelWithStreaming; - - public BuiltinMetricsRecorder( - OperationType operationType, - SpanName spanName, - Map statsAttributes, - StatsWrapper builtinMetricsWrapper) { - this.operationType = operationType; - this.tagger = Tags.getTagger(); - this.statsRecorder = builtinMetricsWrapper.getStatsRecorder(); - this.spanName = spanName; - this.parentContext = tagger.getCurrentTagContext(); - this.statsAttributes = statsAttributes; - - this.attemptLevelNoStreaming = statsRecorder.newMeasureMap(); - this.attemptLevelWithStreaming = statsRecorder.newMeasureMap(); - this.operationLevelNoStreaming = statsRecorder.newMeasureMap(); - this.operationLevelWithStreaming = statsRecorder.newMeasureMap(); - } - - public void recordAttemptLevelWithoutStreaming( - String status, String tableId, String zone, String cluster) { - TagContextBuilder tagCtx = - newTagContextBuilder(tableId, zone, cluster) - .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); - - attemptLevelNoStreaming.record(tagCtx.build()); - } - - public void recordAttemptLevelWithStreaming( - String status, String tableId, String zone, String cluster) { - TagContextBuilder tagCtx = - newTagContextBuilder(tableId, zone, cluster) - .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); - - if (operationType == OperationType.ServerStreaming - && spanName.getMethodName().equals("ReadRows")) { - tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("true")); - } else { - tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("false")); - } - - attemptLevelWithStreaming.record(tagCtx.build()); - } - - public void recordOperationLevelWithoutStreaming( - String status, String tableId, String zone, String cluster) { - TagContextBuilder tagCtx = - newTagContextBuilder(tableId, zone, cluster) - .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); - - operationLevelNoStreaming.record(tagCtx.build()); - } - - public void recordOperationLevelWithStreaming( - String status, String tableId, String zone, String cluster) { - TagContextBuilder tagCtx = - newTagContextBuilder(tableId, zone, cluster) - .putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status)); - - if (operationType == OperationType.ServerStreaming - && spanName.getMethodName().equals("ReadRows")) { - tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("true")); - } else { - tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("false")); - } - - operationLevelWithStreaming.record(tagCtx.build()); - } - - public void recordOperationLatencies(long operationLatency) { - operationLevelWithStreaming.put(BuiltinMeasureConstants.OPERATION_LATENCIES, operationLatency); - } - - public void recordAttemptLatency(long attemptLatency) { - attemptLevelWithStreaming.put(BuiltinMeasureConstants.ATTEMPT_LATENCIES, attemptLatency); - } - - public void recordRetryCount(int attemptCount) { - operationLevelNoStreaming.put(BuiltinMeasureConstants.RETRY_COUNT, attemptCount); - } - - public void recordApplicationLatency( - long applicationLatency, String tableId, String zone, String cluster) { - MeasureMap measures = - statsRecorder - .newMeasureMap() - .put(BuiltinMeasureConstants.APPLICATION_LATENCIES, applicationLatency); - - TagContextBuilder tagCtx = newTagContextBuilder(tableId, zone, cluster); - if (operationType == OperationType.ServerStreaming - && spanName.getMethodName().equals("ReadRows")) { - tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("true")); - } else { - tagCtx.putLocal(BuiltinMeasureConstants.STREAMING, TagValue.create("false")); - } - - measures.record(tagCtx.build()); - } - - public void recordFirstResponseLatency(long firstResponseLatency) { - operationLevelNoStreaming.put( - BuiltinMeasureConstants.FIRST_RESPONSE_LATENCIES, firstResponseLatency); - } - - public void recordGfeLatencies(long serverLatency) { - attemptLevelWithStreaming.put(BuiltinMeasureConstants.SERVER_LATENCIES, serverLatency); - } - - public void recordGfeMissingHeaders(long connectivityErrors) { - attemptLevelNoStreaming.put( - BuiltinMeasureConstants.CONNECTIVITY_ERROR_COUNT, connectivityErrors); - } - - public void recordBatchRequestThrottled( - long throttledTimeMs, String tableId, String zone, String cluster) { - MeasureMap measures = - statsRecorder - .newMeasureMap() - .put(BuiltinMeasureConstants.THROTTLING_LATENCIES, throttledTimeMs); - measures.record(newTagContextBuilder(tableId, zone, cluster).build()); - } - - private TagContextBuilder newTagContextBuilder(String tableId, String zone, String cluster) { - TagContextBuilder tagContextBuilder = - tagger - .toBuilder(parentContext) - .putLocal(BuiltinMeasureConstants.CLIENT_NAME, TagValue.create("bigtable-java")) - .putLocal(BuiltinMeasureConstants.METHOD, TagValue.create(spanName.toString())) - .putLocal(BuiltinMeasureConstants.TABLE, TagValue.create(tableId)) - .putLocal(BuiltinMeasureConstants.ZONE, TagValue.create(zone)) - .putLocal(BuiltinMeasureConstants.CLUSTER, TagValue.create(cluster)); - for (Map.Entry entry : statsAttributes.entrySet()) { - tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())); - } - return tagContextBuilder; - } -} diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java deleted file mode 100644 index ce0cb0e011..0000000000 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinMetricsRecorderTest.java +++ /dev/null @@ -1,433 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.bigtable.stats; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.api.gax.tracing.ApiTracerFactory; -import com.google.api.gax.tracing.SpanName; -import com.google.common.collect.ImmutableMap; -import io.grpc.Status; -import org.junit.Before; -import org.junit.Test; - -public class BuiltinMetricsRecorderTest { - - private final String PROJECT_ID = "fake-project"; - private final String INSTANCE_ID = "fake-instance"; - private final String APP_PROFILE_ID = "fake-app-profile"; - - private final String TABLE_ID = "fake-table-id"; - private final String ZONE = "fake-zone"; - private final String CLUSTER = "fake-cluster"; - - private StatsWrapper wrapper; - - @Before - public void setup() { - this.wrapper = new StatsWrapper(true); - BuiltinViews views = new BuiltinViews(wrapper); - views.registerBigtableBuiltinViews(); - } - - @Test - public void testStreamingOperation() throws InterruptedException { - BuiltinMetricsRecorder tracer = - new BuiltinMetricsRecorder( - ApiTracerFactory.OperationType.ServerStreaming, - SpanName.of("Bigtable", "ReadRows"), - ImmutableMap.of( - BuiltinMeasureConstants.PROJECT_ID.getName(), PROJECT_ID, - BuiltinMeasureConstants.INSTANCE_ID.getName(), INSTANCE_ID, - BuiltinMeasureConstants.APP_PROFILE.getName(), APP_PROFILE_ID), - wrapper); - - long operationLatency = 1234; - int attemptCount = 2; - long attemptLatency = 56; - long serverLatency = 78; - long applicationLatency = 901; - long connectivityErrorCount = 15; - long throttlingLatency = 50; - long firstResponseLatency = 90; - - tracer.recordOperationLatencies(operationLatency); - tracer.recordRetryCount(attemptCount); - tracer.recordAttemptLatency(attemptLatency); - tracer.recordApplicationLatency(applicationLatency, TABLE_ID, ZONE, CLUSTER); - tracer.recordGfeLatencies(serverLatency); - tracer.recordGfeMissingHeaders(connectivityErrorCount); - tracer.recordFirstResponseLatency(firstResponseLatency); - tracer.recordBatchRequestThrottled(throttlingLatency, TABLE_ID, ZONE, CLUSTER); - - tracer.recordAttemptLevelWithoutStreaming( - Status.UNAVAILABLE.toString(), TABLE_ID, ZONE, CLUSTER); - tracer.recordAttemptLevelWithStreaming(Status.ABORTED.toString(), TABLE_ID, ZONE, CLUSTER); - tracer.recordOperationLevelWithoutStreaming("OK", TABLE_ID, ZONE, CLUSTER); - tracer.recordOperationLevelWithStreaming("OK", TABLE_ID, ZONE, CLUSTER); - - Thread.sleep(100); - - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.OPERATION_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, "Bigtable.ReadRows", - BuiltinMeasureConstants.STATUS, "OK", - BuiltinMeasureConstants.TABLE, TABLE_ID, - BuiltinMeasureConstants.ZONE, ZONE, - BuiltinMeasureConstants.CLUSTER, CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java", - BuiltinMeasureConstants.STREAMING, "true"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(operationLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.ATTEMPT_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.ReadRows", - BuiltinMeasureConstants.STATUS, - Status.ABORTED.toString(), - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.STREAMING, - "true"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(attemptLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.RETRY_COUNT_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.ReadRows", - BuiltinMeasureConstants.STATUS, - "OK", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(attemptCount); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.SERVER_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.ReadRows", - BuiltinMeasureConstants.STATUS, - Status.ABORTED.toString(), - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.STREAMING, - "true", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(serverLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.APPLICATION_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.ReadRows", - BuiltinMeasureConstants.STATUS, - "OK", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.STREAMING, - "true"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(applicationLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.CONNECTIVITY_ERROR_COUNT_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.ReadRows", - BuiltinMeasureConstants.STATUS, - Status.UNAVAILABLE.toString(), - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(connectivityErrorCount); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.THROTTLING_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, "Bigtable.ReadRows", - BuiltinMeasureConstants.TABLE, TABLE_ID, - BuiltinMeasureConstants.ZONE, ZONE, - BuiltinMeasureConstants.CLUSTER, CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(throttlingLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.FIRST_RESPONSE_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.ReadRows", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.STATUS, - "OK", - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(firstResponseLatency); - } - - @Test - public void testUnaryOperations() throws InterruptedException { - BuiltinMetricsRecorder tracer = - new BuiltinMetricsRecorder( - ApiTracerFactory.OperationType.ServerStreaming, - SpanName.of("Bigtable", "MutateRow"), - ImmutableMap.of( - BuiltinMeasureConstants.PROJECT_ID.getName(), PROJECT_ID, - BuiltinMeasureConstants.INSTANCE_ID.getName(), INSTANCE_ID, - BuiltinMeasureConstants.APP_PROFILE.getName(), APP_PROFILE_ID), - wrapper); - - long operationLatency = 1234; - int attemptCount = 2; - long attemptLatency = 56; - long serverLatency = 78; - long applicationLatency = 901; - long connectivityErrorCount = 15; - long throttlingLatency = 50; - long firstResponseLatency = 90; - - tracer.recordOperationLatencies(operationLatency); - tracer.recordRetryCount(attemptCount); - tracer.recordAttemptLatency(attemptLatency); - tracer.recordApplicationLatency(applicationLatency, TABLE_ID, ZONE, CLUSTER); - tracer.recordGfeLatencies(serverLatency); - tracer.recordGfeMissingHeaders(connectivityErrorCount); - tracer.recordFirstResponseLatency(firstResponseLatency); - tracer.recordBatchRequestThrottled(throttlingLatency, TABLE_ID, ZONE, CLUSTER); - - tracer.recordOperationLevelWithStreaming("OK", TABLE_ID, ZONE, CLUSTER); - tracer.recordOperationLevelWithoutStreaming("OK", TABLE_ID, ZONE, CLUSTER); - tracer.recordAttemptLevelWithoutStreaming( - Status.UNAVAILABLE.toString(), TABLE_ID, ZONE, CLUSTER); - tracer.recordAttemptLevelWithStreaming(Status.ABORTED.toString(), TABLE_ID, ZONE, CLUSTER); - - Thread.sleep(100); - - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.OPERATION_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, "Bigtable.MutateRow", - BuiltinMeasureConstants.STATUS, "OK", - BuiltinMeasureConstants.TABLE, TABLE_ID, - BuiltinMeasureConstants.ZONE, ZONE, - BuiltinMeasureConstants.CLUSTER, CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java", - BuiltinMeasureConstants.STREAMING, "false"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(operationLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.ATTEMPT_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.MutateRow", - BuiltinMeasureConstants.STATUS, - Status.ABORTED.toString(), - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.STREAMING, - "false"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(attemptLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.RETRY_COUNT_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.MutateRow", - BuiltinMeasureConstants.STATUS, - "OK", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(attemptCount); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.SERVER_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.MutateRow", - BuiltinMeasureConstants.STATUS, - Status.ABORTED.toString(), - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.STREAMING, - "false", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(serverLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.APPLICATION_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.MutateRow", - BuiltinMeasureConstants.STATUS, - "OK", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.STREAMING, - "false"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(applicationLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.CONNECTIVITY_ERROR_COUNT_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.MutateRow", - BuiltinMeasureConstants.STATUS, - Status.UNAVAILABLE.toString(), - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(connectivityErrorCount); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.THROTTLING_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, "Bigtable.MutateRow", - BuiltinMeasureConstants.TABLE, TABLE_ID, - BuiltinMeasureConstants.ZONE, ZONE, - BuiltinMeasureConstants.CLUSTER, CLUSTER, - BuiltinMeasureConstants.CLIENT_NAME, "bigtable-java"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(throttlingLatency); - assertThat( - wrapper.getAggregationValueAsLong( - BuiltinViewConstants.FIRST_RESPONSE_LATENCIES_VIEW, - ImmutableMap.of( - BuiltinMeasureConstants.METHOD, - "Bigtable.MutateRow", - BuiltinMeasureConstants.TABLE, - TABLE_ID, - BuiltinMeasureConstants.ZONE, - ZONE, - BuiltinMeasureConstants.CLUSTER, - CLUSTER, - BuiltinMeasureConstants.STATUS, - "OK", - BuiltinMeasureConstants.CLIENT_NAME, - "bigtable-java"), - PROJECT_ID, - INSTANCE_ID, - APP_PROFILE_ID)) - .isEqualTo(firstResponseLatency); - } -} From 1ef694fc4b9dac2799fa0e8696e8f383cc6316eb Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 8 Jun 2022 15:09:45 -0400 Subject: [PATCH 04/15] update based on comments --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index 74f02cdeb4..d786fd2741 100644 --- a/pom.xml +++ b/pom.xml @@ -336,6 +336,7 @@ google-cloud-bigtable + google-cloud-bigtable-stats grpc-google-cloud-bigtable-admin-v2 grpc-google-cloud-bigtable-v2 proto-google-cloud-bigtable-admin-v2 From 61bdae11d0395952cb2ead446cb6ba430409ff60 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 10 May 2022 11:49:36 -0400 Subject: [PATCH 05/15] feat: update tracers to use built in metrics --- google-cloud-bigtable/pom.xml | 4 + .../data/v2/stub/EnhancedBigtableStub.java | 18 +- .../data/v2/stub/metrics/BigtableTracer.java | 12 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 244 +++++++++++++ .../metrics/BuiltinMetricsTracerFactory.java | 66 ++++ .../data/v2/stub/metrics/CompositeTracer.java | 21 +- .../data/v2/stub/metrics/MetricsTracer.java | 21 +- .../bigtable/data/v2/stub/metrics/Util.java | 44 ++- .../metrics/BuiltinMetricsTracerTest.java | 339 ++++++++++++++++++ .../v2/stub/metrics/CompositeTracerTest.java | 12 +- .../data/v2/stub/metrics/UtilTest.java | 10 +- 11 files changed, 764 insertions(+), 27 deletions(-) create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java create mode 100644 google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml index 6d1c003802..b5128af7ae 100644 --- a/google-cloud-bigtable/pom.xml +++ b/google-cloud-bigtable/pom.xml @@ -61,6 +61,10 @@ + + com.google.cloud + google-cloud-bigtable-stats + com.google.api 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 d8daaa80e6..4087dc59b4 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 @@ -70,6 +70,7 @@ import com.google.cloud.bigtable.data.v2.models.RowAdapter; import com.google.cloud.bigtable.data.v2.models.RowMutation; import com.google.cloud.bigtable.data.v2.models.RowMutationEntry; +import com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.CompositeTracerFactory; import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracerStreamingCallable; import com.google.cloud.bigtable.data.v2.stub.metrics.HeaderTracerUnaryCallable; @@ -89,6 +90,7 @@ import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsUserCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.RowMergingCallable; import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; +import com.google.cloud.bigtable.stats.StatsWrapper; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -152,6 +154,7 @@ public static EnhancedBigtableStubSettings finalizeSettings( EnhancedBigtableStubSettings settings, Tagger tagger, StatsRecorder stats) throws IOException { EnhancedBigtableStubSettings.Builder builder = settings.toBuilder(); + StatsWrapper statsWrapper = StatsWrapper.get(); // TODO: this implementation is on the cusp of unwieldy, if we end up adding more features // consider splitting it up by feature. @@ -194,6 +197,12 @@ public static EnhancedBigtableStubSettings finalizeSettings( RpcMeasureConstants.BIGTABLE_APP_PROFILE_ID, TagValue.create(settings.getAppProfileId())) .build(); + ImmutableMap builtinAttributes = + ImmutableMap.builder() + .put("project_id", settings.getProjectId()) + .put("instance_id", settings.getInstanceId()) + .put("app_profile", settings.getAppProfileId()) + .build(); // Inject Opencensus instrumentation builder.setTracerFactory( new CompositeTracerFactory( @@ -218,6 +227,7 @@ public static EnhancedBigtableStubSettings finalizeSettings( .build()), // Add OpenCensus Metrics MetricsTracerFactory.create(tagger, stats, attributes), + BuiltinMetricsTracerFactory.create(statsWrapper, builtinAttributes), // Add user configured tracer settings.getTracerFactory()))); return builder.build(); @@ -466,7 +476,7 @@ private UnaryCallable> createBulkReadRowsCallable( new TracedBatcherUnaryCallable<>(readRowsUserCallable.all()); UnaryCallable> withHeaderTracer = - new HeaderTracerUnaryCallable(tracedBatcher); + new HeaderTracerUnaryCallable<>(tracedBatcher); UnaryCallable> traced = new TracedUnaryCallable<>(withHeaderTracer, clientContext.getTracerFactory(), span); @@ -594,11 +604,11 @@ private UnaryCallable createBulkMutateRowsCallable() { SpanName spanName = getSpanName("MutateRows"); - UnaryCallable tracedBatcher = new TracedBatcherUnaryCallable<>(userFacing); + UnaryCallable tracedBatcherUnaryCallable = + new TracedBatcherUnaryCallable<>(userFacing); UnaryCallable withHeaderTracer = - new HeaderTracerUnaryCallable<>(tracedBatcher); - + new HeaderTracerUnaryCallable<>(tracedBatcherUnaryCallable); UnaryCallable traced = new TracedUnaryCallable<>(withHeaderTracer, clientContext.getTracerFactory(), spanName); 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 3d7707cc4c..421c5b4724 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 @@ -25,7 +25,7 @@ * A Bigtable specific {@link ApiTracer} that includes additional contexts. This class is a base * implementation that does nothing. */ -@BetaApi("This surface is stable yet it might be removed in the future.") +@BetaApi("This surface is not stable yet it might be removed in the future.") public class BigtableTracer extends BaseApiTracer { private volatile int attempt = 0; @@ -35,6 +35,11 @@ public void attemptStarted(int attemptNumber) { this.attempt = attemptNumber; } + /** annotate when onRequest is called */ + public void onRequest() { + // noop + } + /** * Get the attempt number of the current call. Attempt number for the current call is passed in * and should be recorded in {@link #attemptStarted(int)}. With the getter we can access it from @@ -57,4 +62,9 @@ public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwa public void batchRequestThrottled(long throttledTimeMs) { // noop } + + /** Set the Bigtable zone and cluster so metrics can be tagged with location information. */ + public void setLocations(String zone, String cluster) { + // 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 new file mode 100644 index 0000000000..d369cbfcb6 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java @@ -0,0 +1,244 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.SpanName; +import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import com.google.cloud.bigtable.stats.StatsWrapper; +import com.google.common.base.Stopwatch; +import java.util.Map; +import java.util.concurrent.CancellationException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.Nullable; +import org.threeten.bp.Duration; + +/** + * A {@link BigtableTracer} that records built-in metrics and publish under the + * bigtable.googleapis.com/client namespace + */ +class BuiltinMetricsTracer extends BigtableTracer { + + private final StatsRecorderWrapper recorder; + + private final ApiTracerFactory.OperationType operationType; + private final SpanName spanName; + + // Operation level metrics + private final AtomicBoolean opFinished = new AtomicBoolean(); + private final Stopwatch operationTimer = Stopwatch.createStarted(); + private final Stopwatch firstResponsePerOpTimer = Stopwatch.createStarted(); + + // Attempt level metrics + private int attemptCount = 0; + private Stopwatch attemptTimer; + private volatile int attempt = 0; + + // Total application latency + private final Stopwatch applicationLatencyTimer = Stopwatch.createUnstarted(); + private final AtomicLong totalApplicationLatency = new AtomicLong(0); + + // Monitored resource labels + private String tableId = "undefined"; + private String zone = "undefined"; + private String cluster = "undefined"; + + // gfe stats + private AtomicLong gfeMissingHeaders = new AtomicLong(0); + + BuiltinMetricsTracer( + ApiTracerFactory.OperationType operationType, + SpanName spanName, + Map attributes, + StatsWrapper statsWrapper, + StatsRecorderWrapper recorder) { + this.operationType = operationType; + this.spanName = spanName; + if (recorder != null) { + this.recorder = recorder; + } else { + this.recorder = new StatsRecorderWrapper(operationType, spanName, attributes, statsWrapper); + } + } + + @Override + public Scope inScope() { + return new Scope() { + @Override + public void close() {} + }; + } + + @Override + public void operationSucceeded() { + recordOperationCompletion(null); + } + + @Override + public void operationCancelled() { + recordOperationCompletion(new CancellationException()); + } + + @Override + public void operationFailed(Throwable error) { + recordOperationCompletion(error); + } + + @Override + public void attemptStarted(int attemptNumber) { + attemptStarted(null, attemptNumber); + } + + @Override + public void attemptStarted(Object request, int attemptNumber) { + this.attempt = attemptNumber; + attemptCount++; + attemptTimer = Stopwatch.createStarted(); + if (request != null) { + this.tableId = Util.extractTableId(request); + } + if (applicationLatencyTimer.isRunning()) { + totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); + applicationLatencyTimer.reset(); + } + } + + @Override + public void attemptSucceeded() { + recordAttemptCompletion(null); + } + + @Override + public void attemptCancelled() { + recordAttemptCompletion(new CancellationException()); + } + + @Override + public void attemptFailed(Throwable error, Duration delay) { + if (!applicationLatencyTimer.isRunning()) { + applicationLatencyTimer.start(); + } + recordAttemptCompletion(error); + } + + @Override + public void attemptFailedRetriesExhausted(Throwable error) { + super.attemptFailedRetriesExhausted(error); + } + + @Override + public void attemptPermanentFailure(Throwable error) { + super.attemptPermanentFailure(error); + } + + @Override + public void lroStartFailed(Throwable error) { + super.lroStartFailed(error); + } + + @Override + public void lroStartSucceeded() { + super.lroStartSucceeded(); + } + + @Override + public void onRequest() { + if (applicationLatencyTimer.isRunning()) { + totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); + applicationLatencyTimer.reset(); + } + } + + @Override + public void responseReceived() { + if (!applicationLatencyTimer.isRunning()) { + applicationLatencyTimer.start(); + } + if (firstResponsePerOpTimer.isRunning()) { + firstResponsePerOpTimer.stop(); + } + } + + @Override + public void requestSent() { + super.requestSent(); + } + + @Override + public void batchRequestSent(long elementCount, long requestSize) { + super.batchRequestSent(elementCount, requestSize); + } + + @Override + public int getAttempt() { + return attempt; + } + + @Override + public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) { + // Record the metrics and put in the map after the attempt is done, so we can have cluster and + // zone information + if (latency != null) { + recorder.putGfeLatencies(latency); + } else { + gfeMissingHeaders.incrementAndGet(); + } + recorder.putGfeMissingHeaders(gfeMissingHeaders.get()); + } + + @Override + public void setLocations(String zone, String cluster) { + this.zone = zone; + this.cluster = cluster; + } + + @Override + public void batchRequestThrottled(long throttledTimeMs) { + recorder.putBatchRequestThrottled(throttledTimeMs); + } + + private void recordOperationCompletion(@Nullable Throwable status) { + if (!opFinished.compareAndSet(false, true)) { + return; + } + operationTimer.stop(); + + recorder.putRetryCount(attemptCount); + + if (applicationLatencyTimer.isRunning()) { + applicationLatencyTimer.stop(); + totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); + } + recorder.putApplicationLatencies(totalApplicationLatency.get()); + + recorder.putOperationLatencies(operationTimer.elapsed(TimeUnit.MILLISECONDS)); + + if (operationType == ApiTracerFactory.OperationType.ServerStreaming + && spanName.getMethodName().equals("ReadRows")) { + recorder.putFirstResponseLatencies(firstResponsePerOpTimer.elapsed(TimeUnit.MILLISECONDS)); + } + + recorder.record(Util.extractStatus(status), tableId, zone, cluster); + } + + private void recordAttemptCompletion(@Nullable Throwable status) { + recorder.putAttemptLatencies(attemptTimer.elapsed(TimeUnit.MILLISECONDS)); + + recorder.record(Util.extractStatus(status), tableId, zone, cluster); + } +} 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 new file mode 100644 index 0000000000..3583733a45 --- /dev/null +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerFactory.java @@ -0,0 +1,66 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import com.google.api.core.InternalApi; +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.BaseApiTracerFactory; +import com.google.api.gax.tracing.SpanName; +import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import com.google.cloud.bigtable.stats.StatsWrapper; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; + +/** + * {@link ApiTracerFactory} that will generate OpenCensus metrics by using the {@link ApiTracer} + * api. + */ +@InternalApi("For internal use only") +public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { + + private final ImmutableMap statsAttributes; + private final StatsWrapper statsWrapper; + private final StatsRecorderWrapper statsRecorderWrapper; + + public static BuiltinMetricsTracerFactory create( + StatsWrapper statsWrapper, ImmutableMap statsAttributes) { + return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, null); + } + + @VisibleForTesting + static BuiltinMetricsTracerFactory create( + StatsWrapper statsWrapper, + ImmutableMap statsAttributes, + StatsRecorderWrapper recorder) { + return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, recorder); + } + + private BuiltinMetricsTracerFactory( + StatsWrapper statsWrapper, + ImmutableMap statsAttributes, + StatsRecorderWrapper recorder) { + this.statsAttributes = statsAttributes; + this.statsWrapper = statsWrapper; + this.statsRecorderWrapper = recorder; + } + + @Override + public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { + return new BuiltinMetricsTracer( + operationType, spanName, statsAttributes, statsWrapper, statsRecorderWrapper); + } +} 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 5f4580743b..29c16baafb 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 @@ -92,9 +92,14 @@ public void connectionSelected(String id) { @Override public void attemptStarted(int attemptNumber) { + attemptStarted(null, attemptNumber); + } + + @Override + public void attemptStarted(Object request, int attemptNumber) { this.attempt = attemptNumber; for (ApiTracer child : children) { - child.attemptStarted(attemptNumber); + child.attemptStarted(request, attemptNumber); } } @@ -185,4 +190,18 @@ public void batchRequestThrottled(long throttledTimeMs) { tracer.batchRequestThrottled(throttledTimeMs); } } + + @Override + public void setLocations(String zone, String cluster) { + for (BigtableTracer tracer : bigtableTracers) { + tracer.setLocations(zone, cluster); + } + } + + @Override + public void onRequest() { + for (BigtableTracer tracer : bigtableTracers) { + tracer.onRequest(); + } + } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java index f28b07c0cb..d27941bb71 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java @@ -118,7 +118,9 @@ private void recordOperationCompletion(@Nullable Throwable throwable) { TagContextBuilder tagCtx = newTagCtxBuilder() - .putLocal(RpcMeasureConstants.BIGTABLE_STATUS, Util.extractStatus(throwable)); + .putLocal( + RpcMeasureConstants.BIGTABLE_STATUS, + TagValue.create(Util.extractStatus(throwable))); measures.record(tagCtx.build()); } @@ -171,7 +173,9 @@ private void recordAttemptCompletion(@Nullable Throwable throwable) { TagContextBuilder tagCtx = newTagCtxBuilder() - .putLocal(RpcMeasureConstants.BIGTABLE_STATUS, Util.extractStatus(throwable)); + .putLocal( + RpcMeasureConstants.BIGTABLE_STATUS, + TagValue.create(Util.extractStatus(throwable))); measures.record(tagCtx.build()); } @@ -222,7 +226,8 @@ public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwa } measures.record( newTagCtxBuilder() - .putLocal(RpcMeasureConstants.BIGTABLE_STATUS, Util.extractStatus(throwable)) + .putLocal( + RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create(Util.extractStatus(throwable))) .build()); } @@ -248,4 +253,14 @@ private TagContextBuilder newTagCtxBuilder() { return tagCtx; } + + @Override + public void setLocations(String zone, String cluster) { + // noop + } + + @Override + public void onRequest() { + // noop + } } 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 00995b717a..0440029027 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 @@ -15,10 +15,19 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; +import com.google.api.core.InternalApi; import com.google.api.gax.rpc.ApiCallContext; 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.bigtable.v2.CheckAndMutateRowRequest; +import com.google.bigtable.v2.MutateRowRequest; +import com.google.bigtable.v2.MutateRowsRequest; +import com.google.bigtable.v2.ReadModifyWriteRowRequest; +import com.google.bigtable.v2.ReadRowsRequest; +import com.google.bigtable.v2.SampleRowKeysRequest; +import com.google.bigtable.v2.TableName; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import io.grpc.Metadata; import io.grpc.Status; @@ -38,7 +47,8 @@ import javax.annotation.Nullable; /** Utilities to help integrating with OpenCensus. */ -class Util { +@InternalApi("For internal use only") +public class Util { static final Metadata.Key ATTEMPT_HEADER_KEY = Metadata.Key.of("bigtable-attempt", Metadata.ASCII_STRING_MARSHALLER); static final Metadata.Key ATTEMPT_EPOCH_KEY = @@ -48,14 +58,14 @@ class Util { Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER); private static final Pattern SERVER_TIMING_HEADER_PATTERN = Pattern.compile(".*dur=(?\\d+)"); - private static final TagValue OK_STATUS = TagValue.create(StatusCode.Code.OK.toString()); + static final String TRAILER_KEY = "x-goog-ext-425905942-bin"; - /** Convert an exception into a value that can be used as an OpenCensus tag value. */ - static TagValue extractStatus(@Nullable Throwable error) { + /** Convert an exception into a value that can be used to create an OpenCensus tag value. */ + static String extractStatus(@Nullable Throwable error) { final String statusString; if (error == null) { - return OK_STATUS; + return StatusCode.Code.OK.toString(); } else if (error instanceof CancellationException) { statusString = Status.Code.CANCELLED.toString(); } else if (error instanceof ApiException) { @@ -68,14 +78,14 @@ static TagValue extractStatus(@Nullable Throwable error) { statusString = Code.UNKNOWN.toString(); } - return TagValue.create(statusString); + return statusString; } /** * Await the result of the future and convert it into a value that can be used as an OpenCensus * tag value. */ - static TagValue extractStatus(Future future) { + static TagValue extractStatusFromFuture(Future future) { Throwable error = null; try { @@ -88,7 +98,25 @@ static TagValue extractStatus(Future future) { } catch (RuntimeException e) { error = e; } - return extractStatus(error); + return TagValue.create(extractStatus(error)); + } + + static String extractTableId(Object request) { + String tableName = null; + if (request instanceof ReadRowsRequest) { + tableName = ((ReadRowsRequest) request).getTableName(); + } else if (request instanceof MutateRowsRequest) { + tableName = ((MutateRowsRequest) request).getTableName(); + } else if (request instanceof MutateRowRequest) { + tableName = ((MutateRowRequest) request).getTableName(); + } else if (request instanceof SampleRowKeysRequest) { + tableName = ((SampleRowKeysRequest) request).getTableName(); + } else if (request instanceof CheckAndMutateRowRequest) { + tableName = ((CheckAndMutateRowRequest) request).getTableName(); + } else if (request instanceof ReadModifyWriteRowRequest) { + tableName = ((ReadModifyWriteRowRequest) request).getTableName(); + } + return !Strings.isNullOrEmpty(tableName) ? TableName.parse(tableName).getTable() : "undefined"; } /** 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 new file mode 100644 index 0000000000..9cc30df23b --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java @@ -0,0 +1,339 @@ +/* + * Copyright 2021 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import com.google.api.client.util.Lists; +import com.google.api.core.SettableApiFuture; +import com.google.api.gax.rpc.ClientContext; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.StreamController; +import com.google.bigtable.v2.BigtableGrpc; +import com.google.bigtable.v2.MutateRowRequest; +import com.google.bigtable.v2.MutateRowResponse; +import com.google.bigtable.v2.ReadRowsRequest; +import com.google.bigtable.v2.ReadRowsResponse; +import com.google.cloud.bigtable.data.v2.BigtableDataSettings; +import com.google.cloud.bigtable.data.v2.FakeServiceBuilder; +import com.google.cloud.bigtable.data.v2.models.Query; +import com.google.cloud.bigtable.data.v2.models.Row; +import com.google.cloud.bigtable.data.v2.models.RowMutation; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; +import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; +import com.google.cloud.bigtable.stats.StatsRecorderWrapper; +import com.google.cloud.bigtable.stats.StatsWrapper; +import com.google.common.base.Stopwatch; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Range; +import com.google.protobuf.ByteString; +import com.google.protobuf.BytesValue; +import com.google.protobuf.StringValue; +import io.grpc.ForwardingServerCall; +import io.grpc.Metadata; +import io.grpc.Server; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.grpc.stub.StreamObserver; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.threeten.bp.Duration; + +public class BuiltinMetricsTracerTest { + private static final String PROJECT_ID = "fake-project"; + private static final String INSTANCE_ID = "fake-instance"; + private static final String APP_PROFILE_ID = "default"; + private static final String TABLE_ID = "fake-table"; + private static final String UNDEFINED = "undefined"; + private static final long FAKE_SERVER_TIMING = 50; + private static final long SERVER_LATENCY = 500; + + private final AtomicInteger rpcCount = new AtomicInteger(0); + + private static final ReadRowsResponse READ_ROWS_RESPONSE_1 = + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("fake-key-1")) + .setFamilyName(StringValue.of("cf")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue(ByteString.copyFromUtf8("value")) + .setCommitRow(true)) + .build(); + private static final ReadRowsResponse READ_ROWS_RESPONSE_2 = + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("fake-key-2")) + .setFamilyName(StringValue.of("cf")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue(ByteString.copyFromUtf8("value")) + .setCommitRow(true)) + .build(); + private static final ReadRowsResponse READ_ROWS_RESPONSE_3 = + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("fake-key-3")) + .setFamilyName(StringValue.of("cf")) + .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue(ByteString.copyFromUtf8("value")) + .setCommitRow(true)) + .build(); + + @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); + + private BigtableGrpc.BigtableImplBase mockService; + private Server server; + + private EnhancedBigtableStub stub; + + private StatsRecorderWrapper statsRecorderWrapper; + private ArgumentCaptor longValue; + private ArgumentCaptor intValue; + private ArgumentCaptor status; + private ArgumentCaptor tableId; + private ArgumentCaptor zone; + private ArgumentCaptor cluster; + + private Stopwatch serverRetryDelayStopwatch; + private AtomicLong serverTotalRetryDelay; + + @Before + public void setUp() throws Exception { + mockService = new FakeService(); + + serverRetryDelayStopwatch = Stopwatch.createUnstarted(); + serverTotalRetryDelay = new AtomicLong(0); + + // Add an interceptor to send location information in the trailers and add server-timing in + // headers + ServerInterceptor trailersInterceptor = + new ServerInterceptor() { + private AtomicInteger count = new AtomicInteger(0); + + @Override + public ServerCall.Listener interceptCall( + ServerCall serverCall, + Metadata metadata, + ServerCallHandler serverCallHandler) { + return serverCallHandler.startCall( + new ForwardingServerCall.SimpleForwardingServerCall(serverCall) { + @Override + public void sendHeaders(Metadata headers) { + headers.put( + Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER), + String.format("gfet4t7; dur=%d", FAKE_SERVER_TIMING)); + super.sendHeaders(headers); + } + + @Override + public void close(Status status, Metadata trailers) { + super.close(status, trailers); + } + }, + metadata); + } + }; + + server = FakeServiceBuilder.create(mockService).intercept(trailersInterceptor).start(); + + statsRecorderWrapper = Mockito.mock(StatsRecorderWrapper.class); + + BigtableDataSettings settings = + BigtableDataSettings.newBuilderForEmulator(server.getPort()) + .setProjectId(PROJECT_ID) + .setInstanceId(INSTANCE_ID) + .setAppProfileId(APP_PROFILE_ID) + .build(); + EnhancedBigtableStubSettings.Builder stubSettingsBuilder = + settings.getStubSettings().toBuilder(); + stubSettingsBuilder + .mutateRowSettings() + .retrySettings() + .setInitialRetryDelay(Duration.ofMillis(200)); + stubSettingsBuilder.setTracerFactory( + BuiltinMetricsTracerFactory.create( + StatsWrapper.get(), ImmutableMap.of(), statsRecorderWrapper)); + + EnhancedBigtableStubSettings stubSettings = stubSettingsBuilder.build(); + stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); + + longValue = ArgumentCaptor.forClass(long.class); + intValue = ArgumentCaptor.forClass(int.class); + status = ArgumentCaptor.forClass(String.class); + tableId = ArgumentCaptor.forClass(String.class); + zone = ArgumentCaptor.forClass(String.class); + cluster = ArgumentCaptor.forClass(String.class); + } + + @After + public void tearDown() { + stub.close(); + server.shutdown(); + } + + @Test + public void testOperationLatencies() { + Stopwatch stopwatch = Stopwatch.createStarted(); + Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator()); + long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); + + verify(statsRecorderWrapper).putOperationLatencies(longValue.capture()); + + assertThat(longValue.getValue()).isIn(Range.closed(SERVER_LATENCY, elapsed)); + } + + @Test + public void testGfeMetrics() { + Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID))); + + verify(statsRecorderWrapper).putGfeLatencies(longValue.capture()); + assertThat(longValue.getValue()).isEqualTo(FAKE_SERVER_TIMING); + + verify(statsRecorderWrapper).putGfeMissingHeaders(longValue.capture()); + assertThat(longValue.getValue()).isEqualTo(0); + } + + @Test + public void testReadRowsApplicationLatency() throws Exception { + final long applicationLatency = 1000; + final SettableApiFuture future = SettableApiFuture.create(); + final AtomicInteger counter = new AtomicInteger(0); + // We want to measure how long application waited before requesting another message after + // the previous message is returned from the server. Using ResponseObserver here so that the + // flow will be + // onResponse() -> sleep -> onRequest() (for the next message) which is exactly what we want to + // measure for + // application latency. + // If we do readRowsCallable().call(Query.create(TABLE_ID)).iterator() and iterate through the + // iterator and sleep in + // between responses, when the call started, the client will pre-fetch the first response, which + // won't be counted + // in application latency. So the test will be flaky and hard to debug. + stub.readRowsCallable() + .call( + Query.create(TABLE_ID), + new ResponseObserver() { + @Override + public void onStart(StreamController streamController) {} + + @Override + public void onResponse(Row row) { + try { + counter.incrementAndGet(); + Thread.sleep(applicationLatency); + } catch (InterruptedException e) { + } + } + + @Override + public void onError(Throwable throwable) { + future.setException(throwable); + } + + @Override + public void onComplete() { + future.set(null); + } + }); + future.get(); + + verify(statsRecorderWrapper).putApplicationLatencies(longValue.capture()); + + assertThat(counter.get()).isGreaterThan(0); + assertThat(longValue.getValue()).isAtLeast(applicationLatency * counter.get()); + assertThat(longValue.getValue()).isLessThan(applicationLatency * (counter.get() + 1)); + } + + @Test + public void testRetryCount() { + stub.mutateRowCallable() + .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); + + verify(statsRecorderWrapper).putRetryCount(intValue.capture()); + + assertThat(intValue.getValue()).isEqualTo(3); + } + + @Test + public void testMutateRowAttempts() { + stub.mutateRowCallable() + .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); + + verify(statsRecorderWrapper, times(4)) + .record(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); + assertThat(zone.getAllValues().get(0)).isEqualTo(UNDEFINED); + assertThat(zone.getAllValues().get(1)).isEqualTo(UNDEFINED); + assertThat(zone.getAllValues().get(2)).isEqualTo(UNDEFINED); + assertThat(cluster.getAllValues().get(0)).isEqualTo(UNDEFINED); + assertThat(cluster.getAllValues().get(1)).isEqualTo(UNDEFINED); + assertThat(cluster.getAllValues().get(2)).isEqualTo(UNDEFINED); + assertThat(status.getAllValues().get(0)).isEqualTo("UNAVAILABLE"); + assertThat(status.getAllValues().get(1)).isEqualTo("UNAVAILABLE"); + assertThat(status.getAllValues().get(2)).isEqualTo("OK"); + } + + private class FakeService extends BigtableGrpc.BigtableImplBase { + + @Override + public void readRows( + ReadRowsRequest request, StreamObserver responseObserver) { + try { + Thread.sleep(SERVER_LATENCY); + } catch (InterruptedException e) { + } + responseObserver.onNext(READ_ROWS_RESPONSE_1); + responseObserver.onNext(READ_ROWS_RESPONSE_2); + responseObserver.onNext(READ_ROWS_RESPONSE_3); + responseObserver.onCompleted(); + } + + @Override + public void mutateRow( + MutateRowRequest request, StreamObserver responseObserver) { + if (serverRetryDelayStopwatch.isRunning()) { + serverTotalRetryDelay.addAndGet(serverRetryDelayStopwatch.elapsed(TimeUnit.MILLISECONDS)); + serverRetryDelayStopwatch.reset(); + } + if (rpcCount.get() < 2) { + responseObserver.onError(new StatusRuntimeException(Status.UNAVAILABLE)); + rpcCount.getAndIncrement(); + serverRetryDelayStopwatch.start(); + return; + } + responseObserver.onNext(MutateRowResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + } +} diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java index 69a741d0e3..0de14636c6 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/CompositeTracerTest.java @@ -23,6 +23,7 @@ import com.google.api.gax.tracing.ApiTracer; import com.google.api.gax.tracing.ApiTracer.Scope; +import com.google.bigtable.v2.ReadRowsRequest; import com.google.cloud.bigtable.misc_utilities.MethodComparator; import com.google.common.collect.ImmutableList; import io.grpc.Status; @@ -118,11 +119,12 @@ public void testConnectionSelected() { @Test public void testAttemptStarted() { - compositeTracer.attemptStarted(3); - verify(child1, times(1)).attemptStarted(3); - verify(child2, times(1)).attemptStarted(3); - verify(child3, times(1)).attemptStarted(3); - verify(child4, times(1)).attemptStarted(3); + ReadRowsRequest request = ReadRowsRequest.getDefaultInstance(); + compositeTracer.attemptStarted(request, 3); + verify(child1, times(1)).attemptStarted(request, 3); + verify(child2, times(1)).attemptStarted(request, 3); + verify(child3, times(1)).attemptStarted(request, 3); + verify(child4, times(1)).attemptStarted(request, 3); } @Test diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java index efef3b67d2..3c0fb4e617 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/UtilTest.java @@ -30,13 +30,13 @@ public class UtilTest { @Test public void testOk() { - TagValue tagValue = Util.extractStatus((Throwable) null); + TagValue tagValue = TagValue.create(Util.extractStatus((Throwable) null)); assertThat(tagValue.asString()).isEqualTo("OK"); } @Test public void testOkFuture() { - TagValue tagValue = Util.extractStatus(Futures.immediateFuture(null)); + TagValue tagValue = Util.extractStatusFromFuture(Futures.immediateFuture(null)); assertThat(tagValue.asString()).isEqualTo("OK"); } @@ -45,7 +45,7 @@ public void testError() { DeadlineExceededException error = new DeadlineExceededException( "Deadline exceeded", null, GrpcStatusCode.of(Status.Code.DEADLINE_EXCEEDED), true); - TagValue tagValue = Util.extractStatus(error); + TagValue tagValue = TagValue.create(Util.extractStatus(error)); assertThat(tagValue.asString()).isEqualTo("DEADLINE_EXCEEDED"); } @@ -54,13 +54,13 @@ public void testErrorFuture() { DeadlineExceededException error = new DeadlineExceededException( "Deadline exceeded", null, GrpcStatusCode.of(Status.Code.DEADLINE_EXCEEDED), true); - TagValue tagValue = Util.extractStatus(Futures.immediateFailedFuture(error)); + TagValue tagValue = Util.extractStatusFromFuture(Futures.immediateFailedFuture(error)); assertThat(tagValue.asString()).isEqualTo("DEADLINE_EXCEEDED"); } @Test public void testCancelledFuture() { - TagValue tagValue = Util.extractStatus(Futures.immediateCancelledFuture()); + TagValue tagValue = Util.extractStatusFromFuture(Futures.immediateCancelledFuture()); assertThat(tagValue.asString()).isEqualTo("CANCELLED"); } } From b91b8e9acf702f8bebf39f22ae6e25fa5dea4f65 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 8 Jun 2022 20:50:02 -0400 Subject: [PATCH 06/15] update on comments --- .../data/v2/stub/EnhancedBigtableStub.java | 2 +- .../data/v2/stub/metrics/BigtableTracer.java | 7 ++- .../v2/stub/metrics/BuiltinMetricsTracer.java | 7 ++- .../metrics/BuiltinMetricsTracerFactory.java | 11 ++-- .../metrics/BuiltinMetricsTracerTest.java | 55 +++++++------------ 5 files changed, 35 insertions(+), 47 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 4087dc59b4..6df57061dc 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 @@ -154,7 +154,7 @@ public static EnhancedBigtableStubSettings finalizeSettings( EnhancedBigtableStubSettings settings, Tagger tagger, StatsRecorder stats) throws IOException { EnhancedBigtableStubSettings.Builder builder = settings.toBuilder(); - StatsWrapper statsWrapper = StatsWrapper.get(); + StatsWrapper statsWrapper = StatsWrapper.create(); // TODO: this implementation is on the cusp of unwieldy, if we end up adding more features // consider splitting it up by feature. 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 421c5b4724..6b199e4223 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 @@ -35,7 +35,7 @@ public void attemptStarted(int attemptNumber) { this.attempt = attemptNumber; } - /** annotate when onRequest is called */ + /** annotate when onRequest is called. This will be called in BuiltinMetricsTracer. */ public void onRequest() { // noop } @@ -63,7 +63,10 @@ public void batchRequestThrottled(long throttledTimeMs) { // noop } - /** Set the Bigtable zone and cluster so metrics can be tagged with location information. */ + /** + * Set the Bigtable zone and cluster so metrics can be tagged with location information. This will + * be called in BuiltinMetricsTracer. + */ public void setLocations(String zone, String cluster) { // 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 d369cbfcb6..48fc7dcce8 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 @@ -66,11 +66,12 @@ class BuiltinMetricsTracer extends BigtableTracer { SpanName spanName, Map attributes, StatsWrapper statsWrapper, - StatsRecorderWrapper recorder) { + @Nullable StatsRecorderWrapper statsRecorderWrapper) { this.operationType = operationType; this.spanName = spanName; - if (recorder != null) { - this.recorder = recorder; + if (statsRecorderWrapper != null) { + // A workaround for test to pass in a mock StatsRecorderWrapper + this.recorder = statsRecorderWrapper; } else { this.recorder = new StatsRecorderWrapper(operationType, spanName, attributes, statsWrapper); } 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 3583733a45..6a5a9b6910 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 @@ -41,21 +41,22 @@ public static BuiltinMetricsTracerFactory create( return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, null); } + // A workaround for test to pass in a mock StatsRecorderWrapper @VisibleForTesting - static BuiltinMetricsTracerFactory create( + static BuiltinMetricsTracerFactory createWithRecorder( StatsWrapper statsWrapper, ImmutableMap statsAttributes, - StatsRecorderWrapper recorder) { - return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, recorder); + StatsRecorderWrapper statsRecorderWrapper) { + return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, statsRecorderWrapper); } private BuiltinMetricsTracerFactory( StatsWrapper statsWrapper, ImmutableMap statsAttributes, - StatsRecorderWrapper recorder) { + StatsRecorderWrapper statsRecorderWrapper) { this.statsAttributes = statsAttributes; this.statsWrapper = statsWrapper; - this.statsRecorderWrapper = recorder; + this.statsRecorderWrapper = statsRecorderWrapper; } @Override 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 9cc30df23b..21d4c626f9 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 @@ -1,5 +1,5 @@ /* - * Copyright 2021 Google LLC + * Copyright 2022 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -61,7 +61,8 @@ import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.mockito.Mockito; +import org.mockito.Captor; +import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; import org.threeten.bp.Duration; @@ -118,13 +119,14 @@ public class BuiltinMetricsTracerTest { private EnhancedBigtableStub stub; - private StatsRecorderWrapper statsRecorderWrapper; - private ArgumentCaptor longValue; - private ArgumentCaptor intValue; - private ArgumentCaptor status; - private ArgumentCaptor tableId; - private ArgumentCaptor zone; - private ArgumentCaptor cluster; + @Mock private StatsRecorderWrapper statsRecorderWrapper; + + @Captor private ArgumentCaptor longValue; + @Captor private ArgumentCaptor intValue; + @Captor private ArgumentCaptor status; + @Captor private ArgumentCaptor tableId; + @Captor private ArgumentCaptor zone; + @Captor private ArgumentCaptor cluster; private Stopwatch serverRetryDelayStopwatch; private AtomicLong serverTotalRetryDelay; @@ -136,8 +138,7 @@ public void setUp() throws Exception { serverRetryDelayStopwatch = Stopwatch.createUnstarted(); serverTotalRetryDelay = new AtomicLong(0); - // Add an interceptor to send location information in the trailers and add server-timing in - // headers + // Add an interceptor to add server-timing in headers ServerInterceptor trailersInterceptor = new ServerInterceptor() { private AtomicInteger count = new AtomicInteger(0); @@ -156,11 +157,6 @@ public void sendHeaders(Metadata headers) { String.format("gfet4t7; dur=%d", FAKE_SERVER_TIMING)); super.sendHeaders(headers); } - - @Override - public void close(Status status, Metadata trailers) { - super.close(status, trailers); - } }, metadata); } @@ -168,8 +164,6 @@ public void close(Status status, Metadata trailers) { server = FakeServiceBuilder.create(mockService).intercept(trailersInterceptor).start(); - statsRecorderWrapper = Mockito.mock(StatsRecorderWrapper.class); - BigtableDataSettings settings = BigtableDataSettings.newBuilderForEmulator(server.getPort()) .setProjectId(PROJECT_ID) @@ -183,18 +177,11 @@ public void close(Status status, Metadata trailers) { .retrySettings() .setInitialRetryDelay(Duration.ofMillis(200)); stubSettingsBuilder.setTracerFactory( - BuiltinMetricsTracerFactory.create( - StatsWrapper.get(), ImmutableMap.of(), statsRecorderWrapper)); + BuiltinMetricsTracerFactory.createWithRecorder( + StatsWrapper.create(), ImmutableMap.of(), statsRecorderWrapper)); EnhancedBigtableStubSettings stubSettings = stubSettingsBuilder.build(); stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); - - longValue = ArgumentCaptor.forClass(long.class); - intValue = ArgumentCaptor.forClass(int.class); - status = ArgumentCaptor.forClass(String.class); - tableId = ArgumentCaptor.forClass(String.class); - zone = ArgumentCaptor.forClass(String.class); - cluster = ArgumentCaptor.forClass(String.class); } @After @@ -291,17 +278,13 @@ public void testMutateRowAttempts() { stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); + // record will get called 4 times, 3 times for attempts and 1 for recording operation level + // metrics. verify(statsRecorderWrapper, times(4)) .record(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); - assertThat(zone.getAllValues().get(0)).isEqualTo(UNDEFINED); - assertThat(zone.getAllValues().get(1)).isEqualTo(UNDEFINED); - assertThat(zone.getAllValues().get(2)).isEqualTo(UNDEFINED); - assertThat(cluster.getAllValues().get(0)).isEqualTo(UNDEFINED); - assertThat(cluster.getAllValues().get(1)).isEqualTo(UNDEFINED); - assertThat(cluster.getAllValues().get(2)).isEqualTo(UNDEFINED); - assertThat(status.getAllValues().get(0)).isEqualTo("UNAVAILABLE"); - assertThat(status.getAllValues().get(1)).isEqualTo("UNAVAILABLE"); - assertThat(status.getAllValues().get(2)).isEqualTo("OK"); + assertThat(zone.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); + assertThat(cluster.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); + assertThat(status.getAllValues()).containsExactly("UNAVAILABLE", "UNAVAILABLE", "OK", "OK"); } private class FakeService extends BigtableGrpc.BigtableImplBase { From 35bb7fac464a3db52017710574bb48318851aa71 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Thu, 9 Jun 2022 10:59:12 -0400 Subject: [PATCH 07/15] make stopwatch thread safe --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 18 ++++++++---- .../HeaderTracerStreamingCallable.java | 29 ++++++++++++++++++- .../metrics/BuiltinMetricsTracerTest.java | 12 -------- 3 files changed, 40 insertions(+), 19 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 48fc7dcce8..cf4c850e62 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 @@ -50,8 +50,14 @@ class BuiltinMetricsTracer extends BigtableTracer { private volatile int attempt = 0; // Total application latency - private final Stopwatch applicationLatencyTimer = Stopwatch.createUnstarted(); + // Total application latency needs to be atomic because it's accessed from different threads. E.g. + // request() from + // user thread and attempt failed from grpc thread. private final AtomicLong totalApplicationLatency = new AtomicLong(0); + // Stopwatch is not thread safe so this is a workaround to check if the stopwatch changes is + // flushed to memory. + private final AtomicBoolean applicationLatencyTimerIsRunning = new AtomicBoolean(); + private final Stopwatch applicationLatencyTimer = Stopwatch.createUnstarted(); // Monitored resource labels private String tableId = "undefined"; @@ -113,7 +119,7 @@ public void attemptStarted(Object request, int attemptNumber) { if (request != null) { this.tableId = Util.extractTableId(request); } - if (applicationLatencyTimer.isRunning()) { + if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); applicationLatencyTimer.reset(); } @@ -131,7 +137,7 @@ public void attemptCancelled() { @Override public void attemptFailed(Throwable error, Duration delay) { - if (!applicationLatencyTimer.isRunning()) { + if (applicationLatencyTimerIsRunning.compareAndSet(false, true)) { applicationLatencyTimer.start(); } recordAttemptCompletion(error); @@ -159,7 +165,7 @@ public void lroStartSucceeded() { @Override public void onRequest() { - if (applicationLatencyTimer.isRunning()) { + if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); applicationLatencyTimer.reset(); } @@ -167,7 +173,7 @@ public void onRequest() { @Override public void responseReceived() { - if (!applicationLatencyTimer.isRunning()) { + if (applicationLatencyTimerIsRunning.compareAndSet(false, true)) { applicationLatencyTimer.start(); } if (firstResponsePerOpTimer.isRunning()) { @@ -221,7 +227,7 @@ private void recordOperationCompletion(@Nullable Throwable status) { recorder.putRetryCount(attemptCount); - if (applicationLatencyTimer.isRunning()) { + if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { applicationLatencyTimer.stop(); totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java index 31c5cf1960..8f5c9871b4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -82,7 +82,8 @@ private class HeaderTracerResponseObserver implements ResponseObserve @Override public void onStart(final StreamController controller) { - outerObserver.onStart(controller); + final StreamController tracedController = new TracedStreamController(controller, tracer); + outerObserver.onStart(tracedController); } @Override @@ -108,4 +109,30 @@ public void onComplete() { outerObserver.onComplete(); } } + + private class TracedStreamController implements StreamController { + private final StreamController innerController; + private final BigtableTracer tracer; + + TracedStreamController(StreamController innerController, BigtableTracer tracer) { + this.innerController = innerController; + this.tracer = tracer; + } + + @Override + public void cancel() { + innerController.cancel(); + } + + @Override + public void disableAutoInboundFlowControl() { + innerController.disableAutoInboundFlowControl(); + } + + @Override + public void request(int i) { + tracer.onRequest(); + innerController.request(i); + } + } } 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 21d4c626f9..5e62c44748 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 @@ -55,7 +55,6 @@ import io.grpc.stub.StreamObserver; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicLong; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -128,16 +127,10 @@ public class BuiltinMetricsTracerTest { @Captor private ArgumentCaptor zone; @Captor private ArgumentCaptor cluster; - private Stopwatch serverRetryDelayStopwatch; - private AtomicLong serverTotalRetryDelay; - @Before public void setUp() throws Exception { mockService = new FakeService(); - serverRetryDelayStopwatch = Stopwatch.createUnstarted(); - serverTotalRetryDelay = new AtomicLong(0); - // Add an interceptor to add server-timing in headers ServerInterceptor trailersInterceptor = new ServerInterceptor() { @@ -305,14 +298,9 @@ public void readRows( @Override public void mutateRow( MutateRowRequest request, StreamObserver responseObserver) { - if (serverRetryDelayStopwatch.isRunning()) { - serverTotalRetryDelay.addAndGet(serverRetryDelayStopwatch.elapsed(TimeUnit.MILLISECONDS)); - serverRetryDelayStopwatch.reset(); - } if (rpcCount.get() < 2) { responseObserver.onError(new StatusRuntimeException(Status.UNAVAILABLE)); rpcCount.getAndIncrement(); - serverRetryDelayStopwatch.start(); return; } responseObserver.onNext(MutateRowResponse.getDefaultInstance()); From 9c71da930cab9fab4902473d02b0cd99f8e5e464 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 21 Jun 2022 14:52:28 -0400 Subject: [PATCH 08/15] update comments --- .../data/v2/stub/EnhancedBigtableStub.java | 4 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 57 +++---------------- .../metrics/BuiltinMetricsTracerFactory.java | 29 ++-------- .../HeaderTracerStreamingCallable.java | 2 +- .../data/v2/stub/metrics/MetricsTracer.java | 35 ------------ .../metrics/BuiltinMetricsTracerTest.java | 44 ++++++++++++-- .../v2/stub/metrics/MetricsTracerTest.java | 12 ---- pom.xml | 1 - 8 files changed, 56 insertions(+), 128 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 6df57061dc..b6d17baadf 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 @@ -90,7 +90,6 @@ import com.google.cloud.bigtable.data.v2.stub.readrows.ReadRowsUserCallable; import com.google.cloud.bigtable.data.v2.stub.readrows.RowMergingCallable; import com.google.cloud.bigtable.gaxx.retrying.ApiResultRetryAlgorithm; -import com.google.cloud.bigtable.stats.StatsWrapper; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -154,7 +153,6 @@ public static EnhancedBigtableStubSettings finalizeSettings( EnhancedBigtableStubSettings settings, Tagger tagger, StatsRecorder stats) throws IOException { EnhancedBigtableStubSettings.Builder builder = settings.toBuilder(); - StatsWrapper statsWrapper = StatsWrapper.create(); // TODO: this implementation is on the cusp of unwieldy, if we end up adding more features // consider splitting it up by feature. @@ -227,7 +225,7 @@ public static EnhancedBigtableStubSettings finalizeSettings( .build()), // Add OpenCensus Metrics MetricsTracerFactory.create(tagger, stats, attributes), - BuiltinMetricsTracerFactory.create(statsWrapper, builtinAttributes), + BuiltinMetricsTracerFactory.create(builtinAttributes), // Add user configured tracer settings.getTracerFactory()))); return builder.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 cf4c850e62..7a3ae478e7 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 @@ -15,12 +15,12 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.api.gax.tracing.ApiTracerFactory; +import static com.google.api.gax.tracing.ApiTracerFactory.OperationType; + import com.google.api.gax.tracing.SpanName; import com.google.cloud.bigtable.stats.StatsRecorderWrapper; -import com.google.cloud.bigtable.stats.StatsWrapper; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; -import java.util.Map; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -36,7 +36,7 @@ class BuiltinMetricsTracer extends BigtableTracer { private final StatsRecorderWrapper recorder; - private final ApiTracerFactory.OperationType operationType; + private final OperationType operationType; private final SpanName spanName; // Operation level metrics @@ -51,8 +51,7 @@ class BuiltinMetricsTracer extends BigtableTracer { // Total application latency // Total application latency needs to be atomic because it's accessed from different threads. E.g. - // request() from - // user thread and attempt failed from grpc thread. + // request() from user thread and attempt failed from grpc thread. private final AtomicLong totalApplicationLatency = new AtomicLong(0); // Stopwatch is not thread safe so this is a workaround to check if the stopwatch changes is // flushed to memory. @@ -67,20 +66,12 @@ class BuiltinMetricsTracer extends BigtableTracer { // gfe stats private AtomicLong gfeMissingHeaders = new AtomicLong(0); + @VisibleForTesting BuiltinMetricsTracer( - ApiTracerFactory.OperationType operationType, - SpanName spanName, - Map attributes, - StatsWrapper statsWrapper, - @Nullable StatsRecorderWrapper statsRecorderWrapper) { + OperationType operationType, SpanName spanName, StatsRecorderWrapper recorder) { this.operationType = operationType; this.spanName = spanName; - if (statsRecorderWrapper != null) { - // A workaround for test to pass in a mock StatsRecorderWrapper - this.recorder = statsRecorderWrapper; - } else { - this.recorder = new StatsRecorderWrapper(operationType, spanName, attributes, statsWrapper); - } + this.recorder = recorder; } @Override @@ -143,26 +134,6 @@ public void attemptFailed(Throwable error, Duration delay) { recordAttemptCompletion(error); } - @Override - public void attemptFailedRetriesExhausted(Throwable error) { - super.attemptFailedRetriesExhausted(error); - } - - @Override - public void attemptPermanentFailure(Throwable error) { - super.attemptPermanentFailure(error); - } - - @Override - public void lroStartFailed(Throwable error) { - super.lroStartFailed(error); - } - - @Override - public void lroStartSucceeded() { - super.lroStartSucceeded(); - } - @Override public void onRequest() { if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { @@ -181,16 +152,6 @@ public void responseReceived() { } } - @Override - public void requestSent() { - super.requestSent(); - } - - @Override - public void batchRequestSent(long elementCount, long requestSize) { - super.batchRequestSent(elementCount, requestSize); - } - @Override public int getAttempt() { return attempt; @@ -235,7 +196,7 @@ private void recordOperationCompletion(@Nullable Throwable status) { recorder.putOperationLatencies(operationTimer.elapsed(TimeUnit.MILLISECONDS)); - if (operationType == ApiTracerFactory.OperationType.ServerStreaming + if (operationType == OperationType.ServerStreaming && spanName.getMethodName().equals("ReadRows")) { recorder.putFirstResponseLatencies(firstResponsePerOpTimer.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 6a5a9b6910..794997071d 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 @@ -20,9 +20,7 @@ import com.google.api.gax.tracing.ApiTracerFactory; import com.google.api.gax.tracing.BaseApiTracerFactory; import com.google.api.gax.tracing.SpanName; -import com.google.cloud.bigtable.stats.StatsRecorderWrapper; import com.google.cloud.bigtable.stats.StatsWrapper; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; /** @@ -33,35 +31,20 @@ public class BuiltinMetricsTracerFactory extends BaseApiTracerFactory { private final ImmutableMap statsAttributes; - private final StatsWrapper statsWrapper; - private final StatsRecorderWrapper statsRecorderWrapper; - public static BuiltinMetricsTracerFactory create( - StatsWrapper statsWrapper, ImmutableMap statsAttributes) { - return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, null); + public static BuiltinMetricsTracerFactory create(ImmutableMap statsAttributes) { + return new BuiltinMetricsTracerFactory(statsAttributes); } - // A workaround for test to pass in a mock StatsRecorderWrapper - @VisibleForTesting - static BuiltinMetricsTracerFactory createWithRecorder( - StatsWrapper statsWrapper, - ImmutableMap statsAttributes, - StatsRecorderWrapper statsRecorderWrapper) { - return new BuiltinMetricsTracerFactory(statsWrapper, statsAttributes, statsRecorderWrapper); - } - - private BuiltinMetricsTracerFactory( - StatsWrapper statsWrapper, - ImmutableMap statsAttributes, - StatsRecorderWrapper statsRecorderWrapper) { + private BuiltinMetricsTracerFactory(ImmutableMap statsAttributes) { this.statsAttributes = statsAttributes; - this.statsWrapper = statsWrapper; - this.statsRecorderWrapper = statsRecorderWrapper; } @Override public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { return new BuiltinMetricsTracer( - operationType, spanName, statsAttributes, statsWrapper, statsRecorderWrapper); + operationType, + spanName, + StatsWrapper.createRecorder(operationType, spanName, statsAttributes)); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java index 8f5c9871b4..b5af9d71b5 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -82,7 +82,7 @@ private class HeaderTracerResponseObserver implements ResponseObserve @Override public void onStart(final StreamController controller) { - final StreamController tracedController = new TracedStreamController(controller, tracer); + StreamController tracedController = new TracedStreamController(controller, tracer); outerObserver.onStart(tracedController); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java index d27941bb71..3c63b1b5f7 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracer.java @@ -125,11 +125,6 @@ private void recordOperationCompletion(@Nullable Throwable throwable) { measures.record(tagCtx.build()); } - @Override - public void connectionSelected(String s) { - // noop: cardinality for connection ids is too high to use as tags - } - @Override public void attemptStarted(int attemptNumber) { attempt = attemptNumber; @@ -180,16 +175,6 @@ private void recordAttemptCompletion(@Nullable Throwable throwable) { measures.record(tagCtx.build()); } - @Override - public void lroStartFailed(Throwable throwable) { - // noop - } - - @Override - public void lroStartSucceeded() { - // noop - } - @Override public void responseReceived() { if (firstResponsePerOpTimer.isRunning()) { @@ -199,16 +184,6 @@ public void responseReceived() { operationResponseCount++; } - @Override - public void requestSent() { - // noop: no operations are client streaming - } - - @Override - public void batchRequestSent(long elementCount, long requestSize) { - // noop - } - @Override public int getAttempt() { return attempt; @@ -253,14 +228,4 @@ private TagContextBuilder newTagCtxBuilder() { return tagCtx; } - - @Override - public void setLocations(String zone, String cluster) { - // noop - } - - @Override - public void onRequest() { - // noop - } } 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 5e62c44748..37484871ce 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 @@ -15,15 +15,19 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; +import static com.google.api.gax.tracing.ApiTracerFactory.OperationType; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.google.api.client.util.Lists; import com.google.api.core.SettableApiFuture; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.StreamController; +import com.google.api.gax.tracing.SpanName; import com.google.bigtable.v2.BigtableGrpc; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowResponse; @@ -37,9 +41,7 @@ import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.cloud.bigtable.stats.StatsRecorderWrapper; -import com.google.cloud.bigtable.stats.StatsWrapper; import com.google.common.base.Stopwatch; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Range; import com.google.protobuf.ByteString; import com.google.protobuf.BytesValue; @@ -59,6 +61,8 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; @@ -66,6 +70,7 @@ import org.mockito.junit.MockitoRule; import org.threeten.bp.Duration; +@RunWith(JUnit4.class) public class BuiltinMetricsTracerTest { private static final String PROJECT_ID = "fake-project"; private static final String INSTANCE_ID = "fake-instance"; @@ -118,6 +123,7 @@ public class BuiltinMetricsTracerTest { private EnhancedBigtableStub stub; + @Mock private BuiltinMetricsTracerFactory mockFactory; @Mock private StatsRecorderWrapper statsRecorderWrapper; @Captor private ArgumentCaptor longValue; @@ -169,9 +175,7 @@ public void sendHeaders(Metadata headers) { .mutateRowSettings() .retrySettings() .setInitialRetryDelay(Duration.ofMillis(200)); - stubSettingsBuilder.setTracerFactory( - BuiltinMetricsTracerFactory.createWithRecorder( - StatsWrapper.create(), ImmutableMap.of(), statsRecorderWrapper)); + stubSettingsBuilder.setTracerFactory(mockFactory); EnhancedBigtableStubSettings stubSettings = stubSettingsBuilder.build(); stub = new EnhancedBigtableStub(stubSettings, ClientContext.create(stubSettings)); @@ -185,6 +189,12 @@ public void tearDown() { @Test public void testOperationLatencies() { + when(mockFactory.newTracer(any(), any(), any())) + .thenReturn( + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); Stopwatch stopwatch = Stopwatch.createStarted(); Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator()); long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); @@ -196,6 +206,13 @@ public void testOperationLatencies() { @Test public void testGfeMetrics() { + when(mockFactory.newTracer(any(), any(), any())) + .thenReturn( + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); + Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID))); verify(statsRecorderWrapper).putGfeLatencies(longValue.capture()); @@ -207,6 +224,13 @@ public void testGfeMetrics() { @Test public void testReadRowsApplicationLatency() throws Exception { + when(mockFactory.newTracer(any(), any(), any())) + .thenReturn( + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); + final long applicationLatency = 1000; final SettableApiFuture future = SettableApiFuture.create(); final AtomicInteger counter = new AtomicInteger(0); @@ -258,6 +282,11 @@ public void onComplete() { @Test public void testRetryCount() { + when(mockFactory.newTracer(any(), any(), any())) + .thenReturn( + new BuiltinMetricsTracer( + OperationType.Unary, SpanName.of("Bigtable", "MutateRow"), statsRecorderWrapper)); + stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); @@ -268,6 +297,11 @@ public void testRetryCount() { @Test public void testMutateRowAttempts() { + when(mockFactory.newTracer(any(), any(), any())) + .thenReturn( + new BuiltinMetricsTracer( + OperationType.Unary, SpanName.of("Bigtable", "MutateRow"), statsRecorderWrapper)); + stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java index 1176214de3..b1b966ee9d 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/MetricsTracerTest.java @@ -42,7 +42,6 @@ import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStub; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.cloud.bigtable.data.v2.stub.mutaterows.MutateRowsBatchingDescriptor; -import com.google.cloud.bigtable.misc_utilities.MethodComparator; import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -59,8 +58,6 @@ import io.opencensus.tags.TagKey; import io.opencensus.tags.TagValue; import io.opencensus.tags.Tags; -import java.lang.reflect.Method; -import java.util.Arrays; import java.util.Iterator; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -485,15 +482,6 @@ public Object answer(InvocationOnMock invocation) { assertThat(throttledTimeMetric).isAtLeast(throttled); } - @Test - public void testMethodsOverride() { - Method[] baseMethods = BigtableTracer.class.getDeclaredMethods(); - Method[] metricsTracerMethods = MetricsTracer.class.getDeclaredMethods(); - assertThat(Arrays.asList(metricsTracerMethods)) - .comparingElementsUsing(MethodComparator.METHOD_CORRESPONDENCE) - .containsAtLeastElementsIn(baseMethods); - } - @SuppressWarnings("unchecked") private static StreamObserver anyObserver(Class returnType) { return (StreamObserver) any(returnType); diff --git a/pom.xml b/pom.xml index d786fd2741..74f02cdeb4 100644 --- a/pom.xml +++ b/pom.xml @@ -336,7 +336,6 @@ google-cloud-bigtable - google-cloud-bigtable-stats grpc-google-cloud-bigtable-admin-v2 grpc-google-cloud-bigtable-v2 proto-google-cloud-bigtable-admin-v2 From 02c43895ce1afd31ec33fcae59d0552555a72170 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 24 Jun 2022 12:31:24 -0400 Subject: [PATCH 09/15] calculate application latency correctly --- .../data/v2/stub/metrics/BigtableTracer.java | 14 +- .../v2/stub/metrics/BuiltinMetricsTracer.java | 90 +++++--- .../data/v2/stub/metrics/CompositeTracer.java | 18 +- .../HeaderTracerStreamingCallable.java | 11 +- .../metrics/HeaderTracerUnaryCallable.java | 2 +- .../metrics/BuiltinMetricsTracerTest.java | 211 +++++++++++------- .../metrics/HeaderTracerCallableTest.java | 19 -- 7 files changed, 235 insertions(+), 130 deletions(-) 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 6b199e4223..2640cc1ced 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 @@ -36,7 +36,19 @@ public void attemptStarted(int attemptNumber) { } /** annotate when onRequest is called. This will be called in BuiltinMetricsTracer. */ - public void onRequest() { + public void onRequest(int requestCount) { + // noop + } + + /** + * annotate when automatic flow control is disabled. This will be called in BuiltinMetricsTracer. + */ + public void disableFlowControl() { + // noop + } + + /** annotate after the callback from onResponse. This will be called in BuiltinMetricsTracer. */ + public void afterResponse(long applicationLatency) { // 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 7a3ae478e7..6e1080002a 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 @@ -24,6 +24,7 @@ import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import javax.annotation.Nullable; import org.threeten.bp.Duration; @@ -49,14 +50,18 @@ class BuiltinMetricsTracer extends BigtableTracer { private Stopwatch attemptTimer; private volatile int attempt = 0; - // Total application latency - // Total application latency needs to be atomic because it's accessed from different threads. E.g. - // request() from user thread and attempt failed from grpc thread. - private final AtomicLong totalApplicationLatency = new AtomicLong(0); + // Total server latency needs to be atomic because it's accessed from different threads. E.g. + // request() from user thread and attempt failed from grpc thread. We're only measuring the extra + // time application spent blocking grpc buffer, which will be operationLatency - serverLatency. + private final AtomicLong totalServerLatency = new AtomicLong(0); // Stopwatch is not thread safe so this is a workaround to check if the stopwatch changes is // flushed to memory. - private final AtomicBoolean applicationLatencyTimerIsRunning = new AtomicBoolean(); - private final Stopwatch applicationLatencyTimer = Stopwatch.createUnstarted(); + private final Stopwatch serverLatencyTimer = Stopwatch.createUnstarted(); + private final AtomicBoolean serverLatencyTimerIsRunning = new AtomicBoolean(); + + private boolean flowControlIsDisabled = false; + + private AtomicInteger requestLeft = new AtomicInteger(0); // Monitored resource labels private String tableId = "undefined"; @@ -110,9 +115,10 @@ public void attemptStarted(Object request, int attemptNumber) { if (request != null) { this.tableId = Util.extractTableId(request); } - if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { - totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); - applicationLatencyTimer.reset(); + if (!flowControlIsDisabled) { + if (serverLatencyTimerIsRunning.compareAndSet(false, true)) { + serverLatencyTimer.start(); + } } } @@ -128,27 +134,46 @@ public void attemptCancelled() { @Override public void attemptFailed(Throwable error, Duration delay) { - if (applicationLatencyTimerIsRunning.compareAndSet(false, true)) { - applicationLatencyTimer.start(); - } recordAttemptCompletion(error); } @Override - public void onRequest() { - if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { - totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); - applicationLatencyTimer.reset(); + public void onRequest(int requestCount) { + requestLeft.accumulateAndGet(requestCount, (x, y) -> x + y); + if (flowControlIsDisabled) { + // On request is only called when auto flow control is disabled. When auto flow control is + // disabled, server latency is measured between onRequest and onResponse. + if (serverLatencyTimerIsRunning.compareAndSet(false, true)) { + serverLatencyTimer.start(); + } } } @Override public void responseReceived() { - if (applicationLatencyTimerIsRunning.compareAndSet(false, true)) { - applicationLatencyTimer.start(); + // When auto flow control is enabled, server latency is measured between afterResponse and + // responseReceived. + // When auto flow control is disabled, server latency is measured between onRequest and + // responseReceived. + // When auto flow control is disabled and application requested multiple responses, server + // latency is measured between afterResponse and responseReceived. + // In all the cases, we want to stop the serverLatencyTimer here. + if (serverLatencyTimerIsRunning.compareAndSet(true, false)) { + totalServerLatency.addAndGet(serverLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); + serverLatencyTimer.reset(); } - if (firstResponsePerOpTimer.isRunning()) { - firstResponsePerOpTimer.stop(); + } + + @Override + public void afterResponse(long applicationLatency) { + if (!flowControlIsDisabled || requestLeft.decrementAndGet() > 0) { + // When auto flow control is enabled, request will never be called, so server latency is + // measured between after the last response is processed and before the next response is + // received. If flow control is disabled but requestLeft is greater than 0, + // also start the timer to count the time between afterResponse and responseReceived. + if (serverLatencyTimerIsRunning.compareAndSet(false, true)) { + serverLatencyTimer.start(); + } } } @@ -180,21 +205,26 @@ public void batchRequestThrottled(long throttledTimeMs) { recorder.putBatchRequestThrottled(throttledTimeMs); } + @Override + public void disableFlowControl() { + flowControlIsDisabled = true; + } + private void recordOperationCompletion(@Nullable Throwable status) { if (!opFinished.compareAndSet(false, true)) { return; } operationTimer.stop(); + long operationLatency = operationTimer.elapsed(TimeUnit.MILLISECONDS); recorder.putRetryCount(attemptCount); - if (applicationLatencyTimerIsRunning.compareAndSet(true, false)) { - applicationLatencyTimer.stop(); - totalApplicationLatency.addAndGet(applicationLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); + if (serverLatencyTimerIsRunning.compareAndSet(true, false)) { + serverLatencyTimer.stop(); + totalServerLatency.addAndGet(serverLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); } - recorder.putApplicationLatencies(totalApplicationLatency.get()); - - recorder.putOperationLatencies(operationTimer.elapsed(TimeUnit.MILLISECONDS)); + recorder.putOperationLatencies(operationLatency); + recorder.putApplicationLatencies(operationLatency - totalServerLatency.get()); if (operationType == OperationType.ServerStreaming && spanName.getMethodName().equals("ReadRows")) { @@ -205,8 +235,14 @@ private void recordOperationCompletion(@Nullable Throwable status) { } private void recordAttemptCompletion(@Nullable Throwable status) { + // If the attempt failed, the time spent in retry should be counted in application latency. + // Stop the stopwatch and decrement requestLeft. + if (serverLatencyTimerIsRunning.compareAndSet(true, false)) { + requestLeft.decrementAndGet(); + totalServerLatency.addAndGet(serverLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); + serverLatencyTimer.reset(); + } recorder.putAttemptLatencies(attemptTimer.elapsed(TimeUnit.MILLISECONDS)); - recorder.record(Util.extractStatus(status), tableId, zone, cluster); } } 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 29c16baafb..271782c2f6 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 @@ -199,9 +199,23 @@ public void setLocations(String zone, String cluster) { } @Override - public void onRequest() { + public void onRequest(int requestCount) { for (BigtableTracer tracer : bigtableTracers) { - tracer.onRequest(); + tracer.onRequest(requestCount); + } + } + + @Override + public void disableFlowControl() { + for (BigtableTracer tracer : bigtableTracers) { + tracer.disableFlowControl(); + } + } + + @Override + public void afterResponse(long applicationLatency) { + for (BigtableTracer tracer : bigtableTracers) { + tracer.afterResponse(applicationLatency); } } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java index b5af9d71b5..f73511dc4c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerStreamingCallable.java @@ -22,7 +22,9 @@ import com.google.api.gax.rpc.ServerStreamingCallable; import com.google.api.gax.rpc.StreamController; import com.google.common.base.Preconditions; +import com.google.common.base.Stopwatch; import io.grpc.Metadata; +import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; /** @@ -55,7 +57,7 @@ public void call( RequestT request, ResponseObserver responseObserver, ApiCallContext context) { final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); // tracer should always be an instance of bigtable tracer - if (RpcViews.isGfeMetricsRegistered() && context.getTracer() instanceof BigtableTracer) { + if (context.getTracer() instanceof BigtableTracer) { HeaderTracerResponseObserver innerObserver = new HeaderTracerResponseObserver<>( responseObserver, (BigtableTracer) context.getTracer(), responseMetadata); @@ -82,13 +84,15 @@ private class HeaderTracerResponseObserver implements ResponseObserve @Override public void onStart(final StreamController controller) { - StreamController tracedController = new TracedStreamController(controller, tracer); + TracedStreamController tracedController = new TracedStreamController(controller, tracer); outerObserver.onStart(tracedController); } @Override public void onResponse(ResponseT response) { + Stopwatch stopwatch = Stopwatch.createStarted(); outerObserver.onResponse(response); + tracer.afterResponse(stopwatch.elapsed(TimeUnit.MILLISECONDS)); } @Override @@ -126,12 +130,13 @@ public void cancel() { @Override public void disableAutoInboundFlowControl() { + tracer.disableFlowControl(); innerController.disableAutoInboundFlowControl(); } @Override public void request(int i) { - tracer.onRequest(); + tracer.onRequest(i); innerController.request(i); } } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java index 6335b433ef..adbb6c84a9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerUnaryCallable.java @@ -54,7 +54,7 @@ public HeaderTracerUnaryCallable(@Nonnull UnaryCallable inn @Override public ApiFuture futureCall(RequestT request, ApiCallContext context) { // tracer should always be an instance of BigtableTracer - if (RpcViews.isGfeMetricsRegistered() && context.getTracer() instanceof BigtableTracer) { + if (context.getTracer() instanceof BigtableTracer) { final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata(); final ApiCallContext contextWithResponseMetadata = responseMetadata.addHandlers(context); HeaderTracerUnaryCallback callback = 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 37484871ce..ec5932d525 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 @@ -54,8 +54,14 @@ import io.grpc.ServerInterceptor; import io.grpc.Status; import io.grpc.StatusRuntimeException; +import io.grpc.stub.ServerCallStreamObserver; import io.grpc.stub.StreamObserver; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.Queue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; import org.junit.Before; @@ -68,6 +74,7 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; +import org.mockito.stubbing.Answer; import org.threeten.bp.Duration; @RunWith(JUnit4.class) @@ -78,44 +85,10 @@ public class BuiltinMetricsTracerTest { private static final String TABLE_ID = "fake-table"; private static final String UNDEFINED = "undefined"; private static final long FAKE_SERVER_TIMING = 50; - private static final long SERVER_LATENCY = 500; + private static final long SERVER_LATENCY = 100; private final AtomicInteger rpcCount = new AtomicInteger(0); - private static final ReadRowsResponse READ_ROWS_RESPONSE_1 = - ReadRowsResponse.newBuilder() - .addChunks( - ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("fake-key-1")) - .setFamilyName(StringValue.of("cf")) - .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) - .setTimestampMicros(1_000) - .setValue(ByteString.copyFromUtf8("value")) - .setCommitRow(true)) - .build(); - private static final ReadRowsResponse READ_ROWS_RESPONSE_2 = - ReadRowsResponse.newBuilder() - .addChunks( - ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("fake-key-2")) - .setFamilyName(StringValue.of("cf")) - .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) - .setTimestampMicros(1_000) - .setValue(ByteString.copyFromUtf8("value")) - .setCommitRow(true)) - .build(); - private static final ReadRowsResponse READ_ROWS_RESPONSE_3 = - ReadRowsResponse.newBuilder() - .addChunks( - ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("fake-key-3")) - .setFamilyName(StringValue.of("cf")) - .setQualifier(BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) - .setTimestampMicros(1_000) - .setValue(ByteString.copyFromUtf8("value")) - .setCommitRow(true)) - .build(); - @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); private BigtableGrpc.BigtableImplBase mockService; @@ -188,13 +161,15 @@ public void tearDown() { } @Test - public void testOperationLatencies() { + public void testOperationLatencies() throws InterruptedException { when(mockFactory.newTracer(any(), any(), any())) - .thenReturn( - new BuiltinMetricsTracer( - OperationType.ServerStreaming, - SpanName.of("Bigtable", "ReadRows"), - statsRecorderWrapper)); + .thenAnswer( + (Answer) + invocationOnMock -> + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); Stopwatch stopwatch = Stopwatch.createStarted(); Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator()); long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); @@ -207,44 +182,38 @@ public void testOperationLatencies() { @Test public void testGfeMetrics() { when(mockFactory.newTracer(any(), any(), any())) - .thenReturn( - new BuiltinMetricsTracer( - OperationType.ServerStreaming, - SpanName.of("Bigtable", "ReadRows"), - statsRecorderWrapper)); + .thenAnswer( + (Answer) + invocationOnMock -> + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID))); verify(statsRecorderWrapper).putGfeLatencies(longValue.capture()); assertThat(longValue.getValue()).isEqualTo(FAKE_SERVER_TIMING); - verify(statsRecorderWrapper).putGfeMissingHeaders(longValue.capture()); - assertThat(longValue.getValue()).isEqualTo(0); + verify(statsRecorderWrapper, times(2)).putGfeMissingHeaders(longValue.capture()); + assertThat(longValue.getValue()).isEqualTo(1); } @Test - public void testReadRowsApplicationLatency() throws Exception { + public void testReadRowsApplicationLatencyWithAutoFlowControl() throws Exception { when(mockFactory.newTracer(any(), any(), any())) - .thenReturn( - new BuiltinMetricsTracer( - OperationType.ServerStreaming, - SpanName.of("Bigtable", "ReadRows"), - statsRecorderWrapper)); - - final long applicationLatency = 1000; + .thenAnswer( + (Answer) + invocationOnMock -> + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); + + final long applicationLatency = 200; final SettableApiFuture future = SettableApiFuture.create(); final AtomicInteger counter = new AtomicInteger(0); - // We want to measure how long application waited before requesting another message after - // the previous message is returned from the server. Using ResponseObserver here so that the - // flow will be - // onResponse() -> sleep -> onRequest() (for the next message) which is exactly what we want to - // measure for - // application latency. - // If we do readRowsCallable().call(Query.create(TABLE_ID)).iterator() and iterate through the - // iterator and sleep in - // between responses, when the call started, the client will pre-fetch the first response, which - // won't be counted - // in application latency. So the test will be flaky and hard to debug. + // For auto flow control, application latency is the time application spent in onResponse. stub.readRowsCallable() .call( Query.create(TABLE_ID), @@ -255,7 +224,7 @@ public void onStart(StreamController streamController) {} @Override public void onResponse(Row row) { try { - counter.incrementAndGet(); + counter.getAndIncrement(); Thread.sleep(applicationLatency); } catch (InterruptedException e) { } @@ -280,12 +249,48 @@ public void onComplete() { assertThat(longValue.getValue()).isLessThan(applicationLatency * (counter.get() + 1)); } + @Test + public void testReadRowsApplicationLatencyWithManualFlowControl() throws Exception { + when(mockFactory.newTracer(any(), any(), any())) + .thenAnswer( + (Answer) + invocationOnMock -> + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); + + final long applicationLatency = 200; + final AtomicInteger counter = new AtomicInteger(0); + + Iterator rows = stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator(); + while (rows.hasNext()) { + counter.getAndIncrement(); + Thread.sleep(applicationLatency); + rows.next(); + } + + verify(statsRecorderWrapper).putApplicationLatencies(longValue.capture()); + + // For manual flow control, the last application latency shouldn't count, because at that point + // the server already sent back all the responses. + assertThat(counter.get()).isGreaterThan(1); + assertThat(longValue.getValue()) + .isAtLeast(applicationLatency * (counter.get() - 1) - SERVER_LATENCY); + assertThat(longValue.getValue()) + .isAtMost(applicationLatency * (counter.get()) - SERVER_LATENCY); + } + @Test public void testRetryCount() { when(mockFactory.newTracer(any(), any(), any())) - .thenReturn( - new BuiltinMetricsTracer( - OperationType.Unary, SpanName.of("Bigtable", "MutateRow"), statsRecorderWrapper)); + .thenAnswer( + (Answer) + invocationOnMock -> + new BuiltinMetricsTracer( + OperationType.ServerStreaming, + SpanName.of("Bigtable", "ReadRows"), + statsRecorderWrapper)); stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); @@ -316,25 +321,77 @@ public void testMutateRowAttempts() { private class FakeService extends BigtableGrpc.BigtableImplBase { + private final AtomicInteger counter = new AtomicInteger(0); + private final Iterator source = new ReadRowsGenerator(); + + class ReadRowsGenerator implements Iterator { + private Queue queue = new LinkedList<>(); + + ReadRowsGenerator() { + for (int i = 0; i < 4; i++) { + // create responses that are 1 MB which is the default grpc buffer size + queue.offer( + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("fake-key-" + i)) + .setFamilyName(StringValue.of("cf")) + .setQualifier( + BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue( + ByteString.copyFromUtf8( + String.join("", Collections.nCopies(1024 * 1024, "A")))) + .setCommitRow(true)) + .build()); + } + } + + @Override + public boolean hasNext() { + return !queue.isEmpty(); + } + + @Override + public ReadRowsResponse next() { + return queue.poll(); + } + } + @Override public void readRows( ReadRowsRequest request, StreamObserver responseObserver) { + final AtomicBoolean done = new AtomicBoolean(); + final ServerCallStreamObserver target = + (ServerCallStreamObserver) responseObserver; try { Thread.sleep(SERVER_LATENCY); } catch (InterruptedException e) { } - responseObserver.onNext(READ_ROWS_RESPONSE_1); - responseObserver.onNext(READ_ROWS_RESPONSE_2); - responseObserver.onNext(READ_ROWS_RESPONSE_3); - responseObserver.onCompleted(); + if (counter.getAndIncrement() == 0) { + target.onError(new StatusRuntimeException(Status.UNAVAILABLE)); + return; + } + + // Only return the next response when the buffer is emptied for testing manual flow control. + // The fake service won't keep calling onNext unless it received an onRequest event from + // the application thread + target.setOnReadyHandler( + () -> { + while (target.isReady() && source.hasNext()) { + target.onNext(source.next()); + } + if (!source.hasNext() && done.compareAndSet(false, true)) { + target.onCompleted(); + } + }); } @Override public void mutateRow( MutateRowRequest request, StreamObserver responseObserver) { - if (rpcCount.get() < 2) { + if (rpcCount.getAndIncrement() < 2) { responseObserver.onError(new StatusRuntimeException(Status.UNAVAILABLE)); - rpcCount.getAndIncrement(); return; } responseObserver.onNext(MutateRowResponse.getDefaultInstance()); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java index d6dbb969f1..d93859bbad 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/HeaderTracerCallableTest.java @@ -56,7 +56,6 @@ import io.grpc.stub.StreamObserver; import io.opencensus.impl.stats.StatsComponentImpl; import io.opencensus.stats.StatsComponent; -import io.opencensus.stats.ViewData; import io.opencensus.tags.TagKey; import io.opencensus.tags.TagValue; import io.opencensus.tags.Tags; @@ -383,24 +382,6 @@ public void testMetricsWithErrorResponse() throws InterruptedException { assertThat(missingCount).isEqualTo(attempts); } - @Test - public void testCallableBypassed() throws InterruptedException { - RpcViews.setGfeMetricsRegistered(false); - stub.readRowsCallable().call(Query.create(TABLE_ID)); - Thread.sleep(WAIT_FOR_METRICS_TIME_MS); - ViewData headerMissingView = - localStats - .getViewManager() - .getView(RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW.getName()); - ViewData latencyView = - localStats.getViewManager().getView(RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW.getName()); - // Verify that the view is registered by it's not collecting metrics - assertThat(headerMissingView).isNotNull(); - assertThat(latencyView).isNotNull(); - assertThat(headerMissingView.getAggregationMap()).isEmpty(); - assertThat(latencyView.getAggregationMap()).isEmpty(); - } - private class FakeService extends BigtableImplBase { private final String defaultTableName = NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID); From fab63cc0a5b2f3678e15b159cc2d3e2268858667 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 24 Jun 2022 12:44:04 -0400 Subject: [PATCH 10/15] remove unused check --- .../bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java | 5 +---- 1 file changed, 1 insertion(+), 4 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 6e1080002a..4ce248d81b 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 @@ -219,10 +219,7 @@ private void recordOperationCompletion(@Nullable Throwable status) { recorder.putRetryCount(attemptCount); - if (serverLatencyTimerIsRunning.compareAndSet(true, false)) { - serverLatencyTimer.stop(); - totalServerLatency.addAndGet(serverLatencyTimer.elapsed(TimeUnit.MILLISECONDS)); - } + // serverLatencyTimer should already be stopped in recordAttemptCompletion recorder.putOperationLatencies(operationLatency); recorder.putApplicationLatencies(operationLatency - totalServerLatency.get()); From fc1f215be7e01044f0f0a64813c61dc0117ed27a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 28 Jun 2022 12:11:25 -0400 Subject: [PATCH 11/15] clean up tests --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 3 +- .../metrics/BuiltinMetricsTracerTest.java | 161 ++++++++++-------- 2 files changed, 93 insertions(+), 71 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 4ce248d81b..582aa262b2 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 @@ -139,7 +139,8 @@ public void attemptFailed(Throwable error, Duration delay) { @Override public void onRequest(int requestCount) { - requestLeft.accumulateAndGet(requestCount, (x, y) -> x + y); + requestLeft.accumulateAndGet( + requestCount, (x, y) -> x + y > Integer.MAX_VALUE ? Integer.MAX_VALUE : x + y); if (flowControlIsDisabled) { // On request is only called when auto flow control is disabled. When auto flow control is // disabled, server latency is measured between onRequest and onResponse. 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 ec5932d525..4a215fafae 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 @@ -86,12 +86,11 @@ public class BuiltinMetricsTracerTest { private static final String UNDEFINED = "undefined"; private static final long FAKE_SERVER_TIMING = 50; private static final long SERVER_LATENCY = 100; - - private final AtomicInteger rpcCount = new AtomicInteger(0); + private static final long APPLICATION_LATENCY = 200; @Rule public final MockitoRule mockitoRule = MockitoJUnit.rule(); - private BigtableGrpc.BigtableImplBase mockService; + private FakeService fakeService; private Server server; private EnhancedBigtableStub stub; @@ -99,8 +98,6 @@ public class BuiltinMetricsTracerTest { @Mock private BuiltinMetricsTracerFactory mockFactory; @Mock private StatsRecorderWrapper statsRecorderWrapper; - @Captor private ArgumentCaptor longValue; - @Captor private ArgumentCaptor intValue; @Captor private ArgumentCaptor status; @Captor private ArgumentCaptor tableId; @Captor private ArgumentCaptor zone; @@ -108,13 +105,11 @@ public class BuiltinMetricsTracerTest { @Before public void setUp() throws Exception { - mockService = new FakeService(); + fakeService = new FakeService(); // Add an interceptor to add server-timing in headers ServerInterceptor trailersInterceptor = new ServerInterceptor() { - private AtomicInteger count = new AtomicInteger(0); - @Override public ServerCall.Listener interceptCall( ServerCall serverCall, @@ -134,7 +129,7 @@ public void sendHeaders(Metadata headers) { } }; - server = FakeServiceBuilder.create(mockService).intercept(trailersInterceptor).start(); + server = FakeServiceBuilder.create(fakeService).intercept(trailersInterceptor).start(); BigtableDataSettings settings = BigtableDataSettings.newBuilderForEmulator(server.getPort()) @@ -161,7 +156,7 @@ public void tearDown() { } @Test - public void testOperationLatencies() throws InterruptedException { + public void testOperationLatencies() { when(mockFactory.newTracer(any(), any(), any())) .thenAnswer( (Answer) @@ -170,13 +165,15 @@ public void testOperationLatencies() throws InterruptedException { OperationType.ServerStreaming, SpanName.of("Bigtable", "ReadRows"), statsRecorderWrapper)); + ArgumentCaptor operationLatency = ArgumentCaptor.forClass(Long.class); + Stopwatch stopwatch = Stopwatch.createStarted(); Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator()); long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS); - verify(statsRecorderWrapper).putOperationLatencies(longValue.capture()); + verify(statsRecorderWrapper).putOperationLatencies(operationLatency.capture()); - assertThat(longValue.getValue()).isIn(Range.closed(SERVER_LATENCY, elapsed)); + assertThat(operationLatency.getValue()).isIn(Range.closed(SERVER_LATENCY, elapsed)); } @Test @@ -189,14 +186,19 @@ public void testGfeMetrics() { OperationType.ServerStreaming, SpanName.of("Bigtable", "ReadRows"), statsRecorderWrapper)); + ArgumentCaptor gfeLatency = ArgumentCaptor.forClass(Long.class); + ArgumentCaptor gfeMissingHeaders = ArgumentCaptor.forClass(Long.class); Lists.newArrayList(stub.readRowsCallable().call(Query.create(TABLE_ID))); - verify(statsRecorderWrapper).putGfeLatencies(longValue.capture()); - assertThat(longValue.getValue()).isEqualTo(FAKE_SERVER_TIMING); + // The request was retried and gfe latency is only recorded in the retry attempt + verify(statsRecorderWrapper).putGfeLatencies(gfeLatency.capture()); + assertThat(gfeLatency.getValue()).isEqualTo(FAKE_SERVER_TIMING); - verify(statsRecorderWrapper, times(2)).putGfeMissingHeaders(longValue.capture()); - assertThat(longValue.getValue()).isEqualTo(1); + // The first time the request was retried, it'll increment missing header counter + verify(statsRecorderWrapper, times(fakeService.getRetryCounter().get())) + .putGfeMissingHeaders(gfeMissingHeaders.capture()); + assertThat(gfeMissingHeaders.getValue()).isEqualTo(1); } @Test @@ -210,7 +212,9 @@ public void testReadRowsApplicationLatencyWithAutoFlowControl() throws Exception SpanName.of("Bigtable", "ReadRows"), statsRecorderWrapper)); - final long applicationLatency = 200; + ArgumentCaptor applicationLatency = ArgumentCaptor.forClass(Long.class); + ArgumentCaptor operationLatency = ArgumentCaptor.forClass(Long.class); + final SettableApiFuture future = SettableApiFuture.create(); final AtomicInteger counter = new AtomicInteger(0); // For auto flow control, application latency is the time application spent in onResponse. @@ -225,7 +229,7 @@ public void onStart(StreamController streamController) {} public void onResponse(Row row) { try { counter.getAndIncrement(); - Thread.sleep(applicationLatency); + Thread.sleep(APPLICATION_LATENCY); } catch (InterruptedException e) { } } @@ -242,11 +246,13 @@ public void onComplete() { }); future.get(); - verify(statsRecorderWrapper).putApplicationLatencies(longValue.capture()); + verify(statsRecorderWrapper).putApplicationLatencies(applicationLatency.capture()); + verify(statsRecorderWrapper).putOperationLatencies(operationLatency.capture()); - assertThat(counter.get()).isGreaterThan(0); - assertThat(longValue.getValue()).isAtLeast(applicationLatency * counter.get()); - assertThat(longValue.getValue()).isLessThan(applicationLatency * (counter.get() + 1)); + assertThat(counter.get()).isEqualTo(fakeService.getResponseCounter().get()); + assertThat(applicationLatency.getValue()).isAtLeast(APPLICATION_LATENCY * counter.get()); + assertThat(applicationLatency.getValue()) + .isAtMost(operationLatency.getValue() - SERVER_LATENCY); } @Test @@ -260,25 +266,28 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti SpanName.of("Bigtable", "ReadRows"), statsRecorderWrapper)); - final long applicationLatency = 200; + ArgumentCaptor applicationLatency = ArgumentCaptor.forClass(Long.class); + ArgumentCaptor operationLatency = ArgumentCaptor.forClass(Long.class); final AtomicInteger counter = new AtomicInteger(0); Iterator rows = stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator(); + while (rows.hasNext()) { counter.getAndIncrement(); - Thread.sleep(applicationLatency); + Thread.sleep(APPLICATION_LATENCY); rows.next(); } - verify(statsRecorderWrapper).putApplicationLatencies(longValue.capture()); + verify(statsRecorderWrapper).putApplicationLatencies(applicationLatency.capture()); + verify(statsRecorderWrapper).putOperationLatencies(operationLatency.capture()); // For manual flow control, the last application latency shouldn't count, because at that point // the server already sent back all the responses. - assertThat(counter.get()).isGreaterThan(1); - assertThat(longValue.getValue()) - .isAtLeast(applicationLatency * (counter.get() - 1) - SERVER_LATENCY); - assertThat(longValue.getValue()) - .isAtMost(applicationLatency * (counter.get()) - SERVER_LATENCY); + assertThat(counter.get()).isEqualTo(fakeService.getResponseCounter().get()); + assertThat(applicationLatency.getValue()) + .isAtLeast(APPLICATION_LATENCY * (counter.get() - 1) - SERVER_LATENCY); + assertThat(applicationLatency.getValue()) + .isAtMost(operationLatency.getValue() - SERVER_LATENCY); } @Test @@ -292,12 +301,14 @@ public void testRetryCount() { SpanName.of("Bigtable", "ReadRows"), statsRecorderWrapper)); + ArgumentCaptor retryCount = ArgumentCaptor.forClass(Integer.class); + stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); - verify(statsRecorderWrapper).putRetryCount(intValue.capture()); + verify(statsRecorderWrapper).putRetryCount(retryCount.capture()); - assertThat(intValue.getValue()).isEqualTo(3); + assertThat(retryCount.getValue()).isEqualTo(fakeService.getRetryCounter().get()); } @Test @@ -305,58 +316,59 @@ public void testMutateRowAttempts() { when(mockFactory.newTracer(any(), any(), any())) .thenReturn( new BuiltinMetricsTracer( - OperationType.Unary, SpanName.of("Bigtable", "MutateRow"), statsRecorderWrapper)); + OperationType.Unary, SpanName.of("Bigtable", gi"MutateRow"), statsRecorderWrapper)); stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); // record will get called 4 times, 3 times for attempts and 1 for recording operation level // metrics. - verify(statsRecorderWrapper, times(4)) + verify(statsRecorderWrapper, times(fakeService.getRetryCounter().get() + 1)) .record(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); assertThat(zone.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); assertThat(cluster.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); assertThat(status.getAllValues()).containsExactly("UNAVAILABLE", "UNAVAILABLE", "OK", "OK"); } - private class FakeService extends BigtableGrpc.BigtableImplBase { - - private final AtomicInteger counter = new AtomicInteger(0); - private final Iterator source = new ReadRowsGenerator(); - - class ReadRowsGenerator implements Iterator { - private Queue queue = new LinkedList<>(); - - ReadRowsGenerator() { - for (int i = 0; i < 4; i++) { - // create responses that are 1 MB which is the default grpc buffer size - queue.offer( - ReadRowsResponse.newBuilder() - .addChunks( - ReadRowsResponse.CellChunk.newBuilder() - .setRowKey(ByteString.copyFromUtf8("fake-key-" + i)) - .setFamilyName(StringValue.of("cf")) - .setQualifier( - BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) - .setTimestampMicros(1_000) - .setValue( - ByteString.copyFromUtf8( - String.join("", Collections.nCopies(1024 * 1024, "A")))) - .setCommitRow(true)) - .build()); - } + private class ReadRowsGenerator implements Iterator { + private Queue queue = new LinkedList<>(); + + ReadRowsGenerator() { + for (int i = 0; i < 4; i++) { + // create responses that are 1 MB which is the default grpc buffer size + queue.offer( + ReadRowsResponse.newBuilder() + .addChunks( + ReadRowsResponse.CellChunk.newBuilder() + .setRowKey(ByteString.copyFromUtf8("fake-key-" + i)) + .setFamilyName(StringValue.of("cf")) + .setQualifier( + BytesValue.newBuilder().setValue(ByteString.copyFromUtf8("q"))) + .setTimestampMicros(1_000) + .setValue( + ByteString.copyFromUtf8( + String.join("", Collections.nCopies(1024 * 1024, "A")))) + .setCommitRow(true)) + .build()); } + } - @Override - public boolean hasNext() { - return !queue.isEmpty(); - } + @Override + public boolean hasNext() { + return !queue.isEmpty(); + } - @Override - public ReadRowsResponse next() { - return queue.poll(); - } + @Override + public ReadRowsResponse next() { + return queue.poll(); } + } + + private class FakeService extends BigtableGrpc.BigtableImplBase { + + private final AtomicInteger retryCounter = new AtomicInteger(0); + private final AtomicInteger responseCounter = new AtomicInteger(0); + private final Iterator source = new ReadRowsGenerator(); @Override public void readRows( @@ -368,7 +380,7 @@ public void readRows( Thread.sleep(SERVER_LATENCY); } catch (InterruptedException e) { } - if (counter.getAndIncrement() == 0) { + if (retryCounter.getAndIncrement() == 0) { target.onError(new StatusRuntimeException(Status.UNAVAILABLE)); return; } @@ -379,6 +391,7 @@ public void readRows( target.setOnReadyHandler( () -> { while (target.isReady() && source.hasNext()) { + responseCounter.getAndIncrement(); target.onNext(source.next()); } if (!source.hasNext() && done.compareAndSet(false, true)) { @@ -390,12 +403,20 @@ public void readRows( @Override public void mutateRow( MutateRowRequest request, StreamObserver responseObserver) { - if (rpcCount.getAndIncrement() < 2) { + if (retryCounter.getAndIncrement() < 2) { responseObserver.onError(new StatusRuntimeException(Status.UNAVAILABLE)); return; } responseObserver.onNext(MutateRowResponse.getDefaultInstance()); responseObserver.onCompleted(); } + + public AtomicInteger getRetryCounter() { + return retryCounter; + } + + public AtomicInteger getResponseCounter() { + return responseCounter; + } } } From bdada3fe05ccf7f14e1dfb0efcea76f6297eb6fd Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 28 Jun 2022 13:30:16 -0400 Subject: [PATCH 12/15] fix typo --- .../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 4a215fafae..d05687b4ad 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 @@ -316,7 +316,7 @@ public void testMutateRowAttempts() { when(mockFactory.newTracer(any(), any(), any())) .thenReturn( new BuiltinMetricsTracer( - OperationType.Unary, SpanName.of("Bigtable", gi"MutateRow"), statsRecorderWrapper)); + OperationType.Unary, SpanName.of("Bigtable", "MutateRow"), statsRecorderWrapper)); stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); From 6198018b6ee4d7b8c585114be68f5a884a4e2e3f Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 28 Jun 2022 14:23:51 -0400 Subject: [PATCH 13/15] update test --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 4 +-- .../metrics/BuiltinMetricsTracerTest.java | 29 +++++-------------- 2 files changed, 10 insertions(+), 23 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 582aa262b2..308f652986 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 @@ -21,6 +21,7 @@ import com.google.cloud.bigtable.stats.StatsRecorderWrapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; +import com.google.common.math.IntMath; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -139,8 +140,7 @@ public void attemptFailed(Throwable error, Duration delay) { @Override public void onRequest(int requestCount) { - requestLeft.accumulateAndGet( - requestCount, (x, y) -> x + y > Integer.MAX_VALUE ? Integer.MAX_VALUE : x + y); + requestLeft.accumulateAndGet(requestCount, IntMath::saturatedAdd); if (flowControlIsDisabled) { // On request is only called when auto flow control is disabled. When auto flow control is // disabled, server latency is measured between onRequest and onResponse. 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 d05687b4ad..20d07d81ff 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 @@ -56,10 +56,10 @@ import io.grpc.StatusRuntimeException; import io.grpc.stub.ServerCallStreamObserver; import io.grpc.stub.StreamObserver; +import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; -import java.util.LinkedList; -import java.util.Queue; +import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -330,13 +330,12 @@ public void testMutateRowAttempts() { assertThat(status.getAllValues()).containsExactly("UNAVAILABLE", "UNAVAILABLE", "OK", "OK"); } - private class ReadRowsGenerator implements Iterator { - private Queue queue = new LinkedList<>(); + private static class FakeService extends BigtableGrpc.BigtableImplBase { - ReadRowsGenerator() { + static List createFakeResponse() { + List responses = new ArrayList<>(); for (int i = 0; i < 4; i++) { - // create responses that are 1 MB which is the default grpc buffer size - queue.offer( + responses.add( ReadRowsResponse.newBuilder() .addChunks( ReadRowsResponse.CellChunk.newBuilder() @@ -351,24 +350,12 @@ private class ReadRowsGenerator implements Iterator { .setCommitRow(true)) .build()); } + return responses; } - @Override - public boolean hasNext() { - return !queue.isEmpty(); - } - - @Override - public ReadRowsResponse next() { - return queue.poll(); - } - } - - private class FakeService extends BigtableGrpc.BigtableImplBase { - private final AtomicInteger retryCounter = new AtomicInteger(0); private final AtomicInteger responseCounter = new AtomicInteger(0); - private final Iterator source = new ReadRowsGenerator(); + private final Iterator source = createFakeResponse().listIterator(); @Override public void readRows( From ee755710e5c5ad05a8e5926c3ebd8450e54373de Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 28 Jun 2022 15:57:47 -0400 Subject: [PATCH 14/15] fix flaky test --- .../v2/stub/metrics/BuiltinMetricsTracerTest.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 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 20d07d81ff..d884bec402 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 @@ -18,6 +18,7 @@ import static com.google.api.gax.tracing.ApiTracerFactory.OperationType; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -321,9 +322,13 @@ public void testMutateRowAttempts() { stub.mutateRowCallable() .call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value")); - // record will get called 4 times, 3 times for attempts and 1 for recording operation level - // metrics. - verify(statsRecorderWrapper, times(fakeService.getRetryCounter().get() + 1)) + // record() will get called 4 times, 3 times for attempts and 1 for recording operation level + // metrics. Also set a timeout to reduce flakiness of this test. BasicRetryingFuture will set + // attempt succeeded and set the response which will call complete() in AbstractFuture which + // calls releaseWaiters(). onOperationComplete() is called in TracerFinisher which will be + // called after the mutateRow call is returned. So there's a race between when the call returns + // and when the record() is called in onOperationCompletion(). + verify(statsRecorderWrapper, timeout(10).times(fakeService.getRetryCounter().get() + 1)) .record(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); assertThat(zone.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); assertThat(cluster.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); From eceb78e6a28ceecad2923f1eedee897652b88acd Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 28 Jun 2022 16:24:39 -0400 Subject: [PATCH 15/15] fix retry count --- .../v2/stub/metrics/BuiltinMetricsTracer.java | 2 +- .../metrics/BuiltinMetricsTracerTest.java | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 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 308f652986..2148c674e3 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 @@ -218,7 +218,7 @@ private void recordOperationCompletion(@Nullable Throwable status) { operationTimer.stop(); long operationLatency = operationTimer.elapsed(TimeUnit.MILLISECONDS); - recorder.putRetryCount(attemptCount); + recorder.putRetryCount(attemptCount - 1); // serverLatencyTimer should already be stopped in recordAttemptCompletion recorder.putOperationLatencies(operationLatency); 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 d884bec402..a48df92254 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 @@ -197,7 +197,7 @@ public void testGfeMetrics() { assertThat(gfeLatency.getValue()).isEqualTo(FAKE_SERVER_TIMING); // The first time the request was retried, it'll increment missing header counter - verify(statsRecorderWrapper, times(fakeService.getRetryCounter().get())) + verify(statsRecorderWrapper, times(fakeService.getAttemptCounter().get())) .putGfeMissingHeaders(gfeMissingHeaders.capture()); assertThat(gfeMissingHeaders.getValue()).isEqualTo(1); } @@ -269,12 +269,12 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti ArgumentCaptor applicationLatency = ArgumentCaptor.forClass(Long.class); ArgumentCaptor operationLatency = ArgumentCaptor.forClass(Long.class); - final AtomicInteger counter = new AtomicInteger(0); + int counter = 0; Iterator rows = stub.readRowsCallable().call(Query.create(TABLE_ID)).iterator(); while (rows.hasNext()) { - counter.getAndIncrement(); + counter++; Thread.sleep(APPLICATION_LATENCY); rows.next(); } @@ -284,9 +284,9 @@ public void testReadRowsApplicationLatencyWithManualFlowControl() throws Excepti // For manual flow control, the last application latency shouldn't count, because at that point // the server already sent back all the responses. - assertThat(counter.get()).isEqualTo(fakeService.getResponseCounter().get()); + assertThat(counter).isEqualTo(fakeService.getResponseCounter().get()); assertThat(applicationLatency.getValue()) - .isAtLeast(APPLICATION_LATENCY * (counter.get() - 1) - SERVER_LATENCY); + .isAtLeast(APPLICATION_LATENCY * (counter - 1) - SERVER_LATENCY); assertThat(applicationLatency.getValue()) .isAtMost(operationLatency.getValue() - SERVER_LATENCY); } @@ -309,7 +309,7 @@ public void testRetryCount() { verify(statsRecorderWrapper).putRetryCount(retryCount.capture()); - assertThat(retryCount.getValue()).isEqualTo(fakeService.getRetryCounter().get()); + assertThat(retryCount.getValue()).isEqualTo(fakeService.getAttemptCounter().get() - 1); } @Test @@ -328,7 +328,7 @@ public void testMutateRowAttempts() { // calls releaseWaiters(). onOperationComplete() is called in TracerFinisher which will be // called after the mutateRow call is returned. So there's a race between when the call returns // and when the record() is called in onOperationCompletion(). - verify(statsRecorderWrapper, timeout(10).times(fakeService.getRetryCounter().get() + 1)) + verify(statsRecorderWrapper, timeout(10).times(fakeService.getAttemptCounter().get() + 1)) .record(status.capture(), tableId.capture(), zone.capture(), cluster.capture()); assertThat(zone.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); assertThat(cluster.getAllValues()).containsExactly(UNDEFINED, UNDEFINED, UNDEFINED, UNDEFINED); @@ -358,7 +358,7 @@ static List createFakeResponse() { return responses; } - private final AtomicInteger retryCounter = new AtomicInteger(0); + private final AtomicInteger attemptCounter = new AtomicInteger(0); private final AtomicInteger responseCounter = new AtomicInteger(0); private final Iterator source = createFakeResponse().listIterator(); @@ -372,7 +372,7 @@ public void readRows( Thread.sleep(SERVER_LATENCY); } catch (InterruptedException e) { } - if (retryCounter.getAndIncrement() == 0) { + if (attemptCounter.getAndIncrement() == 0) { target.onError(new StatusRuntimeException(Status.UNAVAILABLE)); return; } @@ -395,7 +395,7 @@ public void readRows( @Override public void mutateRow( MutateRowRequest request, StreamObserver responseObserver) { - if (retryCounter.getAndIncrement() < 2) { + if (attemptCounter.getAndIncrement() < 2) { responseObserver.onError(new StatusRuntimeException(Status.UNAVAILABLE)); return; } @@ -403,8 +403,8 @@ public void mutateRow( responseObserver.onCompleted(); } - public AtomicInteger getRetryCounter() { - return retryCounter; + public AtomicInteger getAttemptCounter() { + return attemptCounter; } public AtomicInteger getResponseCounter() {