Skip to content

Commit f45bb0e

Browse files
sushanbsushanb
authored
fix(bigtable): drop redudant fields from internal otel metrics which are already in monitored resource (#2783)
1 parent de1669e commit f45bb0e

5 files changed

Lines changed: 48 additions & 48 deletions

File tree

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,7 @@ public static BigtableClientContext create(EnhancedBigtableStubSettings settings
121121
.getInternalMetricsProvider()
122122
.createOtelProvider(settings, credentials, backgroundExecutor);
123123
if (internalOtel != null) {
124-
channelPoolMetricsTracer =
125-
new ChannelPoolMetricsTracer(
126-
internalOtel, EnhancedBigtableStub.createBuiltinAttributes(builder.build()));
124+
channelPoolMetricsTracer = new ChannelPoolMetricsTracer(internalOtel);
127125

128126
// Configure grpc metrics
129127
configureGrpcOtel(transportProvider, internalOtel);

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

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import java.util.concurrent.TimeUnit;
3333
import java.util.concurrent.atomic.AtomicReference;
3434
import java.util.logging.Logger;
35-
import javax.annotation.Nullable;
3635

3736
@InternalApi("For internal use only")
3837
public class ChannelPoolMetricsTracer implements Runnable {
@@ -45,15 +44,11 @@ public class ChannelPoolMetricsTracer implements Runnable {
4544
private final AtomicReference<BigtableChannelPoolObserver> bigtableChannelInsightsProviderRef =
4645
new AtomicReference<>();
4746
private final AtomicReference<String> lbPolicyRef = new AtomicReference<>("ROUND_ROBIN");
48-
private final Attributes commonAttrs;
4947

5048
// Attributes for unary and streaming RPCs, built on demand in run()
51-
@Nullable private Attributes unaryAttributes;
52-
@Nullable private Attributes streamingAttributes;
5349

54-
public ChannelPoolMetricsTracer(OpenTelemetry openTelemetry, Attributes commonAttrs) {
50+
public ChannelPoolMetricsTracer(OpenTelemetry openTelemetry) {
5551
Meter meter = openTelemetry.getMeter(METER_NAME);
56-
this.commonAttrs = commonAttrs;
5752
this.outstandingRpcsHistogram =
5853
meter
5954
.histogramBuilder(OUTSTANDING_RPCS_PER_CHANNEL_NAME)
@@ -99,35 +94,51 @@ public void run() {
9994
logger.warning("No Bigtable ChannelPoolObserver available");
10095
return; // Not registered yet
10196
}
102-
String lbPolicy = lbPolicyRef.get();
103-
104-
// Build attributes if they haven't been built yet.
105-
if (unaryAttributes == null || streamingAttributes == null) {
106-
Attributes baseAttrs = commonAttrs.toBuilder().put("lb_policy", lbPolicy).build();
107-
this.unaryAttributes = baseAttrs.toBuilder().put("streaming", false).build();
108-
this.streamingAttributes = baseAttrs.toBuilder().put("streaming", true).build();
109-
}
11097
List<? extends BigtableChannelObserver> channelInsights =
11198
channelInsightsProvider.getChannelInfos();
11299
if (channelInsights == null || channelInsights.isEmpty()) {
113100
return;
114101
}
102+
103+
String lbPolicy = lbPolicyRef.get();
104+
105+
Attributes dpUnaryAttrs =
106+
Attributes.builder()
107+
.put("transport_type", "directpath")
108+
.put("streaming", false)
109+
.put("lb_policy", lbPolicy)
110+
.build();
111+
Attributes dpStreamingAttrs =
112+
Attributes.builder()
113+
.put("transport_type", "directpath")
114+
.put("streaming", true)
115+
.put("lb_policy", lbPolicy)
116+
.build();
117+
Attributes cpUnaryAttrs =
118+
Attributes.builder()
119+
.put("transport_type", "cloudpath")
120+
.put("streaming", false)
121+
.put("lb_policy", lbPolicy)
122+
.build();
123+
Attributes cpStreamingAttrs =
124+
Attributes.builder()
125+
.put("transport_type", "cloudpath")
126+
.put("streaming", true)
127+
.put("lb_policy", lbPolicy)
128+
.build();
129+
115130
for (BigtableChannelObserver info : channelInsights) {
116-
String transportTypeValue = info.isAltsChannel() ? "DIRECTPATH" : "CLOUDPATH";
117-
this.unaryAttributes =
118-
this.unaryAttributes.toBuilder().put("transport_type", transportTypeValue).build();
119-
this.streamingAttributes =
120-
this.streamingAttributes.toBuilder().put("transport_type", transportTypeValue).build();
131+
Attributes unaryAttrs = info.isAltsChannel() ? dpUnaryAttrs : cpUnaryAttrs;
132+
Attributes streamingAttrs = info.isAltsChannel() ? dpStreamingAttrs : cpStreamingAttrs;
121133

122134
long currentOutstandingUnaryRpcs = info.getOutstandingUnaryRpcs();
123135
long currentOutstandingStreamingRpcs = info.getOutstandingStreamingRpcs();
124-
// Record outstanding unary RPCs with streaming=false
125-
outstandingRpcsHistogram.record(currentOutstandingUnaryRpcs, unaryAttributes);
126-
// Record outstanding streaming RPCs with streaming=true
127-
outstandingRpcsHistogram.record(currentOutstandingStreamingRpcs, streamingAttributes);
136+
outstandingRpcsHistogram.record(currentOutstandingUnaryRpcs, unaryAttrs);
137+
outstandingRpcsHistogram.record(currentOutstandingStreamingRpcs, streamingAttrs);
128138

129139
long errors = info.getAndResetErrorCount();
130-
perConnectionErrorCountHistogram.record(errors, commonAttrs);
140+
// Record errors with empty attributes.
141+
perConnectionErrorCountHistogram.record(errors, Attributes.empty());
131142
}
132143
}
133144
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/grpc/BigtableTransportChannelProvider.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
*/
3939
@InternalApi
4040
public final class BigtableTransportChannelProvider implements TransportChannelProvider {
41-
4241
private final InstantiatingGrpcChannelProvider delegate;
4342
private final ChannelPrimer channelPrimer;
4443
@Nullable private final ChannelPoolMetricsTracer channelPoolMetricsTracer;

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ public class ChannelPoolMetricsTracerTest {
6060
private ArgumentCaptor<Runnable> runnableCaptor;
6161

6262
private ChannelPoolMetricsTracer tracker;
63-
private Attributes baseAttributes;
6463

6564
@Mock private BigtableChannelPoolObserver mockInsightsProvider;
6665
@Mock private BigtableChannelObserver mockInsight1;
@@ -74,9 +73,7 @@ public void setUp() {
7473
OpenTelemetry openTelemetry =
7574
OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build();
7675

77-
baseAttributes = Attributes.builder().build();
78-
79-
tracker = new ChannelPoolMetricsTracer(openTelemetry, baseAttributes);
76+
tracker = new ChannelPoolMetricsTracer(openTelemetry);
8077

8178
runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
8279
// Configure mockScheduler to capture the runnable when tracker.start() is called
@@ -114,7 +111,7 @@ private Attributes getExpectedErrorAttributes() {
114111

115112
private static Attributes getExpectedRpcAttributes(String lbPolicy, boolean streaming) {
116113
return Attributes.builder()
117-
.put(AttributeKey.stringKey("transport_type"), "CLOUDPATH")
114+
.put(AttributeKey.stringKey("transport_type"), "cloudpath")
118115
.put(AttributeKey.stringKey("lb_policy"), lbPolicy)
119116
.put(AttributeKey.booleanKey("streaming"), streaming)
120117
.build();

‎test-proxy/src/main/java/com/google/cloud/bigtable/testproxy/CbtTestProxy.java‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,7 @@ public void mutateRow(
289289
// This response is empty.
290290
client.dataClient().mutateRow(mutation);
291291
} catch (ApiException e) {
292-
responseObserver.onNext(
293-
MutateRowResult.newBuilder().setStatus(convertStatus(e)).build());
292+
responseObserver.onNext(MutateRowResult.newBuilder().setStatus(convertStatus(e)).build());
294293
responseObserver.onCompleted();
295294
return;
296295
} catch (StatusRuntimeException e) {
@@ -342,8 +341,7 @@ public void bulkMutateRows(
342341
responseObserver.onCompleted();
343342
return;
344343
} catch (ApiException e) {
345-
responseObserver.onNext(
346-
MutateRowsResult.newBuilder().setStatus(convertStatus(e)).build());
344+
responseObserver.onNext(MutateRowsResult.newBuilder().setStatus(convertStatus(e)).build());
347345
responseObserver.onCompleted();
348346
return;
349347
} catch (StatusRuntimeException e) {
@@ -395,8 +393,7 @@ public void readRow(ReadRowRequest request, StreamObserver<RowResult> responseOb
395393
logger.info(String.format("readRow() did not find row: %s", request.getRowKey()));
396394
}
397395
} catch (ApiException e) {
398-
responseObserver.onNext(
399-
RowResult.newBuilder().setStatus(convertStatus(e)).build());
396+
responseObserver.onNext(RowResult.newBuilder().setStatus(convertStatus(e)).build());
400397
responseObserver.onCompleted();
401398
return;
402399
} catch (StatusRuntimeException e) {
@@ -440,8 +437,7 @@ public void readRows(ReadRowsRequest request, StreamObserver<RowsResult> respons
440437
// Note that the default instance == OK
441438
resultBuilder.setStatus(com.google.rpc.Status.getDefaultInstance()).build());
442439
} catch (ApiException e) {
443-
responseObserver.onNext(
444-
RowsResult.newBuilder().setStatus(convertStatus(e)).build());
440+
responseObserver.onNext(RowsResult.newBuilder().setStatus(convertStatus(e)).build());
445441
responseObserver.onCompleted();
446442
return;
447443
} catch (StatusRuntimeException e) {
@@ -558,8 +554,7 @@ public void sampleRowKeys(
558554
try {
559555
keyOffsets = client.dataClient().sampleRowKeys(tableId);
560556
} catch (ApiException e) {
561-
responseObserver.onNext(
562-
SampleRowKeysResult.newBuilder().setStatus(convertStatus(e)).build());
557+
responseObserver.onNext(SampleRowKeysResult.newBuilder().setStatus(convertStatus(e)).build());
563558
responseObserver.onCompleted();
564559
return;
565560
} catch (StatusRuntimeException e) {
@@ -643,8 +638,7 @@ public void readModifyWriteRow(
643638
"readModifyWriteRow() did not find row: %s", request.getRequest().getRowKey()));
644639
}
645640
} catch (ApiException e) {
646-
responseObserver.onNext(
647-
RowResult.newBuilder().setStatus(convertStatus(e)).build());
641+
responseObserver.onNext(RowResult.newBuilder().setStatus(convertStatus(e)).build());
648642
responseObserver.onCompleted();
649643
return;
650644
} catch (StatusRuntimeException e) {
@@ -700,8 +694,7 @@ public void executeQuery(
700694
responseObserver.onError(e);
701695
return;
702696
} catch (ApiException e) {
703-
responseObserver.onNext(
704-
ExecuteQueryResult.newBuilder().setStatus(convertStatus(e)).build());
697+
responseObserver.onNext(ExecuteQueryResult.newBuilder().setStatus(convertStatus(e)).build());
705698
responseObserver.onCompleted();
706699
return;
707700
} catch (StatusRuntimeException e) {
@@ -803,7 +796,9 @@ private static com.google.rpc.Status convertStatus(ApiException e) {
803796
return status;
804797
}
805798

806-
return com.google.rpc.Status.newBuilder().setCode(e.getStatusCode().getCode().ordinal()).setMessage(e.getMessage())
799+
return com.google.rpc.Status.newBuilder()
800+
.setCode(e.getStatusCode().getCode().ordinal())
801+
.setMessage(e.getMessage())
807802
.build();
808803
}
809804

0 commit comments

Comments
 (0)