Skip to content

Commit c4f360c

Browse files
committed
Fix: suggested changes
1 parent 2b41e1c commit c4f360c

5 files changed

Lines changed: 27 additions & 25 deletions

File tree

api/src/main/java/io/grpc/NameResolver.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,9 +561,7 @@ public Builder toBuilder() {
561561
builder.setOverrideAuthority(overrideAuthority);
562562
builder.setMetricRecorder(metricRecorder);
563563
builder.setNameResolverRegistry(nameResolverRegistry);
564-
if (channelConfigurator != null) {
565-
builder.setChildChannelConfigurator(channelConfigurator);
566-
}
564+
builder.setChildChannelConfigurator(channelConfigurator);
567565
builder.customArgs = cloneCustomArgs(customArgs);
568566
return builder;
569567
}

xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ public XdsStreamingCall(
185185
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
186186
.setRequestMarshaller(reqMarshaller)
187187
.setResponseMarshaller(respMarshaller)
188+
.setSampledToLocalTracing(true)
188189
.build(),
189190
CallOptions.DEFAULT.withCallCredentials(
190191
callCredentials)); // TODO(zivy): support waitForReady

xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ public interface XdsTransportFactory {
3131
* Represents transport for xDS communication (e.g., a gRPC channel).
3232
*/
3333
interface XdsTransport {
34+
/**
35+
* Creates a bidirectional streaming call.
36+
*
37+
* @param fullMethodName should be a constant/literal string so that metric recorders
38+
* like OpenTelemetry can record the exact method name.
39+
*/
3440
<ReqT, RespT> StreamingCall<ReqT, RespT> createStreamingCall(
3541
String fullMethodName, MethodDescriptor.Marshaller<ReqT> reqMarshaller,
3642
MethodDescriptor.Marshaller<RespT> respMarshaller);

xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@
7373
import java.net.InetSocketAddress;
7474
import java.util.Arrays;
7575
import java.util.List;
76-
import java.util.concurrent.atomic.AtomicInteger;
76+
import java.util.concurrent.CountDownLatch;
77+
import java.util.concurrent.TimeUnit;
7778
import org.junit.Before;
7879
import org.junit.Rule;
7980
import org.junit.Test;
@@ -427,25 +428,20 @@ public void childChannelConfigurator_passesMetricSinkToServer_E2E() throws Excep
427428
}
428429

429430
private static final class CountingMetricSink extends NoopMetricSink {
430-
private final AtomicInteger count =
431-
new AtomicInteger();
431+
private final CountDownLatch latch = new CountDownLatch(1);
432432

433433
@Override
434434
public void addLongCounter(
435435
LongCounterMetricInstrument metricInstrument,
436436
long value,
437437
List<String> requiredLabelValues,
438438
List<String> optionalLabelValues) {
439-
count.incrementAndGet();
439+
latch.countDown();
440440
}
441441

442442
public void awaitCall() throws InterruptedException {
443-
long start = System.currentTimeMillis();
444-
while (count.get() == 0) {
445-
if (System.currentTimeMillis() - start > 5000) {
446-
throw new AssertionError("Timed out waiting for metric sink call");
447-
}
448-
Thread.sleep(50);
443+
if (!latch.await(5, TimeUnit.SECONDS)) {
444+
throw new AssertionError("Timed out waiting for metric sink call");
449445
}
450446
}
451447
}

xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsOtelIntegrationTest.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import io.opentelemetry.sdk.metrics.data.MetricData;
3636
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
3737
import java.util.Arrays;
38+
import java.util.concurrent.TimeUnit;
3839
import org.junit.Before;
3940
import org.junit.Rule;
4041
import org.junit.Test;
@@ -118,22 +119,22 @@ public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
118119
SimpleServiceGrpc.newBlockingStub(channel);
119120
blockingStub.unaryRpc(SimpleRequest.getDefaultInstance());
120121

122+
// Shut down the channel to complete the RPC cycle.
123+
channel.shutdownNow();
124+
channel.awaitTermination(5, TimeUnit.SECONDS);
125+
121126
// Verify that OpenTelemetry metrics specifically from the xDS Control Plane ADS stream
122-
// successfully propagated, method name is recorded as 'other' because dynamic descriptors
123-
// are not sampled to local tracing by default.
124-
boolean foundXdsMetrics = false;
125-
for (int i = 0; i < 50; i++) {
126-
for (MetricData metric : metricReader.collectAllMetrics()) {
127-
if (metric.toString().contains("grpc.client.") && metric.toString().contains("other")) {
128-
foundXdsMetrics = true;
127+
// successfully propagated, with the exact stream method name preserved.
128+
boolean foundTargetMethod = false;
129+
for (MetricData metric : metricReader.collectAllMetrics()) {
130+
String name = metric.getName();
131+
if ("grpc.client.attempt.started".equals(name) || "grpc.client.call.duration".equals(name)) {
132+
if (metric.toString().contains("envoy.service.discovery.v3.AggregatedDiscoveryService")) {
133+
foundTargetMethod = true;
129134
break;
130135
}
131136
}
132-
if (foundXdsMetrics) {
133-
break;
134-
}
135-
Thread.sleep(100);
136137
}
137-
assertThat(foundXdsMetrics).isTrue();
138+
assertThat(foundTargetMethod).isTrue();
138139
}
139140
}

0 commit comments

Comments
 (0)