Fix: suggested changes

Author: AgraVator

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

@@ -666,7 +666,7 @@ public <X> T setNameResolverArg(NameResolver.Args.Key<X> key, X value) {
    * Sets a configurator that will be applied to all internal child channels created by this
    * channel.
    *
-   * <p>This allows injecting configuration (like credentials, interceptors, or flow control)
+   * <p>This allows injecting universal configuration (like interceptors)
    * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections.
    *
    * @param channelConfigurator the configurator to apply.

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

@@ -478,7 +478,7 @@ public ChannelLogger getChannelLogger() {
      *
      * @since 1.83.0
      */
-    @Internal
+    @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574")
     public ChannelConfigurator getChildChannelConfigurator() {
       return channelConfigurator;
     }
@@ -561,6 +561,9 @@ public Builder toBuilder() {
       builder.setOverrideAuthority(overrideAuthority);
       builder.setMetricRecorder(metricRecorder);
       builder.setNameResolverRegistry(nameResolverRegistry);
+      if (channelConfigurator != null) {
+        builder.setChildChannelConfigurator(channelConfigurator);
+      }
       builder.customArgs = cloneCustomArgs(customArgs);
       return builder;
     }
@@ -712,6 +715,7 @@ public Builder setNameResolverRegistry(NameResolverRegistry registry) {
        *
        * @since 1.83.0
        */
+      @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574")
       public Builder setChildChannelConfigurator(ChannelConfigurator channelConfigurator) {
         this.channelConfigurator = checkNotNull(channelConfigurator, "channelConfigurator");
         return this;

api/src/test/java/io/grpc/NameResolverTest.java

@@ -22,6 +22,7 @@
 import static org.mockito.Mockito.verify;
 
 import com.google.common.base.Objects;
+import io.grpc.ChannelConfigurator;
 import io.grpc.NameResolver.ConfigOrError;
 import io.grpc.NameResolver.Listener2;
 import io.grpc.NameResolver.ResolutionResult;
@@ -72,6 +73,8 @@ public class NameResolverTest {
   private final int customArgValue = 42;
   @Mock NameResolver.Listener mockListener;
 
+  private final ChannelConfigurator channelConfigurator = builder -> { };
+
   @Test
   public void args() {
     NameResolver.Args args = createArgs();
@@ -84,6 +87,7 @@ public void args() {
     assertThat(args.getOffloadExecutor()).isSameInstanceAs(executor);
     assertThat(args.getOverrideAuthority()).isSameInstanceAs(overrideAuthority);
     assertThat(args.getMetricRecorder()).isSameInstanceAs(metricRecorder);
+    assertThat(args.getChildChannelConfigurator()).isSameInstanceAs(channelConfigurator);
     assertThat(args.getArg(FOO_ARG_KEY)).isEqualTo(customArgValue);
     assertThat(args.getArg(BAR_ARG_KEY)).isNull();
 
@@ -97,6 +101,7 @@ public void args() {
     assertThat(args2.getOffloadExecutor()).isSameInstanceAs(executor);
     assertThat(args2.getOverrideAuthority()).isSameInstanceAs(overrideAuthority);
     assertThat(args.getMetricRecorder()).isSameInstanceAs(metricRecorder);
+    assertThat(args2.getChildChannelConfigurator()).isSameInstanceAs(channelConfigurator);
     assertThat(args.getArg(FOO_ARG_KEY)).isEqualTo(customArgValue);
     assertThat(args.getArg(BAR_ARG_KEY)).isNull();
 
@@ -105,7 +110,6 @@ public void args() {
   }
 
   private NameResolver.Args createArgs() {
-    ChannelConfigurator channelConfigurator = builder -> { };
     return NameResolver.Args.newBuilder()
         .setDefaultPort(defaultPort)
         .setProxyDetector(proxyDetector)
@@ -116,8 +120,8 @@ private NameResolver.Args createArgs() {
         .setOffloadExecutor(executor)
         .setOverrideAuthority(overrideAuthority)
         .setMetricRecorder(metricRecorder)
-        .setArg(FOO_ARG_KEY, customArgValue)
         .setChildChannelConfigurator(channelConfigurator)
+        .setArg(FOO_ARG_KEY, customArgValue)
         .build();
   }
 

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -68,6 +68,7 @@
 import io.grpc.CallCredentials.RequestInfo;
 import io.grpc.CallOptions;
 import io.grpc.Channel;
+import io.grpc.ChannelConfigurator;
 import io.grpc.ChannelCredentials;
 import io.grpc.ChannelLogger;
 import io.grpc.ClientCall;
@@ -496,6 +497,40 @@ public void immediateDeadlineExceeded() {
     assertSame(Status.DEADLINE_EXCEEDED.getCode(), status.getCode());
   }
 
+  @Test
+  public void childChannelConfigurator_passedToNameResolverArgs() {
+    ChannelConfigurator configurator = builder -> { };
+    channelBuilder.childChannelConfigurator(configurator);
+    AtomicReference<NameResolver.Args> actualArgs = new AtomicReference<>();
+    channelBuilder.nameResolverRegistry.register(new NameResolverProvider() {
+      @Override
+      public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
+        actualArgs.set(args);
+        NameResolver resolver = mock(NameResolver.class);
+        when(resolver.getServiceAuthority()).thenReturn("test.example.com");
+        return resolver;
+      }
+
+      @Override
+      public String getDefaultScheme() {
+        return expectedUri.getScheme();
+      }
+
+      @Override
+      protected boolean isAvailable() {
+        return true;
+      }
+
+      @Override
+      protected int priority() {
+        return 10;
+      }
+    });
+    createChannel();
+    assertNotNull(actualArgs.get());
+    assertSame(configurator, actualArgs.get().getChildChannelConfigurator());
+  }
+
   @Test
   public void startCallBeforeNameResolution() throws Exception {
     FakeNameResolverFactory nameResolverFactory =

googleapis/src/main/java/io/grpc/googleapis/GoogleCloudToProdNameResolver.java

@@ -24,6 +24,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.CharStreams;
 import com.google.errorprone.annotations.concurrent.GuardedBy;
+import io.grpc.ChannelConfigurator;
 import io.grpc.MetricRecorder;
 import io.grpc.NameResolver;
 import io.grpc.NameResolverRegistry;
@@ -108,6 +109,7 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
   private final Resource<Executor> executorResource;
   private final String target;
   private final MetricRecorder metricRecorder;
+  private final ChannelConfigurator channelConfigurator;
   private final NameResolver delegate;
   private final boolean usingExecutorResource;
   private final boolean forceXds;
@@ -160,6 +162,7 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
     }
     target = targetUri.toString();
     metricRecorder = args.getMetricRecorder();
+    channelConfigurator = args.getChildChannelConfigurator();
     delegate = checkNotNull(nameResolverFactory, "nameResolverFactory").newNameResolver(
         targetUri, args);
     executor = args.getOffloadExecutor();
@@ -211,6 +214,7 @@ private static synchronized BootstrapInfo getBootstrapInfo(boolean isForcedXds)
     targetUri = modifiedTargetBuilder.build();
     target = targetUri.toString();
     metricRecorder = args.getMetricRecorder();
+    channelConfigurator = args.getChildChannelConfigurator();
     delegate =
         checkNotNull(nameResolverFactory, "nameResolverFactory").newNameResolver(targetUri, args);
     executor = args.getOffloadExecutor();
@@ -278,7 +282,7 @@ public void run() {
             public void run() {
               if (!shutdown && finalBootstrapInfo != null) {
                 xdsClientPool = InternalSharedXdsClientPoolProvider.getOrCreate(
-                    target, finalBootstrapInfo, metricRecorder, null);
+                    target, finalBootstrapInfo, metricRecorder, null, channelConfigurator);
                 xdsClient = xdsClientPool.getObject();
                 delegate.start(listener);
                 succeeded = true;

googleapis/src/test/java/io/grpc/googleapis/GoogleCloudToProdNameResolverTest.java

@@ -22,6 +22,7 @@
 import static org.mockito.Mockito.when;
 
 import com.google.common.collect.Iterables;
+import io.grpc.ChannelConfigurator;
 import io.grpc.ChannelLogger;
 import io.grpc.MetricRecorder;
 import io.grpc.NameResolver;
@@ -102,6 +103,7 @@ public void close(Executor instance) {}
   private final Map<String, NameResolver> delegatedResolver = new HashMap<>();
   private final Map<String, URI> delegatedUri = new HashMap<>();
   private final Map<String, Uri> delegatedRfcUri = new HashMap<>();
+  private final Map<String, Args> delegatedArgs = new HashMap<>();
 
   @Mock
   private NameResolver.Listener2 mockListener;
@@ -236,6 +238,22 @@ public void notOnGcpButForceXds_KeyValueTrue_DelegateToXds() {
   }
 
 
+  @Test
+  public void childChannelConfigurator_passedToDelegatedResolver() {
+    GoogleCloudToProdNameResolver.isOnGcp = false;
+    ChannelConfigurator configurator = builder -> { };
+    Args customArgs = args.toBuilder().setChildChannelConfigurator(configurator).build();
+    resolver = enableRfc3986UrisParam
+        ? new GoogleCloudToProdNameResolver(
+            Uri.create(TARGET_URI), customArgs, fakeExecutorResource, nsRegistry.asFactory())
+        : new GoogleCloudToProdNameResolver(
+            URI.create(TARGET_URI), customArgs, fakeExecutorResource, nsRegistry.asFactory());
+    resolver.start(mockListener);
+    assertThat(delegatedArgs.keySet()).containsExactly("dns");
+    assertThat(delegatedArgs.get("dns").getChildChannelConfigurator())
+        .isSameInstanceAs(configurator);
+  }
+
   @Test
   public void notOnGcpButForceXds_WithMultipleParams_DelegateToXds() {
     GoogleCloudToProdNameResolver.isOnGcp = false;
@@ -338,6 +356,7 @@ private FakeNsProvider(String scheme) {
     public NameResolver newNameResolver(URI targetUri, Args args) {
       if (scheme.equals(targetUri.getScheme())) {
         delegatedUri.put(scheme, targetUri);
+        delegatedArgs.put(scheme, args);
         NameResolver resolver = mock(NameResolver.class);
         delegatedResolver.put(scheme, resolver);
         return resolver;
@@ -349,6 +368,7 @@ public NameResolver newNameResolver(URI targetUri, Args args) {
     public NameResolver newNameResolver(Uri targetUri, Args args) {
       if (scheme.equals(targetUri.getScheme())) {
         delegatedRfcUri.put(scheme, targetUri);
+        delegatedArgs.put(scheme, args);
         NameResolver resolver = mock(NameResolver.class);
         delegatedResolver.put(scheme, resolver);
         return resolver;

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

@@ -17,6 +17,7 @@
 package io.grpc.xds;
 
 import io.grpc.CallCredentials;
+import io.grpc.ChannelConfigurator;
 import io.grpc.Internal;
 import io.grpc.MetricRecorder;
 import io.grpc.internal.ObjectPool;
@@ -85,9 +86,15 @@ public static ObjectPool<XdsClient> getOrCreate(
   public static XdsClientResult getOrCreate(
       String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder,
       CallCredentials transportCallCredentials) {
+    return getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, null);
+  }
+
+  public static XdsClientResult getOrCreate(
+      String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder,
+      CallCredentials transportCallCredentials, ChannelConfigurator channelConfigurator) {
     return new XdsClientResult(SharedXdsClientPoolProvider.getDefaultProvider()
         .getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials,
-            null));
+            channelConfigurator));
   }
 
   /**

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

@@ -26,6 +26,7 @@
 import io.grpc.ManagedChannel;
 import io.grpc.ManagedChannelBuilder;
 import io.grpc.opentelemetry.GrpcOpenTelemetry;
+import io.grpc.testing.GrpcCleanupRule;
 import io.grpc.testing.protobuf.SimpleRequest;
 import io.grpc.testing.protobuf.SimpleServiceGrpc;
 import io.opentelemetry.api.OpenTelemetry;
@@ -44,15 +45,19 @@
 
 /**
  * xDS + OpenTelemetry E2E integration test using a fake control plane.
+ * This class is skipped from Bazel builds because Bazel doesn't compile the
+ * grpc-opentelemetry module.
  */
 @RunWith(Parameterized.class)
 public class FakeControlPlaneXdsOtelIntegrationTest {
 
   @Rule(order = 0)
-  public ControlPlaneRule controlPlane = new ControlPlaneRule();
+  public final GrpcCleanupRule grpcCleanupRule = new GrpcCleanupRule();
   @Rule(order = 1)
-  public DataPlaneRule dataPlane = new DataPlaneRule(controlPlane);
+  public final ControlPlaneRule controlPlane = new ControlPlaneRule();
   @Rule(order = 2)
+  public final DataPlaneRule dataPlane = new DataPlaneRule(controlPlane);
+  @Rule(order = 3)
   public final FlagResetRule flagResetRule = new FlagResetRule();
 
   @Parameters(name = "enableRfc3986UrisParam={0}")
@@ -62,6 +67,21 @@ public static Iterable<Object[]> data() {
 
   @Parameter public boolean enableRfc3986UrisParam;
 
+  @Test
+  public void testInMemoryMetricReader() {
+    InMemoryMetricReader metricReader = InMemoryMetricReader.create();
+    SdkMeterProvider meterProvider = SdkMeterProvider.builder()
+        .registerMetricReader(metricReader)
+        .build();
+    io.opentelemetry.api.metrics.LongCounter counter = meterProvider
+        .meterBuilder("test-scope")
+        .build()
+        .counterBuilder("test-counter")
+        .build();
+    counter.add(10);
+    assertThat(metricReader.collectAllMetrics()).isNotEmpty();
+  }
+
   @Before
   public void setupRfc3986UrisFeatureFlag() throws Exception {
     flagResetRule.setFlagForTest(
@@ -88,32 +108,32 @@ public void configureChannelBuilder(ManagedChannelBuilder<?> builder) {
       }
     };
 
-    ManagedChannel channel = Grpc.newChannelBuilder("test-xds:///test-server",
+    ManagedChannel channel = grpcCleanupRule.register(
+        Grpc.newChannelBuilder("test-xds:///test-server",
             InsecureChannelCredentials.create())
         .childChannelConfigurator(configurator)
-        .build();
+        .build());
 
-    try {
-      SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = SimpleServiceGrpc.newBlockingStub(
-          channel);
-      blockingStub.unaryRpc(SimpleRequest.getDefaultInstance());
+    SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub =
+        SimpleServiceGrpc.newBlockingStub(channel);
+    blockingStub.unaryRpc(SimpleRequest.getDefaultInstance());
 
-      boolean hasMetrics = false;
-      for (int i = 0; i < 20; i++) {
-        for (MetricData metric : metricReader.collectAllMetrics()) {
-          if (metric.getName().startsWith("grpc.client.")) {
-            hasMetrics = true;
-            break;
-          }
-        }
-        if (hasMetrics) {
+    // Verify that OpenTelemetry metrics specifically from the xDS Control Plane ADS stream
+    // successfully propagated, method name is recorded as 'other' because dynamic descriptors
+    // are not sampled to local tracing by default.
+    boolean foundXdsMetrics = false;
+    for (int i = 0; i < 50; i++) {
+      for (MetricData metric : metricReader.collectAllMetrics()) {
+        if (metric.toString().contains("grpc.client.") && metric.toString().contains("other")) {
+          foundXdsMetrics = true;
           break;
         }
-        Thread.sleep(100);
       }
-      assertThat(hasMetrics).isTrue();
-    } finally {
-      channel.shutdownNow();
+      if (foundXdsMetrics) {
+        break;
+      }
+      Thread.sleep(100);
     }
+    assertThat(foundXdsMetrics).isTrue();
   }
 }

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

@@ -349,4 +349,15 @@ public void start_passesChannelConfiguratorToClientPoolFactory() throws Exceptio
     verify(mockPoolFactory).getOrCreate(
         any(), any(), any(), eq(configurer));
   }
+
+  @Test
+  public void childChannelConfigurator_nullThrows() throws IOException {
+    buildBuilder(null);
+    try {
+      builder.childChannelConfigurator(null);
+      fail("exception expected");
+    } catch (NullPointerException expected) {
+      assertThat(expected).hasMessageThat().contains("channelConfigurator");
+    }
+  }
 }