Skip to content

Commit 571722e

Browse files
committed
update based on comments
1 parent 5ed0ebe commit 571722e

File tree

7 files changed

+79
-44
lines changed

7 files changed

+79
-44
lines changed

google-cloud-bigtable-stats/pom.xml

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
<artifactId>guava</artifactId>
5252
</dependency>
5353

54-
5554
<dependency>
5655
<groupId>com.google.truth.extensions</groupId>
5756
<artifactId>truth-proto-extension</artifactId>
@@ -67,11 +66,6 @@
6766
<artifactId>junit</artifactId>
6867
<scope>test</scope>
6968
</dependency>
70-
<dependency>
71-
<groupId>io.grpc</groupId>
72-
<artifactId>grpc-api</artifactId>
73-
<scope>test</scope>
74-
</dependency>
7569
</dependencies>
7670

7771
<build>
@@ -86,7 +80,7 @@
8680
<goal>shade</goal>
8781
</goals>
8882
<configuration>
89-
<shadedArtifactAttached>false</shadedArtifactAttached>
83+
<shadedArtifactAttached>true</shadedArtifactAttached>
9084
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
9185
<artifactSet>
9286
<includes>

google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinMeasureConstants.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,20 @@
2121

2222
/** Built-in metrics that will be readable under bigtable.googleapis.com/client namespace */
2323
class BuiltinMeasureConstants {
24-
// TagKeys
24+
// Monitored resource TagKeys
2525
static final TagKey PROJECT_ID = TagKey.create("project_id");
2626
static final TagKey INSTANCE_ID = TagKey.create("instance_id");
27+
static final TagKey CLUSTER = TagKey.create("cluster");
28+
static final TagKey TABLE = TagKey.create("table");
29+
static final TagKey ZONE = TagKey.create("zone");
30+
static final TagKey CLIENT_ID = TagKey.create("client_id");
31+
32+
// Metrics TagKeys
2733
static final TagKey APP_PROFILE = TagKey.create("app_profile");
2834
static final TagKey METHOD = TagKey.create("method");
2935
static final TagKey STREAMING = TagKey.create("streaming");
3036
static final TagKey STATUS = TagKey.create("status");
3137
static final TagKey CLIENT_NAME = TagKey.create("client_name");
32-
static final TagKey CLIENT_ID = TagKey.create("client_id");
33-
34-
// Monitored resource TagKeys
35-
static final TagKey TABLE = TagKey.create("table");
36-
static final TagKey CLUSTER = TagKey.create("cluster");
37-
static final TagKey ZONE = TagKey.create("zone");
3838

3939
// Units
4040
private static final String COUNT = "1";

google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/BuiltinViews.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.google.cloud.bigtable.stats;
1717

1818
import com.google.api.core.InternalApi;
19+
import com.google.common.annotations.VisibleForTesting;
1920
import com.google.common.collect.ImmutableSet;
2021
import io.opencensus.stats.View;
2122
import io.opencensus.stats.ViewManager;
@@ -29,7 +30,8 @@ public BuiltinViews(StatsWrapper wrapper) {
2930
this.statsWrapper = wrapper;
3031
}
3132

32-
private static final ImmutableSet<View> BIGTABLE_BUILTIN_VIEWS =
33+
@VisibleForTesting
34+
static final ImmutableSet<View> BIGTABLE_BUILTIN_VIEWS =
3335
ImmutableSet.of(
3436
BuiltinViewConstants.OPERATION_LATENCIES_VIEW,
3537
BuiltinViewConstants.ATTEMPT_LATENCIES_VIEW,

google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ private StatsWrapper(ViewManager viewManager, StatsRecorder statsRecorder) {
3838
this.statsRecorder = statsRecorder;
3939
}
4040

41-
public static StatsWrapper get() {
41+
public static StatsWrapper create() {
4242
return new StatsWrapper(Stats.getViewManager(), Stats.getStatsRecorder());
4343
}
4444

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2022 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.bigtable.stats;
17+
18+
import static com.google.common.truth.Truth.assertWithMessage;
19+
20+
import io.opencensus.stats.View;
21+
import org.junit.Test;
22+
23+
public class BuiltinViewConstantsTest {
24+
@Test
25+
public void testBasicTagsExistForAllViews() {
26+
for (View v : BuiltinViews.BIGTABLE_BUILTIN_VIEWS) {
27+
assertWithMessage(v.getName() + " should have all basic tags")
28+
.that(v.getColumns())
29+
.containsAtLeast(
30+
BuiltinMeasureConstants.PROJECT_ID,
31+
BuiltinMeasureConstants.INSTANCE_ID,
32+
BuiltinMeasureConstants.APP_PROFILE,
33+
BuiltinMeasureConstants.METHOD,
34+
BuiltinMeasureConstants.ZONE,
35+
BuiltinMeasureConstants.CLUSTER,
36+
BuiltinMeasureConstants.TABLE);
37+
}
38+
}
39+
}

google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import com.google.api.gax.tracing.ApiTracerFactory;
2121
import com.google.api.gax.tracing.SpanName;
2222
import com.google.common.collect.ImmutableMap;
23-
import io.grpc.Status;
2423
import io.opencensus.stats.AggregationData;
2524
import io.opencensus.stats.View;
2625
import io.opencensus.stats.ViewData;
@@ -54,7 +53,7 @@ public void setup() {
5453

5554
@Test
5655
public void testStreamingOperation() throws InterruptedException {
57-
StatsRecorderWrapper tracer =
56+
StatsRecorderWrapper recorderWrapper =
5857
new StatsRecorderWrapper(
5958
ApiTracerFactory.OperationType.ServerStreaming,
6059
SpanName.of("Bigtable", "ReadRows"),
@@ -73,16 +72,16 @@ public void testStreamingOperation() throws InterruptedException {
7372
long throttlingLatency = 50;
7473
long firstResponseLatency = 90;
7574

76-
tracer.putOperationLatencies(operationLatency);
77-
tracer.putRetryCount(attemptCount);
78-
tracer.putAttemptLatencies(attemptLatency);
79-
tracer.putApplicationLatencies(applicationLatency);
80-
tracer.putGfeLatencies(serverLatency);
81-
tracer.putGfeMissingHeaders(connectivityErrorCount);
82-
tracer.putFirstResponseLatencies(firstResponseLatency);
83-
tracer.putBatchRequestThrottled(throttlingLatency);
75+
recorderWrapper.putOperationLatencies(operationLatency);
76+
recorderWrapper.putRetryCount(attemptCount);
77+
recorderWrapper.putAttemptLatencies(attemptLatency);
78+
recorderWrapper.putApplicationLatencies(applicationLatency);
79+
recorderWrapper.putGfeLatencies(serverLatency);
80+
recorderWrapper.putGfeMissingHeaders(connectivityErrorCount);
81+
recorderWrapper.putFirstResponseLatencies(firstResponseLatency);
82+
recorderWrapper.putBatchRequestThrottled(throttlingLatency);
8483

85-
tracer.record("OK", TABLE_ID, ZONE, CLUSTER);
84+
recorderWrapper.record("OK", TABLE_ID, ZONE, CLUSTER);
8685

8786
Thread.sleep(100);
8887

@@ -244,7 +243,7 @@ public void testStreamingOperation() throws InterruptedException {
244243

245244
@Test
246245
public void testUnaryOperations() throws InterruptedException {
247-
StatsRecorderWrapper tracer =
246+
StatsRecorderWrapper recorderWrapper =
248247
new StatsRecorderWrapper(
249248
ApiTracerFactory.OperationType.ServerStreaming,
250249
SpanName.of("Bigtable", "MutateRow"),
@@ -263,16 +262,16 @@ public void testUnaryOperations() throws InterruptedException {
263262
long throttlingLatency = 50;
264263
long firstResponseLatency = 90;
265264

266-
tracer.putOperationLatencies(operationLatency);
267-
tracer.putRetryCount(attemptCount);
268-
tracer.putAttemptLatencies(attemptLatency);
269-
tracer.putApplicationLatencies(applicationLatency);
270-
tracer.putGfeLatencies(serverLatency);
271-
tracer.putGfeMissingHeaders(connectivityErrorCount);
272-
tracer.putFirstResponseLatencies(firstResponseLatency);
273-
tracer.putBatchRequestThrottled(throttlingLatency);
265+
recorderWrapper.putOperationLatencies(operationLatency);
266+
recorderWrapper.putRetryCount(attemptCount);
267+
recorderWrapper.putAttemptLatencies(attemptLatency);
268+
recorderWrapper.putApplicationLatencies(applicationLatency);
269+
recorderWrapper.putGfeLatencies(serverLatency);
270+
recorderWrapper.putGfeMissingHeaders(connectivityErrorCount);
271+
recorderWrapper.putFirstResponseLatencies(firstResponseLatency);
272+
recorderWrapper.putBatchRequestThrottled(throttlingLatency);
274273

275-
tracer.record(Status.UNAVAILABLE.toString(), TABLE_ID, ZONE, CLUSTER);
274+
recorderWrapper.record("UNAVAILABLE", TABLE_ID, ZONE, CLUSTER);
276275

277276
Thread.sleep(100);
278277

@@ -283,7 +282,7 @@ public void testUnaryOperations() throws InterruptedException {
283282
BuiltinMeasureConstants.METHOD,
284283
"Bigtable.MutateRow",
285284
BuiltinMeasureConstants.STATUS,
286-
Status.UNAVAILABLE.toString(),
285+
"UNAVAILABLE",
287286
BuiltinMeasureConstants.TABLE,
288287
TABLE_ID,
289288
BuiltinMeasureConstants.ZONE,
@@ -305,7 +304,7 @@ public void testUnaryOperations() throws InterruptedException {
305304
BuiltinMeasureConstants.METHOD,
306305
"Bigtable.MutateRow",
307306
BuiltinMeasureConstants.STATUS,
308-
Status.UNAVAILABLE.toString(),
307+
"UNAVAILABLE",
309308
BuiltinMeasureConstants.TABLE,
310309
TABLE_ID,
311310
BuiltinMeasureConstants.ZONE,
@@ -327,7 +326,7 @@ public void testUnaryOperations() throws InterruptedException {
327326
BuiltinMeasureConstants.METHOD,
328327
"Bigtable.MutateRow",
329328
BuiltinMeasureConstants.STATUS,
330-
Status.UNAVAILABLE.toString(),
329+
"UNAVAILABLE",
331330
BuiltinMeasureConstants.TABLE,
332331
TABLE_ID,
333332
BuiltinMeasureConstants.ZONE,
@@ -347,7 +346,7 @@ public void testUnaryOperations() throws InterruptedException {
347346
BuiltinMeasureConstants.METHOD,
348347
"Bigtable.MutateRow",
349348
BuiltinMeasureConstants.STATUS,
350-
Status.UNAVAILABLE.toString(),
349+
"UNAVAILABLE",
351350
BuiltinMeasureConstants.CLIENT_NAME,
352351
"bigtable-java",
353352
BuiltinMeasureConstants.STREAMING,
@@ -369,7 +368,7 @@ public void testUnaryOperations() throws InterruptedException {
369368
BuiltinMeasureConstants.METHOD,
370369
"Bigtable.MutateRow",
371370
BuiltinMeasureConstants.STATUS,
372-
Status.UNAVAILABLE.toString(),
371+
"UNAVAILABLE",
373372
BuiltinMeasureConstants.TABLE,
374373
TABLE_ID,
375374
BuiltinMeasureConstants.ZONE,
@@ -391,7 +390,7 @@ public void testUnaryOperations() throws InterruptedException {
391390
BuiltinMeasureConstants.METHOD,
392391
"Bigtable.MutateRow",
393392
BuiltinMeasureConstants.STATUS,
394-
Status.UNAVAILABLE.toString(),
393+
"UNAVAILABLE",
395394
BuiltinMeasureConstants.CLIENT_NAME,
396395
"bigtable-java",
397396
BuiltinMeasureConstants.TABLE,
@@ -430,7 +429,7 @@ public void testUnaryOperations() throws InterruptedException {
430429
BuiltinMeasureConstants.CLUSTER,
431430
CLUSTER,
432431
BuiltinMeasureConstants.STATUS,
433-
Status.UNAVAILABLE.toString(),
432+
"UNAVAILABLE",
434433
BuiltinMeasureConstants.CLIENT_NAME,
435434
"bigtable-java"),
436435
PROJECT_ID,

pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@
336336

337337
<modules>
338338
<module>google-cloud-bigtable</module>
339+
<module>google-cloud-bigtable-stats</module>
339340
<module>grpc-google-cloud-bigtable-admin-v2</module>
340341
<module>grpc-google-cloud-bigtable-v2</module>
341342
<module>proto-google-cloud-bigtable-admin-v2</module>

0 commit comments

Comments
 (0)