Skip to content

Commit 777549e

Browse files
mutianfmutianf
authored
fix: add status label to gfe metrics (#1077)
1 parent d0709e8 commit 777549e

File tree

8 files changed

+92
-38
lines changed

8 files changed

+92
-38
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,5 @@ public abstract class BigtableTracer extends BaseApiTracer {
3737
* the response from server-timing header. If server-timing header is missing, increment the
3838
* missing header count.
3939
*/
40-
public abstract void recordGfeMetadata(@Nullable Long latency);
40+
public abstract void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable);
4141
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,9 @@ public int getAttempt() {
173173
}
174174

175175
@Override
176-
public void recordGfeMetadata(@Nullable Long latency) {
176+
public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) {
177177
for (BigtableTracer tracer : bigtableTracers) {
178-
tracer.recordGfeMetadata(latency);
178+
tracer.recordGfeMetadata(latency, throwable);
179179
}
180180
}
181181
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,15 @@ public void onError(Throwable t) {
9696
// so it's not checking trailing metadata here.
9797
Metadata metadata = responseMetadata.getMetadata();
9898
Long latency = Util.getGfeLatency(metadata);
99-
tracer.recordGfeMetadata(latency);
99+
tracer.recordGfeMetadata(latency, t);
100100
outerObserver.onError(t);
101101
}
102102

103103
@Override
104104
public void onComplete() {
105105
Metadata metadata = responseMetadata.getMetadata();
106106
Long latency = Util.getGfeLatency(metadata);
107-
tracer.recordGfeMetadata(latency);
107+
tracer.recordGfeMetadata(latency, null);
108108
outerObserver.onComplete();
109109
}
110110
}

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

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
package com.google.cloud.bigtable.data.v2.stub.metrics;
1717

