Skip to content

Commit 1683365

Browse files
mutianfmutianf
authored
fix: reset a measure map everytime the stats are recorded (#1364)
1 parent faa5b36 commit 1683365

File tree

5 files changed

+48
-24
lines changed

5 files changed

+48
-24
lines changed

‎google-cloud-bigtable-stats/clirr-ignored-differences.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,10 @@
77
<method>*StatsRecorderWrapper*</method>
88
<to>*StatsRecorder*</to>
99
</difference>
10+
<!-- Internal API is updated -->
11+
<difference>
12+
<differenceType>7002</differenceType>
13+
<className>com/google/cloud/bigtable/stats/StatsRecorderWrapper</className>
14+
<method>void record(java.lang.String, java.lang.String, java.lang.String, java.lang.String)</method>
15+
</difference>
1016
</differences>

‎google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsRecorderWrapper.java‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ public class StatsRecorderWrapper {
4040
private final SpanName spanName;
4141
private final Map<String, String> statsAttributes;
4242

43-
private MeasureMap measureMap;
43+
private MeasureMap attemptMeasureMap;
44+
private MeasureMap operationMeasureMap;
4445

4546
public StatsRecorderWrapper(
4647
OperationType operationType,
@@ -54,10 +55,11 @@ public StatsRecorderWrapper(
5455
this.parentContext = tagger.getCurrentTagContext();
5556
this.statsAttributes = statsAttributes;
5657

57-
this.measureMap = statsRecorder.newMeasureMap();
58+
this.attemptMeasureMap = statsRecorder.newMeasureMap();
59+
this.operationMeasureMap = statsRecorder.newMeasureMap();
5860
}
5961

60-
public void record(String status, String tableId, String zone, String cluster) {
62+
public void recordOperation(String status, String tableId, String zone, String cluster) {
6163
TagContextBuilder tagCtx =
6264
newTagContextBuilder(tableId, zone, cluster)
6365
.putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status));
@@ -66,39 +68,55 @@ public void record(String status, String tableId, String zone, String cluster) {
6668
tagCtx.putLocal(
6769
BuiltinMeasureConstants.STREAMING, TagValue.create(Boolean.toString(isStreaming)));
6870

69-
measureMap.record(tagCtx.build());
71+
operationMeasureMap.record(tagCtx.build());
72+
// Reinitialize a new map
73+
operationMeasureMap = statsRecorder.newMeasureMap();
74+
}
75+
76+
public void recordAttempt(String status, String tableId, String zone, String cluster) {
77+
TagContextBuilder tagCtx =
78+
newTagContextBuilder(tableId, zone, cluster)
79+
.putLocal(BuiltinMeasureConstants.STATUS, TagValue.create(status));
80+
81+
boolean isStreaming = operationType == OperationType.ServerStreaming;
82+
tagCtx.putLocal(
83+
BuiltinMeasureConstants.STREAMING, TagValue.create(Boolean.toString(isStreaming)));
84+
85+
attemptMeasureMap.record(tagCtx.build());
86+
// Reinitialize a new map
87+
attemptMeasureMap = statsRecorder.newMeasureMap();
7088
}
7189

7290
public void putOperationLatencies(long operationLatency) {
73-
measureMap.put(BuiltinMeasureConstants.OPERATION_LATENCIES, operationLatency);
91+
operationMeasureMap.put(BuiltinMeasureConstants.OPERATION_LATENCIES, operationLatency);
7492
}
7593

7694
public void putAttemptLatencies(long attemptLatency) {
77-
measureMap.put(BuiltinMeasureConstants.ATTEMPT_LATENCIES, attemptLatency);
95+
attemptMeasureMap.put(BuiltinMeasureConstants.ATTEMPT_LATENCIES, attemptLatency);
7896
}
7997

8098
public void putRetryCount(int attemptCount) {
81-
measureMap.put(BuiltinMeasureConstants.RETRY_COUNT, attemptCount);
99+
operationMeasureMap.put(BuiltinMeasureConstants.RETRY_COUNT, attemptCount);
82100
}
83101

84102
public void putApplicationLatencies(long applicationLatency) {
85-
measureMap.put(BuiltinMeasureConstants.APPLICATION_LATENCIES, applicationLatency);
103+
operationMeasureMap.put(BuiltinMeasureConstants.APPLICATION_LATENCIES, applicationLatency);
86104
}
87105

88106
public void putFirstResponseLatencies(long firstResponseLatency) {
89-
measureMap.put(BuiltinMeasureConstants.FIRST_RESPONSE_LATENCIES, firstResponseLatency);
107+
operationMeasureMap.put(BuiltinMeasureConstants.FIRST_RESPONSE_LATENCIES, firstResponseLatency);
90108
}
91109

92110
public void putGfeLatencies(long serverLatency) {
93-
measureMap.put(BuiltinMeasureConstants.SERVER_LATENCIES, serverLatency);
111+
attemptMeasureMap.put(BuiltinMeasureConstants.SERVER_LATENCIES, serverLatency);
94112
}
95113

96114
public void putGfeMissingHeaders(long connectivityErrors) {
97-
measureMap.put(BuiltinMeasureConstants.CONNECTIVITY_ERROR_COUNT, connectivityErrors);
115+
attemptMeasureMap.put(BuiltinMeasureConstants.CONNECTIVITY_ERROR_COUNT, connectivityErrors);
98116
}
99117

100118
public void putBatchRequestThrottled(long throttledTimeMs) {
101-
measureMap.put(BuiltinMeasureConstants.THROTTLING_LATENCIES, throttledTimeMs);
119+
operationMeasureMap.put(BuiltinMeasureConstants.THROTTLING_LATENCIES, throttledTimeMs);
102120
}
103121

104122
private TagContextBuilder newTagContextBuilder(String tableId, String zone, String cluster) {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ public void testStreamingOperation() throws InterruptedException {
9393
recorderWrapper.putFirstResponseLatencies(firstResponseLatency);
9494
recorderWrapper.putBatchRequestThrottled(throttlingLatency);
9595

96-
recorderWrapper.record("OK", TABLE_ID, ZONE, CLUSTER);
96+
recorderWrapper.recordOperation("OK", TABLE_ID, ZONE, CLUSTER);
97+
recorderWrapper.recordAttempt("OK", TABLE_ID, ZONE, CLUSTER);
9798

9899
Thread.sleep(100);
99100

@@ -291,7 +292,8 @@ public void testUnaryOperations() throws InterruptedException {
291292
recorderWrapper.putFirstResponseLatencies(firstResponseLatency);
292293
recorderWrapper.putBatchRequestThrottled(throttlingLatency);
293294

294-
recorderWrapper.record("UNAVAILABLE", TABLE_ID, ZONE, CLUSTER);
295+
recorderWrapper.recordOperation("UNAVAILABLE", TABLE_ID, ZONE, CLUSTER);
296+
recorderWrapper.recordAttempt("UNAVAILABLE", TABLE_ID, ZONE, CLUSTER);
295297

296298
Thread.sleep(100);
297299

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracer.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ private void recordOperationCompletion(@Nullable Throwable status) {
242242
recorder.putFirstResponseLatencies(firstResponsePerOpTimer.elapsed(TimeUnit.MILLISECONDS));
243243
}
244244

245-
recorder.record(Util.extractStatus(status), tableId, zone, cluster);
245+
recorder.recordOperation(Util.extractStatus(status), tableId, zone, cluster);
246246
}
247247

248248
private void recordAttemptCompletion(@Nullable Throwable status) {
@@ -257,6 +257,6 @@ private void recordAttemptCompletion(@Nullable Throwable status) {
257257
}
258258
}
259259
recorder.putAttemptLatencies(attemptTimer.elapsed(TimeUnit.MILLISECONDS));
260-
recorder.record(Util.extractStatus(status), tableId, zone, cluster);
260+
recorder.recordAttempt(Util.extractStatus(status), tableId, zone, cluster);
261261
}
262262
}

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsTracerTest.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -331,18 +331,16 @@ public void testMutateRowAttempts() {
331331
stub.mutateRowCallable()
332332
.call(RowMutation.create(TABLE_ID, "random-row").setCell("cf", "q", "value"));
333333

334-
// record() will get called 4 times, 3 times for attempts and 1 for recording operation level
335-
// metrics. Also set a timeout to reduce flakiness of this test. BasicRetryingFuture will set
334+
// Set a timeout to reduce flakiness of this test. BasicRetryingFuture will set
336335
// attempt succeeded and set the response which will call complete() in AbstractFuture which
337336
// calls releaseWaiters(). onOperationComplete() is called in TracerFinisher which will be
338337
// called after the mutateRow call is returned. So there's a race between when the call returns
339338
// and when the record() is called in onOperationCompletion().
340-
verify(statsRecorderWrapper, timeout(50).times(fakeService.getAttemptCounter().get() + 1))
341-
.record(status.capture(), tableId.capture(), zone.capture(), cluster.capture());
342-
assertThat(zone.getAllValues()).containsExactly("global", "global", ZONE, ZONE);
343-
assertThat(cluster.getAllValues())
344-
.containsExactly("unspecified", "unspecified", CLUSTER, CLUSTER);
345-
assertThat(status.getAllValues()).containsExactly("UNAVAILABLE", "UNAVAILABLE", "OK", "OK");
339+
verify(statsRecorderWrapper, timeout(50).times(fakeService.getAttemptCounter().get()))
340+
.recordAttempt(status.capture(), tableId.capture(), zone.capture(), cluster.capture());
341+
assertThat(zone.getAllValues()).containsExactly("global", "global", ZONE);
342+
assertThat(cluster.getAllValues()).containsExactly("unspecified", "unspecified", CLUSTER);
343+
assertThat(status.getAllValues()).containsExactly("UNAVAILABLE", "UNAVAILABLE", "OK");
346344
}
347345

348346
private static class FakeService extends BigtableGrpc.BigtableImplBase {

0 commit comments

Comments
 (0)