From bba41837febc10e9507afc7117e2e4ec2d15fb11 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 19 Nov 2024 00:24:22 -0500 Subject: [PATCH] fix: make client side metrics tag in sync with server (#2401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Rollback plan is reviewed and LGTMed - [ ] All new data plane features have a completed end to end testing plan Fixes # ☕️ If you write sample code, please follow the [samples format]( https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md). --- .../data/v2/stub/metrics/BuiltinMetricsTracer.java | 4 ++-- .../cloud/bigtable/data/v2/stub/metrics/Util.java | 14 ++++++++++---- .../data/v2/it/StreamingMetricsMetadataIT.java | 4 ++-- .../data/v2/it/UnaryMetricsMetadataIT.java | 4 ++-- .../v2/stub/metrics/BuiltinMetricsTracerTest.java | 12 ++++++------ 5 files changed, 22 insertions(+), 16 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 e639ea5627..4683ff9c8e 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 @@ -79,9 +79,9 @@ class BuiltinMetricsTracer extends BigtableTracer { private final AtomicInteger requestLeft = new AtomicInteger(0); // Monitored resource labels - private String tableId = "unspecified"; + private String tableId = ""; private String zone = "global"; - private String cluster = "unspecified"; + private String cluster = ""; private final AtomicLong totalClientBlockingTime = new AtomicLong(0); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java index 4c3fd7a42d..590917c814 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 @@ -24,8 +24,10 @@ import com.google.api.gax.rpc.StatusCode.Code; import com.google.bigtable.v2.AuthorizedViewName; import com.google.bigtable.v2.CheckAndMutateRowRequest; +import com.google.bigtable.v2.GenerateInitialChangeStreamPartitionsRequest; import com.google.bigtable.v2.MutateRowRequest; import com.google.bigtable.v2.MutateRowsRequest; +import com.google.bigtable.v2.ReadChangeStreamRequest; import com.google.bigtable.v2.ReadModifyWriteRowRequest; import com.google.bigtable.v2.ReadRowsRequest; import com.google.bigtable.v2.ResponseParams; @@ -127,14 +129,18 @@ static String extractTableId(Object request) { } else if (request instanceof ReadModifyWriteRowRequest) { tableName = ((ReadModifyWriteRowRequest) request).getTableName(); authorizedViewName = ((ReadModifyWriteRowRequest) request).getAuthorizedViewName(); + } else if (request instanceof GenerateInitialChangeStreamPartitionsRequest) { + tableName = ((GenerateInitialChangeStreamPartitionsRequest) request).getTableName(); + } else if (request instanceof ReadChangeStreamRequest) { + tableName = ((ReadChangeStreamRequest) request).getTableName(); } - if (tableName == null && authorizedViewName == null) return "undefined"; - if (tableName.isEmpty() && authorizedViewName.isEmpty()) return "undefined"; - if (!tableName.isEmpty()) { + if (tableName != null && !tableName.isEmpty()) { return TableName.parse(tableName).getTable(); - } else { + } + if (authorizedViewName != null && !authorizedViewName.isEmpty()) { return AuthorizedViewName.parse(authorizedViewName).getTable(); } + return ""; } /** diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/StreamingMetricsMetadataIT.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/StreamingMetricsMetadataIT.java index 84ab24f1c8..11da6a6c15 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/StreamingMetricsMetadataIT.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/StreamingMetricsMetadataIT.java @@ -167,9 +167,9 @@ public void testFailure() { assertThat(pointData) .comparingElementsUsing(POINT_DATA_CLUSTER_ID_CONTAINS) - .contains("unspecified"); + .contains(""); assertThat(pointData).comparingElementsUsing(POINT_DATA_ZONE_ID_CONTAINS).contains("global"); - assertThat(clusterAttributes).contains("unspecified"); + assertThat(clusterAttributes).contains(""); assertThat(zoneAttributes).contains("global"); } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/UnaryMetricsMetadataIT.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/UnaryMetricsMetadataIT.java index 42adb8ea6e..a6e4f9e88b 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/UnaryMetricsMetadataIT.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/it/UnaryMetricsMetadataIT.java @@ -182,7 +182,7 @@ public void testFailure() throws Exception { assertThat(pointData) .comparingElementsUsing(POINT_DATA_CLUSTER_ID_CONTAINS) - .contains("unspecified"); + .contains(""); assertThat(pointData).comparingElementsUsing(POINT_DATA_ZONE_ID_CONTAINS).contains("global"); List clusterAttributes = pointData.stream() @@ -193,7 +193,7 @@ public void testFailure() throws Exception { .map(pd -> pd.getAttributes().get(BuiltinMetricsConstants.ZONE_ID_KEY)) .collect(Collectors.toList()); - assertThat(clusterAttributes).contains("unspecified"); + assertThat(clusterAttributes).contains(""); assertThat(zoneAttributes).contains("global"); } } 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 6842f2c88c..c2b2d37af6 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 @@ -395,7 +395,7 @@ public void testGfeMetrics() { .put(STATUS_KEY, "UNAVAILABLE") .put(TABLE_ID_KEY, TABLE) .put(ZONE_ID_KEY, "global") - .put(CLUSTER_ID_KEY, "unspecified") + .put(CLUSTER_ID_KEY, "") .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .build(); @@ -549,7 +549,7 @@ public void testMutateRowAttemptsTagValues() { .put(STATUS_KEY, "UNAVAILABLE") .put(TABLE_ID_KEY, TABLE) .put(ZONE_ID_KEY, "global") - .put(CLUSTER_ID_KEY, "unspecified") + .put(CLUSTER_ID_KEY, "") .put(METHOD_KEY, "Bigtable.MutateRow") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) @@ -619,7 +619,7 @@ public void testMutateRowsRpcError() { .put(STATUS_KEY, "NOT_FOUND") .put(TABLE_ID_KEY, BAD_TABLE_ID) .put(ZONE_ID_KEY, "global") - .put(CLUSTER_ID_KEY, "unspecified") + .put(CLUSTER_ID_KEY, "") .put(METHOD_KEY, "Bigtable.MutateRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, false) @@ -640,7 +640,7 @@ public void testReadRowsAttemptsTagValues() { .put(STATUS_KEY, "UNAVAILABLE") .put(TABLE_ID_KEY, TABLE) .put(ZONE_ID_KEY, "global") - .put(CLUSTER_ID_KEY, "unspecified") + .put(CLUSTER_ID_KEY, "") .put(METHOD_KEY, "Bigtable.ReadRows") .put(CLIENT_NAME_KEY, CLIENT_NAME) .put(STREAMING_KEY, true) @@ -751,7 +751,7 @@ public void testPermanentFailure() { .toBuilder() .put(STATUS_KEY, "NOT_FOUND") .put(TABLE_ID_KEY, BAD_TABLE_ID) - .put(CLUSTER_ID_KEY, "unspecified") + .put(CLUSTER_ID_KEY, "") .put(ZONE_ID_KEY, "global") .put(STREAMING_KEY, true) .put(METHOD_KEY, "Bigtable.ReadRows") @@ -776,7 +776,7 @@ public void testRemainingDeadline() { .put(TABLE_ID_KEY, TABLE) .put(METHOD_KEY, "Bigtable.ReadRows") .put(ZONE_ID_KEY, "global") - .put(CLUSTER_ID_KEY, "unspecified") + .put(CLUSTER_ID_KEY, "") .put(STREAMING_KEY, true) .put(CLIENT_NAME_KEY, CLIENT_NAME) .build();