Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Support AuthorizedView in bigtable data client #2177

Merged
merged 3 commits into from
Mar 27, 2024
Merged

feat: Support AuthorizedView in bigtable data client #2177

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
512818e
feat: Support AuthorizedView in bigtable data client
Mar 20, 2024
c0d05d8
Fix extractTableId() and add a unit test for it
Mar 27, 2024
4fd09d6
Mark SampleRowKeysRequest#fromProto as @InternalApi
Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix extractTableId() and add a unit test for it 
Change-Id: Icc172b2b6d369ef8ce2b77a8e69c37af6e9aa3d7
  • Loading branch information
Lixia Chen committed Mar 27, 2024
commit c0d05d845b20829691868bc66d82b68751162e8a
5 changes: 3 additions & 2 deletions google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,9 @@ static String extractTableId(Object request) {
tableName = ((ReadModifyWriteRowRequest) request).getTableName();
authorizedViewName = ((ReadModifyWriteRowRequest) request).getAuthorizedViewName();
}
if (tableName == null) return "undefined";
if (tableName.isEmpty()) {
if (tableName == null && authorizedViewName == null) return "undefined";
if (tableName.isEmpty() && authorizedViewName.isEmpty()) return "undefined";
if (!tableName.isEmpty()) {
return TableName.parse(tableName).getTable();
} else {
return AuthorizedViewName.parse(authorizedViewName).getTable();
Expand Down
34 changes: 33 additions & 1 deletion ...rc/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.google.bigtable.v2.ResponseParams;
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
import com.google.cloud.bigtable.data.v2.FakeServiceBuilder;
import com.google.cloud.bigtable.data.v2.models.AuthorizedViewId;
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;
Expand Down Expand Up @@ -105,7 +106,7 @@ public class BuiltinMetricsTracerTest {
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 AUTHORIZED_VIEW_ID = "fake-authorized-view";
private static final String BAD_TABLE_ID = "non-exist-table";
private static final String ZONE = "us-west-1";
private static final String CLUSTER = "cluster-0";
Expand Down Expand Up @@ -272,6 +273,37 @@ public void testReadRowsOperationLatencies() {
assertThat(cluster.getAllValues()).containsExactly(CLUSTER);
}

@Test
public void testReadRowsOperationLatenciesOnAuthorizedView() {
when(mockFactory.newTracer(any(), any(), any()))
.thenAnswer(
(Answer<BuiltinMetricsTracer>)
invocationOnMock ->
new BuiltinMetricsTracer(
OperationType.ServerStreaming,
SpanName.of("Bigtable", "ReadRows"),
statsRecorderWrapper));
ArgumentCaptor<Long> operationLatency = ArgumentCaptor.forClass(Long.class);

Stopwatch stopwatch = Stopwatch.createStarted();
Lists.newArrayList(
stub.readRowsCallable()
.call(Query.create(AuthorizedViewId.of(TABLE_ID, AUTHORIZED_VIEW_ID)))
.iterator());
long elapsed = stopwatch.elapsed(TimeUnit.MILLISECONDS);

verify(statsRecorderWrapper).putOperationLatencies(operationLatency.capture());
// verify record operation is only called once
verify(statsRecorderWrapper)
.recordOperation(status.capture(), tableId.capture(), zone.capture(), cluster.capture());

assertThat(operationLatency.getValue()).isIn(Range.closed(SERVER_LATENCY, elapsed));
assertThat(status.getAllValues()).containsExactly("OK");
assertThat(tableId.getAllValues()).containsExactly(TABLE_ID);
assertThat(zone.getAllValues()).containsExactly(ZONE);
assertThat(cluster.getAllValues()).containsExactly(CLUSTER);
}

@Test
public void testGfeMetrics() {
when(mockFactory.newTracer(any(), any(), any()))
Expand Down