1818
import com.google.api.core.ApiFuture;
19+
import com.google.api.core.ApiFutureCallback;
20+
import com.google.api.core.ApiFutures;
1921
import com.google.api.core.InternalApi;
2022
import com.google.api.gax.grpc.GrpcResponseMetadata;
2123
import com.google.api.gax.rpc.ApiCallContext;
@@ -51,27 +53,42 @@ public HeaderTracerUnaryCallable(@Nonnull UnaryCallable<RequestT, ResponseT> inn
5153

5254
@Override
5355
public ApiFuture futureCall(RequestT request, ApiCallContext context) {
54-
if (RpcViews.isGfeMetricsRegistered()) {
56+
// tracer should always be an instance of BigtableTracer
57+
if (RpcViews.isGfeMetricsRegistered() && context.getTracer() instanceof BigtableTracer) {
5558
final GrpcResponseMetadata responseMetadata = new GrpcResponseMetadata();
5659
final ApiCallContext contextWithResponseMetadata = responseMetadata.addHandlers(context);
60+
HeaderTracerUnaryCallback callback =
61+
new HeaderTracerUnaryCallback((BigtableTracer) context.getTracer(), responseMetadata);
5762
ApiFuture<ResponseT> future = innerCallable.futureCall(request, contextWithResponseMetadata);
58-
future.addListener(
59-
new Runnable() {
60-
@Override
61-
public void run() {
62-
// this should always be true
63-
if (contextWithResponseMetadata.getTracer() instanceof BigtableTracer) {
64-
BigtableTracer tracer = (BigtableTracer) contextWithResponseMetadata.getTracer();
65-
Metadata metadata = responseMetadata.getMetadata();
66-
Long latency = Util.getGfeLatency(metadata);
67-
tracer.recordGfeMetadata(latency);
68-
}
69-
}
70-
},
71-
MoreExecutors.directExecutor());
63+
ApiFutures.addCallback(future, callback, MoreExecutors.directExecutor());
7264
return future;
7365
} else {
7466
return innerCallable.futureCall(request, context);
7567
}
7668
}
69+
70+
class HeaderTracerUnaryCallback<ResponseT> implements ApiFutureCallback<ResponseT> {
71+
72+
private final BigtableTracer tracer;
73+
private final GrpcResponseMetadata responseMetadata;
74+
75+
HeaderTracerUnaryCallback(BigtableTracer tracer, GrpcResponseMetadata responseMetadata) {
76+
this.tracer = tracer;
77+
this.responseMetadata = responseMetadata;
78+
}
79+
80+
@Override
81+
public void onFailure(Throwable throwable) {
82+
Metadata metadata = responseMetadata.getMetadata();
83+
Long latency = Util.getGfeLatency(metadata);
84+
tracer.recordGfeMetadata(latency, throwable);
85+
}
86+
87+
@Override
88+
public void onSuccess(ResponseT response) {
89+
Metadata metadata = responseMetadata.getMetadata();
90+
Long latency = Util.getGfeLatency(metadata);
91+
tracer.recordGfeMetadata(latency, null);
92+
}
93+
}
7794
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ public int getAttempt() {
211211
}
212212

213213
@Override
214-
public void recordGfeMetadata(@Nullable Long latency) {
214+
public void recordGfeMetadata(@Nullable Long latency, @Nullable Throwable throwable) {
215215
MeasureMap measures = stats.newMeasureMap();
216216
if (latency != null) {
217217
measures
@@ -220,7 +220,10 @@ public void recordGfeMetadata(@Nullable Long latency) {
220220
} else {
221221
measures.put(RpcMeasureConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT, 1L);
222222
}
223-
measures.record(newTagCtxBuilder().build());
223+
measures.record(
224+
newTagCtxBuilder()
225+
.putLocal(RpcMeasureConstants.BIGTABLE_STATUS, Util.extractStatus(throwable))
226+
.build());
224227
}
225228

226229
private TagContextBuilder newTagCtxBuilder() {

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,11 @@ class RpcViewConstants {
136136
BIGTABLE_GFE_LATENCY,
137137
AGGREGATION_WITH_MILLIS_HISTOGRAM,
138138
ImmutableList.of(
139-
BIGTABLE_INSTANCE_ID, BIGTABLE_PROJECT_ID, BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP));
139+
BIGTABLE_INSTANCE_ID,
140+
BIGTABLE_PROJECT_ID,
141+
BIGTABLE_APP_PROFILE_ID,
142+
BIGTABLE_OP,
143+
BIGTABLE_STATUS));
140144

141145
static final View BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW =
142146
View.create(
@@ -145,5 +149,9 @@ class RpcViewConstants {
145149
BIGTABLE_GFE_HEADER_MISSING_COUNT,
146150
SUM,
147151
ImmutableList.of(
148-
BIGTABLE_INSTANCE_ID, BIGTABLE_PROJECT_ID, BIGTABLE_APP_PROFILE_ID, BIGTABLE_OP));
152+
BIGTABLE_INSTANCE_ID,
153+
BIGTABLE_PROJECT_ID,
154+
BIGTABLE_APP_PROFILE_ID,
155+
BIGTABLE_OP,
156+
BIGTABLE_STATUS));
149157
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import com.google.api.gax.tracing.ApiTracer;
2424
import com.google.api.gax.tracing.ApiTracer.Scope;
2525
import com.google.common.collect.ImmutableList;
26+
import io.grpc.Status;
27+
import io.grpc.StatusRuntimeException;
2628
import org.junit.Assert;
2729
import org.junit.Before;
2830
import org.junit.Rule;
@@ -222,8 +224,9 @@ public void testGetAttempt() {
222224

223225
@Test
224226
public void testRecordGfeLatency() {
225-
compositeTracer.recordGfeMetadata(20L);
226-
verify(child3, times(1)).recordGfeMetadata(20L);
227-
verify(child4, times(1)).recordGfeMetadata(20L);
227+
Throwable t = new StatusRuntimeException(Status.UNAVAILABLE);
228+
compositeTracer.recordGfeMetadata(20L, t);
229+
verify(child3, times(1)).recordGfeMetadata(20L, t);
230+
verify(child4, times(1)).recordGfeMetadata(20L, t);
228231
}
229232
}

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

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ public void testGFELatencyMetricReadRows() throws InterruptedException {
168168
localStats,
169169
RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW,
170170
ImmutableMap.<TagKey, TagValue>of(
171-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")),
171+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows"),
172+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
172173
PROJECT_ID,
173174
INSTANCE_ID,
174175
APP_PROFILE_ID);
@@ -186,7 +187,9 @@ public void testGFELatencyMetricMutateRow() throws InterruptedException {
186187
StatsTestUtils.getAggregationValueAsLong(
187188
localStats,
188189
RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW,
189-
ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")),
190+
ImmutableMap.of(
191+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow"),
192+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
190193
PROJECT_ID,
191194
INSTANCE_ID,
192195
APP_PROFILE_ID);
@@ -208,7 +211,8 @@ public void testGFELatencyMetricMutateRows() throws InterruptedException {
208211
localStats,
209212
RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW,
210213
ImmutableMap.of(
211-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRows")),
214+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRows"),
215+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
212216
PROJECT_ID,
213217
INSTANCE_ID,
214218
APP_PROFILE_ID);
@@ -226,7 +230,8 @@ public void testGFELatencySampleRowKeys() throws InterruptedException {
226230
localStats,
227231
RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW,
228232
ImmutableMap.of(
229-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.SampleRowKeys")),
233+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.SampleRowKeys"),
234+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
230235
PROJECT_ID,
231236
INSTANCE_ID,
232237
APP_PROFILE_ID);
@@ -246,7 +251,8 @@ public void testGFELatencyCheckAndMutateRow() throws InterruptedException {
246251
localStats,
247252
RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW,
248253
ImmutableMap.of(
249-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.CheckAndMutateRow")),
254+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.CheckAndMutateRow"),
255+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
250256
PROJECT_ID,
251257
INSTANCE_ID,
252258
APP_PROFILE_ID);
@@ -266,7 +272,8 @@ public void testGFELatencyReadModifyWriteRow() throws InterruptedException {
266272
localStats,
267273
RpcViewConstants.BIGTABLE_GFE_LATENCY_VIEW,
268274
ImmutableMap.of(
269-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadModifyWriteRow")),
275+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadModifyWriteRow"),
276+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
270277
PROJECT_ID,
271278
INSTANCE_ID,
272279
APP_PROFILE_ID);
@@ -285,7 +292,11 @@ public void testGFEMissingHeaderMetric() throws InterruptedException {
285292
StatsTestUtils.getAggregationValueAsLong(
286293
localStats,
287294
RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW,
288-
ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")),
295+
ImmutableMap.of(
296+
RpcMeasureConstants.BIGTABLE_OP,
297+
TagValue.create("Bigtable.MutateRow"),
298+
RpcMeasureConstants.BIGTABLE_STATUS,
299+
TagValue.create("OK")),
289300
PROJECT_ID,
290301
INSTANCE_ID,
291302
APP_PROFILE_ID);
@@ -294,7 +305,8 @@ public void testGFEMissingHeaderMetric() throws InterruptedException {
294305
localStats,
295306
RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW,
296307
ImmutableMap.<TagKey, TagValue>of(
297-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")),
308+
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows"),
309+
RpcMeasureConstants.BIGTABLE_STATUS, TagValue.create("OK")),
298310
PROJECT_ID,
299311
INSTANCE_ID,
300312
APP_PROFILE_ID);
@@ -321,7 +333,11 @@ public void testGFEMissingHeaderMetric() throws InterruptedException {
321333
StatsTestUtils.getAggregationValueAsLong(
322334
localStats,
323335
RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW,
324-
ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.MutateRow")),
336+
ImmutableMap.of(
337+
RpcMeasureConstants.BIGTABLE_OP,
338+
TagValue.create("Bigtable.MutateRow"),
339+
RpcMeasureConstants.BIGTABLE_STATUS,
340+
TagValue.create("OK")),
325341
PROJECT_ID,
326342
INSTANCE_ID,
327343
APP_PROFILE_ID);
@@ -330,7 +346,10 @@ public void testGFEMissingHeaderMetric() throws InterruptedException {
330346
localStats,
331347
RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW,
332348
ImmutableMap.<TagKey, TagValue>of(
333-
RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")),
349+
RpcMeasureConstants.BIGTABLE_OP,
350+
TagValue.create("Bigtable.ReadRows"),
351+
RpcMeasureConstants.BIGTABLE_STATUS,
352+
TagValue.create("OK")),
334353
PROJECT_ID,
335354
INSTANCE_ID,
336355
APP_PROFILE_ID);
@@ -353,7 +372,11 @@ public void testMetricsWithErrorResponse() throws InterruptedException {
353372
StatsTestUtils.getAggregationValueAsLong(
354373
localStats,
355374
RpcViewConstants.BIGTABLE_GFE_HEADER_MISSING_COUNT_VIEW,
356-
ImmutableMap.of(RpcMeasureConstants.BIGTABLE_OP, TagValue.create("Bigtable.ReadRows")),
375+
ImmutableMap.of(
376+
RpcMeasureConstants.BIGTABLE_OP,
377+
TagValue.create("Bigtable.ReadRows"),
378+
RpcMeasureConstants.BIGTABLE_STATUS,
379+
TagValue.create("UNAVAILABLE")),
357380
PROJECT_ID,
358381
INSTANCE_ID,
359382
APP_PROFILE_ID);

0 commit comments

Comments
 (0)