From 476e5a8868551ad4be1444f537f45529b141778a Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 22 Dec 2025 14:09:35 +0530 Subject: [PATCH 01/10] enable child channel plugins --- .../java/io/grpc/ChildChannelConfigurer.java | 70 +++++++++++++ .../java/io/grpc/ChildChannelConfigurers.java | 99 +++++++++++++++++++ .../io/grpc/ForwardingChannelBuilder.java | 12 +++ .../io/grpc/ForwardingChannelBuilder2.java | 12 +++ api/src/main/java/io/grpc/ManagedChannel.java | 17 ++++ .../java/io/grpc/ManagedChannelBuilder.java | 35 +++++++ api/src/main/java/io/grpc/MetricRecorder.java | 52 ++++++++++ api/src/main/java/io/grpc/NameResolver.java | 21 ++++ .../internal/ForwardingManagedChannel.java | 6 ++ .../io/grpc/internal/ManagedChannelImpl.java | 41 +++++++- .../internal/ManagedChannelImplBuilder.java | 40 ++++++++ .../io/grpc/xds/GrpcXdsTransportFactory.java | 17 +++- .../InternalSharedXdsClientPoolProvider.java | 2 +- .../grpc/xds/SharedXdsClientPoolProvider.java | 30 ++++-- .../io/grpc/xds/XdsClientPoolFactory.java | 5 + .../java/io/grpc/xds/XdsNameResolver.java | 11 ++- .../java/io/grpc/xds/CsdsServiceTest.java | 8 ++ .../grpc/xds/GrpcXdsClientImplTestBase.java | 2 +- .../grpc/xds/GrpcXdsTransportFactoryTest.java | 2 +- .../io/grpc/xds/LoadReportClientTest.java | 2 +- .../xds/SharedXdsClientPoolProviderTest.java | 2 +- .../io/grpc/xds/XdsClientFallbackTest.java | 6 +- .../java/io/grpc/xds/XdsNameResolverTest.java | 19 ++++ .../java/io/grpc/xds/XdsServerTestHelper.java | 20 ++++ 24 files changed, 509 insertions(+), 22 deletions(-) create mode 100644 api/src/main/java/io/grpc/ChildChannelConfigurer.java create mode 100644 api/src/main/java/io/grpc/ChildChannelConfigurers.java diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurer.java b/api/src/main/java/io/grpc/ChildChannelConfigurer.java new file mode 100644 index 00000000000..fb764509ab1 --- /dev/null +++ b/api/src/main/java/io/grpc/ChildChannelConfigurer.java @@ -0,0 +1,70 @@ +/* + * Copyright 2025 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import java.util.function.Consumer; + +/** + * A configurer for child channels created by gRPC's internal infrastructure. + * + *

This interface allows users to inject configuration (such as credentials, interceptors, + * or flow control settings) into channels created automatically by gRPC for control plane + * operations. Common use cases include: + *

+ * + *

Usage Example: + *

{@code
+ * // 1. Define the configurer
+ * ChildChannelConfigurer configurer = builder -> {
+ *   builder.intercept(new MyAuthInterceptor());
+ *   builder.maxInboundMessageSize(4 * 1024 * 1024);
+ * };
+ *
+ * // 2. Apply to parent channel - automatically used for ALL child channels
+ * ManagedChannel channel = ManagedChannelBuilder
+ *     .forTarget("xds:///my-service")
+ *     .childChannelConfigurer(configurer)
+ *     .build();
+ * }
+ * + *

Implementations must be thread-safe as {@link #accept} may be invoked concurrently + * by multiple internal components. + * + * @since 1.79.0 + */ +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") +@FunctionalInterface +public interface ChildChannelConfigurer extends Consumer> { + + /** + * Configures a builder for a new child channel. + * + *

This method is invoked synchronously during the creation of the child channel, + * before {@link ManagedChannelBuilder#build()} is called. + * + *

Note: The provided {@code builder} is generic (`?`). Implementations should use + * universal configuration methods (like {@code intercept()}, {@code userAgent()}) rather + * than casting to specific implementation types. + * + * @param builder the mutable channel builder for the new child channel + */ + @Override + void accept(ManagedChannelBuilder builder); +} \ No newline at end of file diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurers.java b/api/src/main/java/io/grpc/ChildChannelConfigurers.java new file mode 100644 index 00000000000..03c348b4a77 --- /dev/null +++ b/api/src/main/java/io/grpc/ChildChannelConfigurers.java @@ -0,0 +1,99 @@ +/* + * Copyright 2025 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Utilities for working with {@link ChildChannelConfigurer}. + * + * @since 1.79.0 + */ +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") +public final class ChildChannelConfigurers { + private static final Logger logger = Logger.getLogger(ChildChannelConfigurers.class.getName()); + + // Singleton no-op instance to avoid object churn + private static final ChildChannelConfigurer NO_OP = builder -> { + }; + + private ChildChannelConfigurers() { // Prevent instantiation + } + + /** + * Returns a configurer that does nothing. + * Useful as a default value to avoid null checks in internal code. + */ + public static ChildChannelConfigurer noOp() { + return NO_OP; + } + + /** + * Returns a configurer that applies all the given configurers in sequence. + * + *

If any configurer in the chain throws an exception, the remaining ones are skipped + * (unless wrapped in {@link #safe(ChildChannelConfigurer)}). + * + * @param configurers the configurers to apply in order. Null elements are ignored. + */ + public static ChildChannelConfigurer compose(ChildChannelConfigurer... configurers) { + checkNotNull(configurers, "configurers"); + return builder -> { + for (ChildChannelConfigurer configurer : configurers) { + if (configurer != null) { + configurer.accept(builder); + } + } + }; + } + + /** + * Returns a configurer that applies the delegate but catches and logs any exceptions. + * + *

This prevents a buggy configurer (e.g., one that fails metric setup) from crashing + * the critical path of channel creation. + * + * @param delegate the configurer to wrap. + */ + public static ChildChannelConfigurer safe(ChildChannelConfigurer delegate) { + checkNotNull(delegate, "delegate"); + return builder -> { + try { + delegate.accept(builder); + } catch (Exception e) { + logger.log(Level.WARNING, "Failed to apply child channel configuration", e); + } + }; + } + + /** + * Returns a configurer that applies the delegate only if the given condition is true. + * + *

Useful for applying interceptors only in specific environments (e.g., Debug/Test). + * + * @param condition true to apply the delegate, false to do nothing. + * @param delegate the configurer to apply if condition is true. + */ + public static ChildChannelConfigurer conditional(boolean condition, + ChildChannelConfigurer delegate) { + checkNotNull(delegate, "delegate"); + return condition ? delegate : NO_OP; + } +} \ No newline at end of file diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java index 1202582421a..baedb55acfd 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java @@ -242,6 +242,18 @@ public T disableServiceConfigLookUp() { return thisT(); } + @Override + public T configureChannel(ManagedChannel parentChannel) { + delegate().configureChannel(parentChannel); + return thisT(); + } + + @Override + public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + delegate().childChannelConfigurer(childChannelConfigurer); + return thisT(); + } + /** * Returns the correctly typed version of the builder. */ diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java index 78fe730d91a..7fc02ebd6a7 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java @@ -269,6 +269,18 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { return thisT(); } + @Override + public T configureChannel(ManagedChannel parentChannel) { + delegate().configureChannel(parentChannel); + return thisT(); + } + + @Override + public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + delegate().childChannelConfigurer(childChannelConfigurer); + return thisT(); + } + /** * Returns the {@link ManagedChannel} built by the delegate by default. Overriding method can * return different value. diff --git a/api/src/main/java/io/grpc/ManagedChannel.java b/api/src/main/java/io/grpc/ManagedChannel.java index 7875fdb57f2..7d6914c96b9 100644 --- a/api/src/main/java/io/grpc/ManagedChannel.java +++ b/api/src/main/java/io/grpc/ManagedChannel.java @@ -85,6 +85,23 @@ public ConnectivityState getState(boolean requestConnection) { throw new UnsupportedOperationException("Not implemented"); } + /** + * Returns the configurer for child channels. + * + *

This method is intended for use by the internal gRPC infrastructure (specifically + * load balancers and the channel builder) to propagate configuration to child channels. + * Application code should not call this method. + * + * @return the configurer, or {@code null} if none is set. + * @since 1.79.0 + */ + @Internal + public ChildChannelConfigurer getChildChannelConfigurer() { + // Return null by default so we don't break existing custom ManagedChannel implementations + // (like wrappers or mocks) that don't override this method. + return null; + } + /** * Registers a one-off callback that will be run if the connectivity state of the channel diverges * from the given {@code source}, which is typically what has just been returned by {@link diff --git a/api/src/main/java/io/grpc/ManagedChannelBuilder.java b/api/src/main/java/io/grpc/ManagedChannelBuilder.java index 3f370ab3003..6c4a18516c8 100644 --- a/api/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -661,6 +661,41 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { throw new UnsupportedOperationException(); } + /** + * Configures this builder using settings derived from an existing parent channel. + * + *

This method is typically used by internal components (like LoadBalancers) when creating + * child channels to ensure they inherit relevant configuration (like the + * {@link ChildChannelConfigurer}) from the parent. + * + *

The specific settings copied are implementation dependent, but typically include + * the child channel configurer and potentially user agents or offload executors. + * + * @param parentChannel the channel to inherit configuration from + * @return this + * @since 1.79.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") + public T configureChannel(ManagedChannel parentChannel) { + throw new UnsupportedOperationException(); + } + + /** + * Sets a configurer that will be applied to all internal child channels created by this channel. + * + *

This allows injecting configuration (like credentials, interceptors, or flow control) + * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections + * or OOB load balancing channels. + * + * @param childChannelConfigurer the configurer to apply. + * @return this + * @since 1.79.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") + public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + throw new UnsupportedOperationException("Not implemented"); + } + /** * Builds a channel using the given parameters. * diff --git a/api/src/main/java/io/grpc/MetricRecorder.java b/api/src/main/java/io/grpc/MetricRecorder.java index 897c28011cd..1f765ddc115 100644 --- a/api/src/main/java/io/grpc/MetricRecorder.java +++ b/api/src/main/java/io/grpc/MetricRecorder.java @@ -26,6 +26,15 @@ */ @Internal public interface MetricRecorder { + + /** + * Returns a {@link MetricRecorder} that performs no operations. + * The returned instance ignores all calls and skips all validation checks. + */ + static MetricRecorder noOp() { + return NoOpMetricRecorder.INSTANCE; + } + /** * Adds a value for a double-precision counter metric instrument. * @@ -176,4 +185,47 @@ interface Registration extends AutoCloseable { @Override void close(); } + + /** + * No-Op implementation of MetricRecorder. + * Overrides all default methods to skip validation checks for maximum performance. + */ + final class NoOpMetricRecorder implements MetricRecorder { + private static final NoOpMetricRecorder INSTANCE = new NoOpMetricRecorder(); + + @Override + public void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, double value, + List requiredLabelValues, + List optionalLabelValues) { + } + + @Override + public void addLongCounter(LongCounterMetricInstrument metricInstrument, long value, + List requiredLabelValues, List optionalLabelValues) { + } + + @Override + public void addLongUpDownCounter(LongUpDownCounterMetricInstrument metricInstrument, long value, + List requiredLabelValues, + List optionalLabelValues) { + } + + @Override + public void recordDoubleHistogram(DoubleHistogramMetricInstrument metricInstrument, + double value, List requiredLabelValues, + List optionalLabelValues) { + } + + @Override + public void recordLongHistogram(LongHistogramMetricInstrument metricInstrument, long value, + List requiredLabelValues, + List optionalLabelValues) { + } + + @Override + public Registration registerBatchCallback(BatchCallback callback, + CallbackMetricInstrument... metricInstruments) { + return () -> { }; + } + } } diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index 0e8315e812c..d6e6c563823 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -323,6 +323,7 @@ public static final class Args { @Nullable private final MetricRecorder metricRecorder; @Nullable private final NameResolverRegistry nameResolverRegistry; @Nullable private final IdentityHashMap, Object> customArgs; + @Nullable private final ManagedChannel parentChannel; private Args(Builder builder) { this.defaultPort = checkNotNull(builder.defaultPort, "defaultPort not set"); @@ -337,6 +338,7 @@ private Args(Builder builder) { this.metricRecorder = builder.metricRecorder; this.nameResolverRegistry = builder.nameResolverRegistry; this.customArgs = cloneCustomArgs(builder.customArgs); + this.parentChannel = builder.parentChannel; } /** @@ -435,6 +437,14 @@ public ChannelLogger getChannelLogger() { return channelLogger; } + /** + * Returns the parent {@link ManagedChannel} served by this NameResolver. + */ + @Internal + public ManagedChannel getParentChannel() { + return parentChannel; + } + /** * Returns the Executor on which this resolver should execute long-running or I/O bound work. * Null if no Executor was set. @@ -544,6 +554,7 @@ public static final class Builder { private MetricRecorder metricRecorder; private NameResolverRegistry nameResolverRegistry; private IdentityHashMap, Object> customArgs; + private ManagedChannel parentChannel; Builder() { } @@ -659,6 +670,16 @@ public Builder setNameResolverRegistry(NameResolverRegistry registry) { return this; } + /** + * See {@link Args#parentChannel}. This is an optional field. + * + * @since 1.79.0 + */ + public Builder setParentChannel(ManagedChannel parentChannel) { + this.parentChannel = parentChannel; + return this; + } + /** * Builds an {@link Args}. * diff --git a/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java b/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java index 7ef4ce42e97..ae09785258b 100644 --- a/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java +++ b/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java @@ -18,6 +18,7 @@ import com.google.common.base.MoreObjects; import io.grpc.CallOptions; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ConnectivityState; import io.grpc.ManagedChannel; @@ -92,4 +93,9 @@ public void enterIdle() { public String toString() { return MoreObjects.toStringHelper(this).add("delegate", delegate).toString(); } + + @Override + public ChildChannelConfigurer getChildChannelConfigurer() { + return delegate.getChildChannelConfigurer(); + } } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 849e4b8e45c..2fa2ad8742a 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -40,6 +40,8 @@ import io.grpc.ChannelCredentials; import io.grpc.ChannelLogger; import io.grpc.ChannelLogger.ChannelLogLevel; +import io.grpc.ChildChannelConfigurer; +import io.grpc.ChildChannelConfigurers; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -154,6 +156,15 @@ public Result selectConfig(PickSubchannelArgs args) { private static final LoadBalancer.PickDetailsConsumer NOOP_PICK_DETAILS_CONSUMER = new LoadBalancer.PickDetailsConsumer() {}; + /** + * Stores the user-provided configuration function for internal child channels. + * + *

This is intended for use by gRPC internal components (NameResolvers, LoadBalancers) + * that are responsible for creating auxillary {@code ManagedChannel} instances. + * guaranteed to be not null (defaults to no-op). + */ + private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); + private final InternalLogId logId; private final String target; @Nullable @@ -553,6 +564,9 @@ ClientStream newSubstream( Supplier stopwatchSupplier, List interceptors, final TimeProvider timeProvider) { + if (builder.childChannelConfigurer != null) { + this.childChannelConfigurer = builder.childChannelConfigurer; + } this.target = checkNotNull(builder.target, "target"); this.logId = InternalLogId.allocate("Channel", target); this.timeProvider = checkNotNull(timeProvider, "timeProvider"); @@ -599,7 +613,8 @@ ClientStream newSubstream( .setOffloadExecutor(this.offloadExecutorHolder) .setOverrideAuthority(this.authorityOverride) .setMetricRecorder(this.metricRecorder) - .setNameResolverRegistry(builder.nameResolverRegistry); + .setNameResolverRegistry(builder.nameResolverRegistry) + .setParentChannel(this); builder.copyAllNameResolverCustomArgsTo(nameResolverArgsBuilder); this.nameResolverArgs = nameResolverArgsBuilder.build(); this.nameResolver = getNameResolver( @@ -675,6 +690,19 @@ public CallTracer create() { } } + /** + * Retrieves the user-provided configuration function for internal child channels. + * + *

This method is intended for use by gRPC internal components (NameResolvers, LoadBalancers) + * that are responsible for creating auxiliary {@code ManagedChannel} instances. + * + * @return the ChildChannelConfigurer, guaranteed to be not null (defaults to no-op). + */ + @Override + public ChildChannelConfigurer getChildChannelConfigurer() { + return childChannelConfigurer; + } + @VisibleForTesting static NameResolver getNameResolver( URI targetUri, @Nullable final String overrideAuthority, @@ -1467,7 +1495,11 @@ void onStateChange(InternalSubchannel is, ConnectivityStateInfo newState) { subchannelLogger, transportFilters, target, - lbHelper.getMetricRecorder()); + /* + * TODO(AgraVator): we are breaking the metrics for the internal sub channels of + * OobChannels by passing in MetricRecorder.noOp(). Point this out in the release notes. + */ + MetricRecorder.noOp()); oobChannelTracer.reportEvent(new ChannelTrace.Event.Builder() .setDescription("Child Subchannel created") .setSeverity(ChannelTrace.Event.Severity.CT_INFO) @@ -1558,6 +1590,11 @@ protected ManagedChannelBuilder delegate() { ResolvingOobChannelBuilder builder = new ResolvingOobChannelBuilder(); + // Note that we follow the global configurator pattern and try to fuse the configurations as + // soon as the builder gets created + ManagedChannel parentChannel = ManagedChannelImpl.this; + builder.configureChannel(parentChannel); + return builder // TODO(zdapeng): executors should not outlive the parent channel. .executor(executor) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 1773c04388d..00aa7c62707 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -29,6 +29,7 @@ import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ChannelCredentials; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientTransportFilter; @@ -124,6 +125,8 @@ public static ManagedChannelBuilder forTarget(String target) { private static final Method GET_CLIENT_INTERCEPTOR_METHOD; + ChildChannelConfigurer childChannelConfigurer; + static { Method getClientInterceptorMethod = null; try { @@ -714,6 +717,43 @@ protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { return this; } + /** + * Applies the configuration logic from the given parent channel to this builder. + * + *

+ * This mechanism allows properties (like metrics, tracing, or interceptors) to propagate + * automatically from a parent channel to any child channels it creates + * (e.g., Subchannels or OOB channels). + * + * @param parentChannel the channel whose configuration logic should be applied to this builder. + */ + @Override + public ManagedChannelImplBuilder configureChannel(ManagedChannel parentChannel) { + if (parentChannel != null) { + ChildChannelConfigurer childChannelConfigurer = parentChannel.getChildChannelConfigurer(); + if (childChannelConfigurer != null) { + childChannelConfigurer.accept(this); + } + } + return this; + } + + /** + * Sets the configurer that will be stored in the channel built by this builder. + * + *

+ * This configurer will subsequently be used to configure any descendants (children) + * created by that channel. + * + * @param childChannelConfigurer the configurer to store in the channel. + */ + @Override + public ManagedChannelImplBuilder childChannelConfigurer( + ChildChannelConfigurer childChannelConfigurer) { + this.childChannelConfigurer = childChannelConfigurer; + return this; + } + @Override public ManagedChannel build() { ClientTransportFactory clientTransportFactory = diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index 0da51bf47f7..54d600edee3 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -36,14 +36,16 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { private final CallCredentials callCredentials; + private final ManagedChannel parentChannel; - GrpcXdsTransportFactory(CallCredentials callCredentials) { + GrpcXdsTransportFactory(CallCredentials callCredentials, ManagedChannel parentChannel) { this.callCredentials = callCredentials; + this.parentChannel = parentChannel; } @Override public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { - return new GrpcXdsTransport(serverInfo, callCredentials); + return new GrpcXdsTransport(serverInfo, callCredentials, parentChannel); } @VisibleForTesting @@ -75,6 +77,17 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call this.callCredentials = callCredentials; } + public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials, + ManagedChannel parentChannel) { + String target = serverInfo.target(); + ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig(); + this.channel = Grpc.newChannelBuilder(target, channelCredentials) + .keepAliveTime(5, TimeUnit.MINUTES) + .configureChannel(parentChannel) + .build(); + this.callCredentials = callCredentials; + } + @VisibleForTesting public GrpcXdsTransport(ManagedChannel channel, CallCredentials callCredentials) { this.channel = checkNotNull(channel, "channel"); diff --git a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java index cc5ff128274..1e3200eaef6 100644 --- a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java @@ -86,7 +86,7 @@ public static XdsClientResult getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, CallCredentials transportCallCredentials) { return new XdsClientResult(SharedXdsClientPoolProvider.getDefaultProvider() - .getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials)); + .getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, null)); } /** diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index 45c379244af..a5f3b875f9d 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.grpc.CallCredentials; +import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; @@ -57,6 +58,7 @@ final class SharedXdsClientPoolProvider implements XdsClientPoolFactory { @Nullable private final Bootstrapper bootstrapper; private final Object lock = new Object(); + // The first one wins, anything with the same target string uses the client created for the first one private final Map> targetToXdsClientMap = new ConcurrentHashMap<>(); SharedXdsClientPoolProvider() { @@ -88,20 +90,31 @@ public ObjectPool getOrCreate( } else { bootstrapInfo = GrpcBootstrapperImpl.defaultBootstrap(); } - return getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials); + return getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, + null); } @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder) { - return getOrCreate(target, bootstrapInfo, metricRecorder, null); + return getOrCreate(target, bootstrapInfo, metricRecorder, null, + null); + } + + @Override + public ObjectPool getOrCreate( + String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, + ManagedChannel parentChannel) { + return getOrCreate(target, bootstrapInfo, metricRecorder, null, + parentChannel); } public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - CallCredentials transportCallCredentials) { + CallCredentials transportCallCredentials, + ManagedChannel parentChannel) { ObjectPool ref = targetToXdsClientMap.get(target); if (ref == null) { synchronized (lock) { @@ -109,7 +122,7 @@ public ObjectPool getOrCreate( if (ref == null) { ref = new RefCountedXdsClientObjectPool( - bootstrapInfo, target, metricRecorder, transportCallCredentials); + bootstrapInfo, target, metricRecorder, transportCallCredentials, parentChannel); targetToXdsClientMap.put(target, ref); } } @@ -134,6 +147,7 @@ class RefCountedXdsClientObjectPool implements ObjectPool { private final String target; // The target associated with the xDS client. private final MetricRecorder metricRecorder; private final CallCredentials transportCallCredentials; + private final ManagedChannel parentChannel; private final Object lock = new Object(); @GuardedBy("lock") private ScheduledExecutorService scheduler; @@ -147,7 +161,7 @@ class RefCountedXdsClientObjectPool implements ObjectPool { @VisibleForTesting RefCountedXdsClientObjectPool( BootstrapInfo bootstrapInfo, String target, MetricRecorder metricRecorder) { - this(bootstrapInfo, target, metricRecorder, null); + this(bootstrapInfo, target, metricRecorder, null, null); } @VisibleForTesting @@ -155,11 +169,13 @@ class RefCountedXdsClientObjectPool implements ObjectPool { BootstrapInfo bootstrapInfo, String target, MetricRecorder metricRecorder, - CallCredentials transportCallCredentials) { + CallCredentials transportCallCredentials, + ManagedChannel parentChannel) { this.bootstrapInfo = checkNotNull(bootstrapInfo, "bootstrapInfo"); this.target = target; this.metricRecorder = checkNotNull(metricRecorder, "metricRecorder"); this.transportCallCredentials = transportCallCredentials; + this.parentChannel = parentChannel; } @Override @@ -172,7 +188,7 @@ public XdsClient getObject() { scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE); metricReporter = new XdsClientMetricReporterImpl(metricRecorder, target); GrpcXdsTransportFactory xdsTransportFactory = - new GrpcXdsTransportFactory(transportCallCredentials); + new GrpcXdsTransportFactory(transportCallCredentials, parentChannel); xdsClient = new XdsClientImpl( xdsTransportFactory, diff --git a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java index 6df8d566a7a..4fbd5d7df30 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java @@ -16,6 +16,7 @@ package io.grpc.xds; +import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; import io.grpc.internal.ObjectPool; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; @@ -30,5 +31,9 @@ interface XdsClientPoolFactory { ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder); + ObjectPool getOrCreate( + String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, + ManagedChannel parentChannel); + List getTargets(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 196d51fb5a6..abc4ff09371 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -39,6 +39,7 @@ import io.grpc.InternalConfigSelector; import io.grpc.InternalLogId; import io.grpc.LoadBalancer.PickSubchannelArgs; +import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MetricRecorder; @@ -182,7 +183,8 @@ final class XdsNameResolver extends NameResolver { } else { checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); this.xdsClientPool = new BootstrappingXdsClientPool( - xdsClientPoolFactory, target, bootstrapOverride, metricRecorder); + xdsClientPoolFactory, target, bootstrapOverride, metricRecorder, + nameResolverArgs.getParentChannel()); } this.random = checkNotNull(random, "random"); this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry"); @@ -1054,16 +1056,19 @@ private static final class BootstrappingXdsClientPool implements XdsClientPool { private final @Nullable Map bootstrapOverride; private final @Nullable MetricRecorder metricRecorder; private ObjectPool xdsClientPool; + private ManagedChannel parentChannel; BootstrappingXdsClientPool( XdsClientPoolFactory xdsClientPoolFactory, String target, @Nullable Map bootstrapOverride, - @Nullable MetricRecorder metricRecorder) { + @Nullable MetricRecorder metricRecorder, + @Nullable ManagedChannel parentChannel) { this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); this.target = checkNotNull(target, "target"); this.bootstrapOverride = bootstrapOverride; this.metricRecorder = metricRecorder; + this.parentChannel = parentChannel; } @Override @@ -1076,7 +1081,7 @@ public XdsClient getObject() throws XdsInitializationException { bootstrapInfo = new GrpcBootstrapperImpl().bootstrap(bootstrapOverride); } this.xdsClientPool = - xdsClientPoolFactory.getOrCreate(target, bootstrapInfo, metricRecorder); + xdsClientPoolFactory.getOrCreate(target, bootstrapInfo, metricRecorder, parentChannel); } return xdsClientPool.getObject(); } diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index e8bd7461736..ff5edaded00 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -39,6 +39,7 @@ import io.envoyproxy.envoy.type.matcher.v3.NodeMatcher; import io.grpc.Deadline; import io.grpc.InsecureChannelCredentials; +import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; import io.grpc.Status; import io.grpc.Status.Code; @@ -517,5 +518,12 @@ public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder) { throw new UnsupportedOperationException("Should not be called"); } + + @Override + public ObjectPool getOrCreate( + String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, + ManagedChannel parentChannel) { + throw new UnsupportedOperationException("Should not be called"); + } } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 887923b169b..caad48a54e9 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -5068,7 +5068,7 @@ public void serverFailureMetricReport_forRetryAndBackoff() { private XdsClientImpl createXdsClient(String serverUri) { BootstrapInfo bootstrapInfo = buildBootStrap(serverUri); return new XdsClientImpl( - new GrpcXdsTransportFactory(null), + new GrpcXdsTransportFactory(null, null), bootstrapInfo, fakeClock.getScheduledExecutorService(), backoffPolicyProvider, diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index 66e0d4b3198..93b3be3c9ba 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -92,7 +92,7 @@ public void onCompleted() { @Test public void callApis() throws Exception { XdsTransportFactory.XdsTransport xdsTransport = - new GrpcXdsTransportFactory(null) + new GrpcXdsTransportFactory(null, null) .create( Bootstrapper.ServerInfo.create( "localhost:" + server.getPort(), InsecureChannelCredentials.create())); diff --git a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java index 9bdf86132b6..bfc7db19750 100644 --- a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java +++ b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java @@ -181,7 +181,7 @@ public void cancelled(Context context) { lrsClient = new LoadReportClient( loadStatsManager, - new GrpcXdsTransportFactory(null).createForTest(channel), + new GrpcXdsTransportFactory(null, null).createForTest(channel), NODE, syncContext, fakeClock.getScheduledExecutorService(), diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index 29b149f166f..9f091a79ff9 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -207,7 +207,7 @@ public void xdsClient_usesCallCredentials() throws Exception { // Create xDS client that uses the CallCredentials on the transport ObjectPool xdsClientPool = - provider.getOrCreate("target", bootstrapInfo, metricRecorder, sampleCreds); + provider.getOrCreate("target", bootstrapInfo, metricRecorder, sampleCreds, null); XdsClient xdsClient = xdsClientPool.getObject(); xdsClient.watchXdsResource( XdsListenerResource.getInstance(), "someLDSresource", ldsResourceWatcher); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java index 27ee8d22825..4d5e7d09ad4 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java @@ -484,7 +484,7 @@ public void fallbackFromBadUrlToGoodOne() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(garbageUri, validUri), - new GrpcXdsTransportFactory(null), + new GrpcXdsTransportFactory(null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, @@ -509,7 +509,7 @@ public void testGoodUrlFollowedByBadUrl() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(validUri, garbageUri), - new GrpcXdsTransportFactory(null), + new GrpcXdsTransportFactory(null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, @@ -536,7 +536,7 @@ public void testTwoBadUrl() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(garbageUri1, garbageUri2), - new GrpcXdsTransportFactory(null), + new GrpcXdsTransportFactory(null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 45a96ee172f..95c4cde279a 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -57,6 +57,7 @@ import io.grpc.InternalConfigSelector.Result; import io.grpc.LoadBalancer.PickDetailsConsumer; import io.grpc.LoadBalancer.PickSubchannelArgs; +import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; @@ -2493,6 +2494,24 @@ public XdsClient returnObject(Object object) { }; } + @Override + public ObjectPool getOrCreate( + String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, + ManagedChannel parentChannel) { + targets.add(target); + return new ObjectPool() { + @Override + public XdsClient getObject() { + return xdsClient; + } + + @Override + public XdsClient returnObject(Object object) { + return null; + } + }; + } + @Override public List getTargets() { if (targets.isEmpty()) { diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index 386793299d8..bc300a4ef68 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -23,6 +23,7 @@ import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; import io.grpc.InsecureChannelCredentials; +import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; import io.grpc.Status; import io.grpc.StatusOr; @@ -182,6 +183,25 @@ public XdsClient returnObject(Object object) { }; } + @Override + public ObjectPool getOrCreate( + String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, + ManagedChannel parentChannel) { + this.savedBootstrapInfo = bootstrapInfo; + return new ObjectPool() { + @Override + public XdsClient getObject() { + return xdsClient; + } + + @Override + public XdsClient returnObject(Object object) { + xdsClient.shutdown(); + return null; + } + }; + } + @Override public List getTargets() { return Collections.singletonList("fake-target"); From 2554a04d9305b50e878e94419096eb5449e1b3c7 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 22 Dec 2025 16:53:50 +0530 Subject: [PATCH 02/10] add test cases --- .../java/io/grpc/ChildChannelConfigurer.java | 2 +- .../java/io/grpc/ChildChannelConfigurers.java | 2 +- .../io/grpc/ChildChannelConfigurersTest.java | 58 +++++++++ .../test/java/io/grpc/NameResolverTest.java | 27 ++++ .../grpc/internal/ManagedChannelImplTest.java | 120 +++++++++++++++++- .../grpc/internal/MetricRecorderImplTest.java | 22 ++++ .../grpc/xds/SharedXdsClientPoolProvider.java | 5 +- 7 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 api/src/test/java/io/grpc/ChildChannelConfigurersTest.java diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurer.java b/api/src/main/java/io/grpc/ChildChannelConfigurer.java index fb764509ab1..36b67098797 100644 --- a/api/src/main/java/io/grpc/ChildChannelConfigurer.java +++ b/api/src/main/java/io/grpc/ChildChannelConfigurer.java @@ -67,4 +67,4 @@ public interface ChildChannelConfigurer extends Consumer builder); -} \ No newline at end of file +} diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurers.java b/api/src/main/java/io/grpc/ChildChannelConfigurers.java index 03c348b4a77..92508f5e71c 100644 --- a/api/src/main/java/io/grpc/ChildChannelConfigurers.java +++ b/api/src/main/java/io/grpc/ChildChannelConfigurers.java @@ -96,4 +96,4 @@ public static ChildChannelConfigurer conditional(boolean condition, checkNotNull(delegate, "delegate"); return condition ? delegate : NO_OP; } -} \ No newline at end of file +} diff --git a/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java b/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java new file mode 100644 index 00000000000..08ebcbc6e08 --- /dev/null +++ b/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2025 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ChildChannelConfigurersTest { + + @Test + public void noOp_doesNothing() { + ManagedChannelBuilder builder = mock(ManagedChannelBuilder.class); + ChildChannelConfigurers.noOp().accept(builder); + verifyNoInteractions(builder); + } + + @Test + public void compose_runsInOrder() { + ManagedChannelBuilder builder = mock(ManagedChannelBuilder.class); + ChildChannelConfigurer configurer1 = b -> b.userAgent("agent1"); + ChildChannelConfigurer configurer2 = b -> b.maxInboundMessageSize(123); + + ChildChannelConfigurers.compose(configurer1, configurer2).accept(builder); + + verify(builder).userAgent("agent1"); + verify(builder).maxInboundMessageSize(123); + } + + @Test + public void compose_ignoresNulls() { + ManagedChannelBuilder builder = mock(ManagedChannelBuilder.class); + ChildChannelConfigurer configurer = b -> b.userAgent("agent1"); + + ChildChannelConfigurers.compose(null, configurer, null).accept(builder); + + verify(builder).userAgent("agent1"); + } +} \ No newline at end of file diff --git a/api/src/test/java/io/grpc/NameResolverTest.java b/api/src/test/java/io/grpc/NameResolverTest.java index 82abe5c7505..ead5c462286 100644 --- a/api/src/test/java/io/grpc/NameResolverTest.java +++ b/api/src/test/java/io/grpc/NameResolverTest.java @@ -105,6 +105,7 @@ public void args() { } private NameResolver.Args createArgs() { + ManagedChannel parent = mock(ManagedChannel.class); return NameResolver.Args.newBuilder() .setDefaultPort(defaultPort) .setProxyDetector(proxyDetector) @@ -116,9 +117,35 @@ private NameResolver.Args createArgs() { .setOverrideAuthority(overrideAuthority) .setMetricRecorder(metricRecorder) .setArg(FOO_ARG_KEY, customArgValue) + .setParentChannel(parent) .build(); } + @Test + public void args_parentChannel() { + ManagedChannel parent = mock(ManagedChannel.class); + + // Create a real SynchronizationContext instead of mocking it + SynchronizationContext realSyncContext = new SynchronizationContext( + new Thread.UncaughtExceptionHandler() { + @Override + public void uncaughtException(Thread t, Throwable e) { + throw new AssertionError(e); + } + }); + + NameResolver.Args args = NameResolver.Args.newBuilder() + .setDefaultPort(8080) + .setProxyDetector(mock(ProxyDetector.class)) + .setSynchronizationContext(realSyncContext) + .setServiceConfigParser(mock(NameResolver.ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) + .setParentChannel(parent) + .build(); + + assertThat(args.getParentChannel()).isSameInstanceAs(parent); + } + @Test @SuppressWarnings("deprecation") public void startOnOldListener_wrapperListener2UsedToStart() { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 91a9f506bc8..a7214049897 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -69,6 +69,7 @@ import io.grpc.Channel; import io.grpc.ChannelCredentials; import io.grpc.ChannelLogger; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -101,10 +102,12 @@ import io.grpc.LoadBalancerRegistry; import io.grpc.LongCounterMetricInstrument; import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; import io.grpc.MetricInstrumentRegistry; +import io.grpc.MetricRecorder; import io.grpc.MetricSink; import io.grpc.NameResolver; import io.grpc.NameResolver.ConfigOrError; @@ -302,9 +305,12 @@ public String getPolicyName() { private boolean panicExpected; @Captor private ArgumentCaptor resolvedAddressCaptor; - private ArgumentCaptor streamListenerCaptor = ArgumentCaptor.forClass(ClientStreamListener.class); + @Mock + private ChildChannelConfigurer mockChildChannelConfigurer; + @Mock + private MetricRecorder mockMetricRecorder; private void createChannel(ClientInterceptor... interceptors) { createChannel(false, interceptors); @@ -5139,4 +5145,116 @@ private static ManagedChannelServiceConfig createManagedChannelServiceConfig( return ManagedChannelServiceConfig .fromServiceConfig(rawServiceConfig, true, 3, 4, policySelection); } + + @Test + public void getChildChannelConfigurer_returnsConfiguredValue() { + ManagedChannelImplBuilder builder = new ManagedChannelImplBuilder(TARGET, + new ClientTransportFactoryBuilder() { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return mockTransportFactory; + } + }, + new FixedPortProvider(DEFAULT_PORT)); + + when(mockTransportFactory.getSupportedSocketAddressTypes()) + .thenReturn(Collections.singleton(InetSocketAddress.class)); + + ManagedChannel channel = builder + .nameResolverFactory(new FakeNameResolverFactory( + Collections.singletonList(URI.create(TARGET)), + Collections.emptyList(), + true, + null)) + .childChannelConfigurer(mockChildChannelConfigurer) + .build(); + + assertThat(channel.getChildChannelConfigurer()).isSameInstanceAs(mockChildChannelConfigurer); + channel.shutdownNow(); + } + + @Test + public void configureChannel_propagatesConfigurerToNewBuilder_endToEnd() { + when(mockTransportFactory.getSupportedSocketAddressTypes()) + .thenReturn(Collections.singleton(InetSocketAddress.class)); + + // 1. Setup Interceptor + final AtomicInteger interceptorCalls = new AtomicInteger(0); + final ClientInterceptor trackingInterceptor = new ClientInterceptor() { + @Override + public ClientCall interceptCall( + MethodDescriptor method, CallOptions callOptions, Channel next) { + interceptorCalls.incrementAndGet(); + return next.newCall(method, callOptions); + } + }; + + // 2. Setup Configurer + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void accept(ManagedChannelBuilder builder) { + builder.intercept(trackingInterceptor); + } + }; + + // 3. Create Parent Channel + ManagedChannelImplBuilder parentBuilder = new ManagedChannelImplBuilder("fake://parent-target", + new ClientTransportFactoryBuilder() { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return mockTransportFactory; + } + }, + new FixedPortProvider(DEFAULT_PORT)); + + ManagedChannel parentChannel = parentBuilder + // MATCH THE CONSTRUCTOR SIGNATURE: (List, List, boolean, Status) + .nameResolverFactory(new FakeNameResolverFactory( + Collections.singletonList(URI.create("fake://parent-target")), + Collections.emptyList(), + true, + null)) + .childChannelConfigurer(configurer) + .build(); + + // 4. Create Child Channel Builder + ManagedChannelImplBuilder childBuilder = new ManagedChannelImplBuilder("fake://child-target", + new ClientTransportFactoryBuilder() { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return mockTransportFactory; + } + }, + new FixedPortProvider(DEFAULT_PORT)); + + childBuilder.configureChannel(parentChannel); + + // Ensure child also has a resolver factory + childBuilder.nameResolverFactory(new FakeNameResolverFactory( + Collections.singletonList(URI.create("fake://child-target")), + Collections.emptyList(), + true, + null)); + + ManagedChannel childChannel = childBuilder.build(); + + // 5. Verification + ClientCall call = childChannel.newCall( + MethodDescriptor.newBuilder() + .setType(MethodDescriptor.MethodType.UNARY) + .setFullMethodName("service/method") + .setRequestMarshaller(new StringMarshaller()) + .setResponseMarshaller(new IntegerMarshaller()) + .build(), + CallOptions.DEFAULT); + + call.start(new ClientCall.Listener() { + }, new Metadata()); + + assertThat(interceptorCalls.get()) + .isEqualTo(1); + + childChannel.shutdownNow(); + parentChannel.shutdownNow(); + } } diff --git a/core/src/test/java/io/grpc/internal/MetricRecorderImplTest.java b/core/src/test/java/io/grpc/internal/MetricRecorderImplTest.java index 33bf9bb41e2..715b09a6cc4 100644 --- a/core/src/test/java/io/grpc/internal/MetricRecorderImplTest.java +++ b/core/src/test/java/io/grpc/internal/MetricRecorderImplTest.java @@ -16,6 +16,7 @@ package io.grpc.internal; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; @@ -326,4 +327,25 @@ public void recordLongGaugeMismatchedOptionalLabelValues() { callbackCaptor.getValue().run(); registration.close(); } + + @Test + public void noOp_returnsSingleton() { + assertThat(MetricRecorder.noOp()).isSameInstanceAs(MetricRecorder.noOp()); + } + + @Test + public void noOp_methodsDoNotThrow() { + MetricRecorder recorder = MetricRecorder.noOp(); + + recorder.addDoubleCounter(null, 1.0, + null, null); + recorder.addLongCounter(null, 1, + null, null); + recorder.addLongUpDownCounter(null, 1, + null, null); + recorder.recordDoubleHistogram(null, 1.0, + null, null); + recorder.recordLongHistogram(null, 1, + null, null); + } } diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index a5f3b875f9d..a1705610300 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -58,7 +58,10 @@ final class SharedXdsClientPoolProvider implements XdsClientPoolFactory { @Nullable private final Bootstrapper bootstrapper; private final Object lock = new Object(); - // The first one wins, anything with the same target string uses the client created for the first one + /* + The first one wins. + Anything with the same target string uses the client created for the first one. + */ private final Map> targetToXdsClientMap = new ConcurrentHashMap<>(); SharedXdsClientPoolProvider() { From cf4909ecb09469e29fdeab1f8c0dbc0f6126f72b Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 22 Dec 2025 17:27:24 +0530 Subject: [PATCH 03/10] remove unused field --- .../test/java/io/grpc/internal/ManagedChannelImplTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index a7214049897..8312e15d98d 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -107,7 +107,6 @@ import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; import io.grpc.MetricInstrumentRegistry; -import io.grpc.MetricRecorder; import io.grpc.MetricSink; import io.grpc.NameResolver; import io.grpc.NameResolver.ConfigOrError; @@ -309,8 +308,7 @@ public String getPolicyName() { ArgumentCaptor.forClass(ClientStreamListener.class); @Mock private ChildChannelConfigurer mockChildChannelConfigurer; - @Mock - private MetricRecorder mockMetricRecorder; + private void createChannel(ClientInterceptor... interceptors) { createChannel(false, interceptors); From 4905f728081c723a24d0f74d2c04a5d862351297 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 22 Dec 2025 21:48:33 +0530 Subject: [PATCH 04/10] correct javadoc --- .../java/io/grpc/internal/ManagedChannelImplBuilder.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 00aa7c62707..8cc588f2117 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -720,8 +720,7 @@ protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { /** * Applies the configuration logic from the given parent channel to this builder. * - *

- * This mechanism allows properties (like metrics, tracing, or interceptors) to propagate + *

This mechanism allows properties (like metrics, tracing, or interceptors) to propagate * automatically from a parent channel to any child channels it creates * (e.g., Subchannels or OOB channels). * @@ -741,8 +740,7 @@ public ManagedChannelImplBuilder configureChannel(ManagedChannel parentChannel) /** * Sets the configurer that will be stored in the channel built by this builder. * - *

- * This configurer will subsequently be used to configure any descendants (children) + *

This configurer will subsequently be used to configure any descendants (children) * created by that channel. * * @param childChannelConfigurer the configurer to store in the channel. From 36c2927e75ffb45e688bbb1cb63cd5c90db3304a Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 29 Dec 2025 17:13:14 +0530 Subject: [PATCH 05/10] plumb child channel options through xDS server --- .../io/grpc/ForwardingChannelBuilder.java | 6 +++ .../io/grpc/ForwardingChannelBuilder2.java | 6 +++ .../java/io/grpc/ForwardingServerBuilder.java | 12 +++++ api/src/main/java/io/grpc/ManagedChannel.java | 6 +-- .../java/io/grpc/ManagedChannelBuilder.java | 22 ++++++-- api/src/main/java/io/grpc/Server.java | 17 +++++++ api/src/main/java/io/grpc/ServerBuilder.java | 33 ++++++++++++ .../io/grpc/internal/ManagedChannelImpl.java | 6 +-- .../internal/ManagedChannelImplBuilder.java | 25 ++++++++- .../io/grpc/xds/GrpcXdsTransportFactory.java | 24 ++++++--- .../InternalSharedXdsClientPoolProvider.java | 3 +- .../grpc/xds/SharedXdsClientPoolProvider.java | 25 +++++---- .../io/grpc/xds/XdsClientPoolFactory.java | 3 +- .../java/io/grpc/xds/XdsNameResolver.java | 3 +- .../java/io/grpc/xds/XdsServerBuilder.java | 20 +++++++- .../java/io/grpc/xds/XdsServerWrapper.java | 51 ++++++++++++++++++- .../java/io/grpc/xds/CsdsServiceTest.java | 3 +- .../grpc/xds/GrpcXdsClientImplTestBase.java | 2 +- .../grpc/xds/GrpcXdsTransportFactoryTest.java | 2 +- .../io/grpc/xds/LoadReportClientTest.java | 2 +- .../xds/SharedXdsClientPoolProviderTest.java | 3 +- .../io/grpc/xds/XdsClientFallbackTest.java | 6 +-- .../java/io/grpc/xds/XdsNameResolverTest.java | 3 +- .../java/io/grpc/xds/XdsServerTestHelper.java | 3 +- 24 files changed, 243 insertions(+), 43 deletions(-) diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java index baedb55acfd..44facd8796b 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java @@ -248,6 +248,12 @@ public T configureChannel(ManagedChannel parentChannel) { return thisT(); } + @Override + public T configureChannel(Server parentServer) { + delegate().configureChannel(parentServer); + return thisT(); + } + @Override public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { delegate().childChannelConfigurer(childChannelConfigurer); diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java index 7fc02ebd6a7..aed2ff8f45c 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java @@ -275,6 +275,12 @@ public T configureChannel(ManagedChannel parentChannel) { return thisT(); } + @Override + public T configureChannel(Server parentServer) { + delegate().configureChannel(parentServer); + return thisT(); + } + @Override public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { delegate().childChannelConfigurer(childChannelConfigurer); diff --git a/api/src/main/java/io/grpc/ForwardingServerBuilder.java b/api/src/main/java/io/grpc/ForwardingServerBuilder.java index 9cef7cfa331..d7f5be03ea2 100644 --- a/api/src/main/java/io/grpc/ForwardingServerBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingServerBuilder.java @@ -192,6 +192,18 @@ public T setBinaryLog(BinaryLog binaryLog) { return thisT(); } + @Override + public T configureChannel(Server parentServer) { + delegate().configureChannel(parentServer); + return thisT(); + } + + @Override + public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + delegate().childChannelConfigurer(childChannelConfigurer); + return thisT(); + } + /** * Returns the {@link Server} built by the delegate by default. Overriding method can return * different value. diff --git a/api/src/main/java/io/grpc/ManagedChannel.java b/api/src/main/java/io/grpc/ManagedChannel.java index 7d6914c96b9..a2498097833 100644 --- a/api/src/main/java/io/grpc/ManagedChannel.java +++ b/api/src/main/java/io/grpc/ManagedChannel.java @@ -92,14 +92,14 @@ public ConnectivityState getState(boolean requestConnection) { * load balancers and the channel builder) to propagate configuration to child channels. * Application code should not call this method. * - * @return the configurer, or {@code null} if none is set. + * @return the configurer, or {@code noOp()} if none is set. * @since 1.79.0 */ @Internal public ChildChannelConfigurer getChildChannelConfigurer() { - // Return null by default so we don't break existing custom ManagedChannel implementations + // Return noOP() by default so we don't break existing custom ManagedChannel implementations // (like wrappers or mocks) that don't override this method. - return null; + return ChildChannelConfigurers.noOp(); } /** diff --git a/api/src/main/java/io/grpc/ManagedChannelBuilder.java b/api/src/main/java/io/grpc/ManagedChannelBuilder.java index 6c4a18516c8..b4211c504c0 100644 --- a/api/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -668,9 +668,6 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { * child channels to ensure they inherit relevant configuration (like the * {@link ChildChannelConfigurer}) from the parent. * - *

The specific settings copied are implementation dependent, but typically include - * the child channel configurer and potentially user agents or offload executors. - * * @param parentChannel the channel to inherit configuration from * @return this * @since 1.79.0 @@ -680,12 +677,27 @@ public T configureChannel(ManagedChannel parentChannel) { throw new UnsupportedOperationException(); } + /** + * Configures this builder using settings derived from an existing parent server. + * + *

This method is typically used by internal components (like LoadBalancers) when creating + * child channels to ensure they inherit relevant configuration (like the + * {@link ChildChannelConfigurer}) from the parent. + * + * @param parentServer the server to inherit configuration from + * @return this + * @since 1.79.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") + public T configureChannel(Server parentServer) { + throw new UnsupportedOperationException(); + } + /** * Sets a configurer that will be applied to all internal child channels created by this channel. * *

This allows injecting configuration (like credentials, interceptors, or flow control) - * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections - * or OOB load balancing channels. + * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections. * * @param childChannelConfigurer the configurer to apply. * @return this diff --git a/api/src/main/java/io/grpc/Server.java b/api/src/main/java/io/grpc/Server.java index 97ea06a81c2..92868cb6569 100644 --- a/api/src/main/java/io/grpc/Server.java +++ b/api/src/main/java/io/grpc/Server.java @@ -178,4 +178,21 @@ public List getMutableServices() { * @since 1.0.0 */ public abstract void awaitTermination() throws InterruptedException; + + /** + * Returns the configurer for child channels. + * + *

This method is intended for use by the internal gRPC infrastructure + * to propagate configuration to child channels. + * Application code should not call this method. + * + * @return the configurer, or {@code noOp()} if none is set. + * @since 1.79.0 + */ + @Internal + public ChildChannelConfigurer getChildChannelConfigurer() { + // Return noOp() by default so we don't break existing custom ManagedChannel implementations + // (like wrappers or mocks) that don't override this method. + return ChildChannelConfigurers.noOp(); + } } diff --git a/api/src/main/java/io/grpc/ServerBuilder.java b/api/src/main/java/io/grpc/ServerBuilder.java index cd1cddbb93f..7f929dba15b 100644 --- a/api/src/main/java/io/grpc/ServerBuilder.java +++ b/api/src/main/java/io/grpc/ServerBuilder.java @@ -424,6 +424,39 @@ public T setBinaryLog(BinaryLog binaryLog) { throw new UnsupportedOperationException(); } + /** + * Configures this builder using settings derived from an existing parent server. + * + *

This method is typically used by internal components when creating + * child channels to ensure they inherit relevant configuration (like the + * {@link ChildChannelConfigurer}) from the parent. + * + * @param parentServer the server to inherit configuration from + * @return this + * @since 1.79.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") + public T configureChannel(Server parentServer) { + throw new UnsupportedOperationException(); + } + + /** + * Sets a configurer that will be applied to all internal child channels created by this server. + * + *

This allows injecting configuration (like credentials, interceptors, or flow control) + * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections + * or OOB load balancing channels. + * + * @param childChannelConfigurer the configurer to apply. + * @return this + * @since 1.79.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") + public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + throw new UnsupportedOperationException("Not implemented"); + } + + /** * Builds a server using the given parameters. * diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 2fa2ad8742a..016ad96daa6 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -159,8 +159,8 @@ public Result selectConfig(PickSubchannelArgs args) { /** * Stores the user-provided configuration function for internal child channels. * - *

This is intended for use by gRPC internal components (NameResolvers, LoadBalancers) - * that are responsible for creating auxillary {@code ManagedChannel} instances. + *

This is intended for use by gRPC internal components + * that are responsible for creating auxiliary {@code ManagedChannel} instances. * guaranteed to be not null (defaults to no-op). */ private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); @@ -693,7 +693,7 @@ public CallTracer create() { /** * Retrieves the user-provided configuration function for internal child channels. * - *

This method is intended for use by gRPC internal components (NameResolvers, LoadBalancers) + *

This method is intended for use by gRPC internal components * that are responsible for creating auxiliary {@code ManagedChannel} instances. * * @return the ChildChannelConfigurer, guaranteed to be not null (defaults to no-op). diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 8cc588f2117..bcbe6e63054 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -46,6 +46,7 @@ import io.grpc.NameResolverProvider; import io.grpc.NameResolverRegistry; import io.grpc.ProxyDetector; +import io.grpc.Server; import io.grpc.StatusOr; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -724,7 +725,8 @@ protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { * automatically from a parent channel to any child channels it creates * (e.g., Subchannels or OOB channels). * - * @param parentChannel the channel whose configuration logic should be applied to this builder. + * @param parentChannel the channel whose child's configuration logic + * should be applied to this builder. */ @Override public ManagedChannelImplBuilder configureChannel(ManagedChannel parentChannel) { @@ -737,6 +739,27 @@ public ManagedChannelImplBuilder configureChannel(ManagedChannel parentChannel) return this; } + /** + * Applies the configuration logic from the given parent server to this builder. + * + *

This mechanism allows properties (like metrics, tracing, or interceptors) to propagate + * automatically from a parent server to any child channels it creates + * (e.g., xDS). + * + * @param parentServer the server whose child's configuration logic + * should be applied to this builder. + */ + @Override + public ManagedChannelImplBuilder configureChannel(Server parentServer) { + if (parentServer != null) { + ChildChannelConfigurer childChannelConfigurer = parentServer.getChildChannelConfigurer(); + if (childChannelConfigurer != null) { + childChannelConfigurer.accept(this); + } + } + return this; + } + /** * Sets the configurer that will be stored in the channel built by this builder. * diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index 54d600edee3..f41ff62dc7a 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -26,8 +26,10 @@ import io.grpc.Context; import io.grpc.Grpc; import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.Server; import io.grpc.Status; import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.XdsTransportFactory; @@ -37,15 +39,19 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { private final CallCredentials callCredentials; private final ManagedChannel parentChannel; + private final Server parentServer; - GrpcXdsTransportFactory(CallCredentials callCredentials, ManagedChannel parentChannel) { + + GrpcXdsTransportFactory(CallCredentials callCredentials, ManagedChannel parentChannel, + Server parentServer) { this.callCredentials = callCredentials; this.parentChannel = parentChannel; + this.parentServer = parentServer; } @Override public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { - return new GrpcXdsTransport(serverInfo, callCredentials, parentChannel); + return new GrpcXdsTransport(serverInfo, callCredentials, parentChannel, parentServer); } @VisibleForTesting @@ -78,13 +84,17 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call } public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, Server parentServer) { String target = serverInfo.target(); ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig(); - this.channel = Grpc.newChannelBuilder(target, channelCredentials) - .keepAliveTime(5, TimeUnit.MINUTES) - .configureChannel(parentChannel) - .build(); + ManagedChannelBuilder channelBuilder = Grpc.newChannelBuilder(target, channelCredentials) + .keepAliveTime(5, TimeUnit.MINUTES); + if (parentChannel != null) { + channelBuilder.configureChannel(parentChannel); + } else if (parentServer != null) { + channelBuilder.configureChannel(parentServer); + } + this.channel = channelBuilder.build(); this.callCredentials = callCredentials; } diff --git a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java index 1e3200eaef6..2855a9a7321 100644 --- a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java @@ -86,7 +86,8 @@ public static XdsClientResult getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, CallCredentials transportCallCredentials) { return new XdsClientResult(SharedXdsClientPoolProvider.getDefaultProvider() - .getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, null)); + .getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, + null, null)); } /** diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index a1705610300..fe08b594420 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -24,6 +24,7 @@ import io.grpc.CallCredentials; import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; +import io.grpc.Server; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; @@ -94,22 +95,22 @@ public ObjectPool getOrCreate( bootstrapInfo = GrpcBootstrapperImpl.defaultBootstrap(); } return getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, - null); + null, null); } @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder) { return getOrCreate(target, bootstrapInfo, metricRecorder, null, - null); + null, null); } @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, Server parentServer) { return getOrCreate(target, bootstrapInfo, metricRecorder, null, - parentChannel); + null, parentServer); } public ObjectPool getOrCreate( @@ -117,7 +118,8 @@ public ObjectPool getOrCreate( BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, CallCredentials transportCallCredentials, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, + Server parentServer) { ObjectPool ref = targetToXdsClientMap.get(target); if (ref == null) { synchronized (lock) { @@ -125,7 +127,8 @@ public ObjectPool getOrCreate( if (ref == null) { ref = new RefCountedXdsClientObjectPool( - bootstrapInfo, target, metricRecorder, transportCallCredentials, parentChannel); + bootstrapInfo, target, metricRecorder, transportCallCredentials, parentChannel, + parentServer); targetToXdsClientMap.put(target, ref); } } @@ -151,6 +154,7 @@ class RefCountedXdsClientObjectPool implements ObjectPool { private final MetricRecorder metricRecorder; private final CallCredentials transportCallCredentials; private final ManagedChannel parentChannel; + private final Server parentServer; private final Object lock = new Object(); @GuardedBy("lock") private ScheduledExecutorService scheduler; @@ -164,7 +168,8 @@ class RefCountedXdsClientObjectPool implements ObjectPool { @VisibleForTesting RefCountedXdsClientObjectPool( BootstrapInfo bootstrapInfo, String target, MetricRecorder metricRecorder) { - this(bootstrapInfo, target, metricRecorder, null, null); + this(bootstrapInfo, target, metricRecorder, null, null, + null); } @VisibleForTesting @@ -173,12 +178,14 @@ class RefCountedXdsClientObjectPool implements ObjectPool { String target, MetricRecorder metricRecorder, CallCredentials transportCallCredentials, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, + Server parentServer) { this.bootstrapInfo = checkNotNull(bootstrapInfo, "bootstrapInfo"); this.target = target; this.metricRecorder = checkNotNull(metricRecorder, "metricRecorder"); this.transportCallCredentials = transportCallCredentials; this.parentChannel = parentChannel; + this.parentServer = parentServer; } @Override @@ -191,7 +198,7 @@ public XdsClient getObject() { scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE); metricReporter = new XdsClientMetricReporterImpl(metricRecorder, target); GrpcXdsTransportFactory xdsTransportFactory = - new GrpcXdsTransportFactory(transportCallCredentials, parentChannel); + new GrpcXdsTransportFactory(transportCallCredentials, parentChannel, parentServer); xdsClient = new XdsClientImpl( xdsTransportFactory, diff --git a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java index 4fbd5d7df30..5d9c63301c7 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java @@ -18,6 +18,7 @@ import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; +import io.grpc.Server; import io.grpc.internal.ObjectPool; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; import io.grpc.xds.client.XdsClient; @@ -33,7 +34,7 @@ ObjectPool getOrCreate( ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel); + ManagedChannel parentChannel, Server parentServer); List getTargets(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index abc4ff09371..6f60008151d 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -1081,7 +1081,8 @@ public XdsClient getObject() throws XdsInitializationException { bootstrapInfo = new GrpcBootstrapperImpl().bootstrap(bootstrapOverride); } this.xdsClientPool = - xdsClientPoolFactory.getOrCreate(target, bootstrapInfo, metricRecorder, parentChannel); + xdsClientPoolFactory.getOrCreate(target, bootstrapInfo, metricRecorder, parentChannel, + null); } return xdsClientPool.getObject(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index 4a4fb71aa84..bbfa12eb26f 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -25,6 +25,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.errorprone.annotations.DoNotCall; import io.grpc.Attributes; +import io.grpc.ChildChannelConfigurer; +import io.grpc.ChildChannelConfigurers; import io.grpc.ExperimentalApi; import io.grpc.ForwardingServerBuilder; import io.grpc.Internal; @@ -58,6 +60,7 @@ public final class XdsServerBuilder extends ForwardingServerBuilder bootstrapOverride; private long drainGraceTime = 10; private TimeUnit drainGraceTimeUnit = TimeUnit.MINUTES; + private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); private XdsServerBuilder(NettyServerBuilder nettyDelegate, int port) { this.delegate = nettyDelegate; @@ -100,6 +103,20 @@ public XdsServerBuilder drainGraceTime(long drainGraceTime, TimeUnit drainGraceT return this; } + /** + * Sets the configurer that will be stored in the server built by this builder. + * + *

This configurer will subsequently be used to configure any child channels + * created by that server. + * + * @param childChannelConfigurer the configurer to store in the channel. + */ + @Override + public XdsServerBuilder childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + this.childChannelConfigurer = childChannelConfigurer; + return this; + } + @DoNotCall("Unsupported. Use forPort(int, ServerCredentials) instead") public static ServerBuilder forPort(int port) { throw new UnsupportedOperationException( @@ -128,7 +145,8 @@ public Server build() { } InternalNettyServerBuilder.eagAttributes(delegate, builder.build()); return new XdsServerWrapper("0.0.0.0:" + port, delegate, xdsServingStatusListener, - filterChainSelectorManager, xdsClientPoolFactory, bootstrapOverride, filterRegistry); + filterChainSelectorManager, xdsClientPoolFactory, bootstrapOverride, filterRegistry, + this.childChannelConfigurer); } @VisibleForTesting diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index 5529f96c7a2..54a33752ed2 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -29,6 +29,8 @@ import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; import io.grpc.Attributes; +import io.grpc.ChildChannelConfigurer; +import io.grpc.ChildChannelConfigurers; import io.grpc.InternalServerInterceptors; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -128,6 +130,15 @@ public void uncaughtException(Thread t, Throwable e) { // NamedFilterConfig.filterStateKey -> filter_instance. private final HashMap activeFiltersDefaultChain = new HashMap<>(); + /** + * Stores the user-provided configuration function for internal child channels. + * + *

This is intended for use by gRPC internal components + * that are responsible for creating auxiliary {@code ManagedChannel} instances. + * guaranteed to be not null (defaults to no-op). + */ + private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); + XdsServerWrapper( String listenerAddress, ServerBuilder delegateBuilder, @@ -148,6 +159,30 @@ public void uncaughtException(Thread t, Throwable e) { sharedTimeService = true; } + XdsServerWrapper( + String listenerAddress, + ServerBuilder delegateBuilder, + XdsServingStatusListener listener, + FilterChainSelectorManager filterChainSelectorManager, + XdsClientPoolFactory xdsClientPoolFactory, + @Nullable Map bootstrapOverride, + FilterRegistry filterRegistry, + ChildChannelConfigurer childChannelConfigurer) { + this( + listenerAddress, + delegateBuilder, + listener, + filterChainSelectorManager, + xdsClientPoolFactory, + bootstrapOverride, + filterRegistry, + SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE)); + sharedTimeService = true; + if (childChannelConfigurer != null) { + this.childChannelConfigurer = childChannelConfigurer; + } + } + @VisibleForTesting XdsServerWrapper( String listenerAddress, @@ -202,7 +237,8 @@ private void internalStart() { bootstrapInfo = new GrpcBootstrapperImpl().bootstrap(bootstrapOverride); } xdsClientPool = xdsClientPoolFactory.getOrCreate( - "#server", bootstrapInfo, new MetricRecorder() {}); + "#server", bootstrapInfo, new MetricRecorder() {}, + null, this); } catch (Exception e) { StatusException statusException = Status.UNAVAILABLE.withDescription( "Failed to initialize xDS").withCause(e).asException(); @@ -228,6 +264,19 @@ private void internalStart() { discoveryState = new DiscoveryState(listenerTemplate.replaceAll("%s", replacement)); } + /** + * Retrieves the user-provided configuration function for internal child channels. + * + *

This method is intended for use by gRPC internal components (NameResolvers, LoadBalancers) + * that are responsible for creating auxiliary {@code ManagedChannel} instances. + * + * @return the ChildChannelConfigurer, guaranteed to be not null (defaults to no-op). + */ + @Override + public ChildChannelConfigurer getChildChannelConfigurer() { + return childChannelConfigurer; + } + @Override public Server shutdown() { if (!shutdown.compareAndSet(false, true)) { diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index ff5edaded00..74410b8f01c 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -41,6 +41,7 @@ import io.grpc.InsecureChannelCredentials; import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; +import io.grpc.Server; import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusRuntimeException; @@ -522,7 +523,7 @@ public ObjectPool getOrCreate( @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, Server parentServer) { throw new UnsupportedOperationException("Should not be called"); } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index caad48a54e9..ff9b4076c0a 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -5068,7 +5068,7 @@ public void serverFailureMetricReport_forRetryAndBackoff() { private XdsClientImpl createXdsClient(String serverUri) { BootstrapInfo bootstrapInfo = buildBootStrap(serverUri); return new XdsClientImpl( - new GrpcXdsTransportFactory(null, null), + new GrpcXdsTransportFactory(null, null, null), bootstrapInfo, fakeClock.getScheduledExecutorService(), backoffPolicyProvider, diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index 93b3be3c9ba..e1d31d50dc4 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -92,7 +92,7 @@ public void onCompleted() { @Test public void callApis() throws Exception { XdsTransportFactory.XdsTransport xdsTransport = - new GrpcXdsTransportFactory(null, null) + new GrpcXdsTransportFactory(null, null, null) .create( Bootstrapper.ServerInfo.create( "localhost:" + server.getPort(), InsecureChannelCredentials.create())); diff --git a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java index bfc7db19750..55dc862cc38 100644 --- a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java +++ b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java @@ -181,7 +181,7 @@ public void cancelled(Context context) { lrsClient = new LoadReportClient( loadStatsManager, - new GrpcXdsTransportFactory(null, null).createForTest(channel), + new GrpcXdsTransportFactory(null, null, null).createForTest(channel), NODE, syncContext, fakeClock.getScheduledExecutorService(), diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index 9f091a79ff9..c775037e0d5 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -207,7 +207,8 @@ public void xdsClient_usesCallCredentials() throws Exception { // Create xDS client that uses the CallCredentials on the transport ObjectPool xdsClientPool = - provider.getOrCreate("target", bootstrapInfo, metricRecorder, sampleCreds, null); + provider.getOrCreate("target", bootstrapInfo, metricRecorder, sampleCreds, + null, null); XdsClient xdsClient = xdsClientPool.getObject(); xdsClient.watchXdsResource( XdsListenerResource.getInstance(), "someLDSresource", ldsResourceWatcher); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java index 4d5e7d09ad4..09b330a2d43 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java @@ -484,7 +484,7 @@ public void fallbackFromBadUrlToGoodOne() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(garbageUri, validUri), - new GrpcXdsTransportFactory(null, null), + new GrpcXdsTransportFactory(null, null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, @@ -509,7 +509,7 @@ public void testGoodUrlFollowedByBadUrl() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(validUri, garbageUri), - new GrpcXdsTransportFactory(null, null), + new GrpcXdsTransportFactory(null, null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, @@ -536,7 +536,7 @@ public void testTwoBadUrl() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(garbageUri1, garbageUri2), - new GrpcXdsTransportFactory(null, null), + new GrpcXdsTransportFactory(null, null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 95c4cde279a..7098a4d315d 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -68,6 +68,7 @@ import io.grpc.NameResolver.ServiceConfigParser; import io.grpc.NoopClientCall; import io.grpc.NoopClientCall.NoopClientCallListener; +import io.grpc.Server; import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusOr; @@ -2497,7 +2498,7 @@ public XdsClient returnObject(Object object) { @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, Server parentServer) { targets.add(target); return new ObjectPool() { @Override diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index bc300a4ef68..9f32d0aab4b 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -25,6 +25,7 @@ import io.grpc.InsecureChannelCredentials; import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; +import io.grpc.Server; import io.grpc.Status; import io.grpc.StatusOr; import io.grpc.internal.ObjectPool; @@ -186,7 +187,7 @@ public XdsClient returnObject(Object object) { @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel) { + ManagedChannel parentChannel, Server parentServer) { this.savedBootstrapInfo = bootstrapInfo; return new ObjectPool() { @Override From 3259d67a73863a2c5ec8cdddef11e18268aa7c96 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 29 Dec 2025 20:00:10 +0530 Subject: [PATCH 06/10] add test case for xDS server and others --- .../io/grpc/ForwardingChannelBuilder2.java | 4 +- .../java/io/grpc/ManagedChannelBuilder.java | 12 +-- .../io/grpc/ChildChannelConfigurersTest.java | 2 +- .../internal/ManagedChannelImplBuilder.java | 4 +- .../ManagedChannelImplBuilderTest.java | 69 ++++++++++++++++ .../grpc/xds/GrpcXdsTransportFactoryTest.java | 79 +++++++++++++++++++ .../java/io/grpc/xds/XdsNameResolverTest.java | 63 +++++++++++++++ .../io/grpc/xds/XdsServerBuilderTest.java | 27 +++++++ 8 files changed, 247 insertions(+), 13 deletions(-) diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java index aed2ff8f45c..eac1d824145 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java @@ -95,7 +95,7 @@ public T intercept(ClientInterceptor... interceptors) { } @Override - protected T interceptWithTarget(InterceptorFactory factory) { + public T interceptWithTarget(InterceptorFactory factory) { delegate().interceptWithTarget(factory); return thisT(); } @@ -258,7 +258,7 @@ public T disableServiceConfigLookUp() { } @Override - protected T addMetricSink(MetricSink metricSink) { + public T addMetricSink(MetricSink metricSink) { delegate().addMetricSink(metricSink); return thisT(); } diff --git a/api/src/main/java/io/grpc/ManagedChannelBuilder.java b/api/src/main/java/io/grpc/ManagedChannelBuilder.java index b4211c504c0..94daa47d83b 100644 --- a/api/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -160,17 +160,14 @@ public T offloadExecutor(Executor executor) { public abstract T intercept(ClientInterceptor... interceptors); /** - * Internal-only: Adds a factory that will construct an interceptor based on the channel's target. + * Adds a factory that will construct an interceptor based on the channel's target. * This can be used to work around nameResolverFactory() changing the target string. */ - @Internal - protected T interceptWithTarget(InterceptorFactory factory) { + public T interceptWithTarget(InterceptorFactory factory) { throw new UnsupportedOperationException(); } - /** Internal-only. */ - @Internal - protected interface InterceptorFactory { + public interface InterceptorFactory { ClientInterceptor newInterceptor(String target); } @@ -638,8 +635,7 @@ public T disableServiceConfigLookUp() { * @return this * @since 1.64.0 */ - @Internal - protected T addMetricSink(MetricSink metricSink) { + public T addMetricSink(MetricSink metricSink) { throw new UnsupportedOperationException(); } diff --git a/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java b/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java index 08ebcbc6e08..ffcb88d289e 100644 --- a/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java +++ b/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java @@ -55,4 +55,4 @@ public void compose_ignoresNulls() { verify(builder).userAgent("agent1"); } -} \ No newline at end of file +} diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index bcbe6e63054..e56d04ad9eb 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -404,7 +404,7 @@ public ManagedChannelImplBuilder intercept(ClientInterceptor... interceptors) { } @Override - protected ManagedChannelImplBuilder interceptWithTarget(InterceptorFactory factory) { + public ManagedChannelImplBuilder interceptWithTarget(InterceptorFactory factory) { // Add a placeholder instance to the interceptor list, and replace it with a real instance // during build(). this.interceptors.add(new InterceptorFactoryWrapper(factory)); @@ -713,7 +713,7 @@ public ManagedChannelImplBuilder enableCheckAuthority() { } @Override - protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { + public ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { metricSinks.add(checkNotNull(metricSink, "metric sink")); return this; } diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java index a054e65a6e8..a1e7f7f3e64 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -34,6 +35,7 @@ import com.google.common.util.concurrent.MoreExecutors; import io.grpc.CallOptions; import io.grpc.Channel; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.CompressorRegistry; @@ -70,6 +72,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -762,6 +765,72 @@ public void setNameResolverExtArgs() { assertThat(builder.nameResolverCustomArgs.get(testKey)).isEqualTo(42); } + @Test + public void childChannelConfigurer_propagatesMetricsAndInterceptors_xdsTarget() { + // Setup Mocks + when(mockClientTransportFactory.getScheduledExecutorService()) + .thenReturn(clock.getScheduledExecutorService()); + when(mockClientTransportFactoryBuilder.buildClientTransportFactory()) + .thenReturn(mockClientTransportFactory); + when(mockClientTransportFactory.getSupportedSocketAddressTypes()) + .thenReturn(Collections.singleton(InetSocketAddress.class)); + + MetricSink mockMetricSink = mock(MetricSink.class); + ClientInterceptor mockInterceptor = mock(ClientInterceptor.class); + + // Define the Configurer + ChildChannelConfigurer configurer = (builder) -> { + builder.addMetricSink(mockMetricSink); + + // Assuming InternalInterceptorFactory is also accessible + builder.interceptWithTarget(target -> mockInterceptor); + }; + + // Mock NameResolver.Factory to capture Args + NameResolver.Factory mockNameResolverFactory = mock(NameResolver.Factory.class); + when(mockNameResolverFactory.getDefaultScheme()).thenReturn("xds"); + NameResolver mockNameResolver = mock(NameResolver.class); + when(mockNameResolver.getServiceAuthority()).thenReturn("foo.authority"); + ArgumentCaptor argsCaptor = ArgumentCaptor.forClass(NameResolver.Args.class); + when(mockNameResolverFactory.newNameResolver(any(), + argsCaptor.capture())).thenReturn(mockNameResolver); + + // Use the configurer and the mock factory + NameResolverRegistry registry = new NameResolverRegistry(); + registry.register(new NameResolverFactoryToProviderFacade(mockNameResolverFactory)); + + ManagedChannelBuilder parentBuilder = new ManagedChannelImplBuilder( + "xds:///my-service-target", + mockClientTransportFactoryBuilder, + new FixedPortProvider(DUMMY_PORT)) + .childChannelConfigurer(configurer) + .nameResolverRegistry(registry); + + ManagedChannel channel = parentBuilder.build(); + grpcCleanupRule.register(channel); + + // Verify that newNameResolver was called + verify(mockNameResolverFactory).newNameResolver(any(), any()); + + // Extract the parent channel from Args + NameResolver.Args args = argsCaptor.getValue(); + ManagedChannel parentChannelInArgs = args.getParentChannel(); + assertNotNull("Parent channel should be present in NameResolver.Args", + parentChannelInArgs); + + // Verify the configurer on the parent channel is the one we passed + assertThat(parentChannelInArgs.getChildChannelConfigurer()).isSameInstanceAs(configurer); + + // Verify the configurer logically applies (by running it on a mock) + ManagedChannelBuilder mockChildBuilder = mock(ManagedChannelBuilder.class); + // Stub addMetricSink to return the builder to avoid generic return type issues + doReturn(mockChildBuilder).when(mockChildBuilder).addMetricSink(any()); + + configurer.accept(mockChildBuilder); + verify(mockChildBuilder).addMetricSink(mockMetricSink); + verify(mockChildBuilder).interceptWithTarget(any()); + } + @Test public void metricSinks() { MetricSink mocksink = mock(MetricSink.class); diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index e1d31d50dc4..36b91120bf3 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -17,15 +17,25 @@ package io.grpc.xds; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.service.discovery.v3.AggregatedDiscoveryServiceGrpc; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryRequest; import io.envoyproxy.envoy.service.discovery.v3.DiscoveryResponse; import io.grpc.BindableService; +import io.grpc.CallOptions; +import io.grpc.Channel; +import io.grpc.ChildChannelConfigurer; +import io.grpc.ClientInterceptor; import io.grpc.Grpc; import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureServerCredentials; +import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.MethodDescriptor; import io.grpc.Server; import io.grpc.Status; @@ -139,5 +149,74 @@ public void onStatusReceived(Status status) { endFuture.set(status); } } + + @Test + @SuppressWarnings("unchecked") + public void verifyConfigApplied_interceptor() { + // Create a mock Interceptor + final ClientInterceptor mockInterceptor = mock(ClientInterceptor.class); + when(mockInterceptor.interceptCall(any(MethodDescriptor.class), + any(CallOptions.class), any(Channel.class))) + .thenReturn(new io.grpc.NoopClientCall<>()); + + // Create Configurer that adds the interceptor + ChildChannelConfigurer configurer = (builder) -> builder.intercept(mockInterceptor); + + // Mock Parent Channel + ManagedChannel mockParentChannel = mock(ManagedChannel.class); + when(mockParentChannel.getChildChannelConfigurer()).thenReturn(configurer); + + // Create Factory + GrpcXdsTransportFactory factory = new GrpcXdsTransportFactory( + null, + mockParentChannel, + null); + + // Create Transport + XdsTransportFactory.XdsTransport transport = factory.create( + Bootstrapper.ServerInfo.create("localhost:8080", InsecureChannelCredentials.create())); + + // Create a Call to trigger interceptors + MethodDescriptor method = MethodDescriptor.newBuilder() + .setType(MethodDescriptor.MethodType.UNARY) + .setFullMethodName("service/method") + .setFullMethodName("service/method") + .setRequestMarshaller(mock(MethodDescriptor.Marshaller.class)) + .setResponseMarshaller(mock(MethodDescriptor.Marshaller.class)) + .build(); + + transport.createStreamingCall(method.getFullMethodName(), method.getRequestMarshaller(), + method.getResponseMarshaller()); + + // Verify interceptor was invoked + verify(mockInterceptor).interceptCall(any(MethodDescriptor.class), + any(CallOptions.class), any(Channel.class)); + + transport.shutdown(); + } + + @Test + public void useParentServerConfig() { + // 1. Mock Server and Configurer + Server mockServer = mock(Server.class); + ChildChannelConfigurer mockConfigurer = mock(ChildChannelConfigurer.class); + when(mockServer.getChildChannelConfigurer()).thenReturn(mockConfigurer); + + // 2. Create Factory with Parent Server + GrpcXdsTransportFactory factory = new GrpcXdsTransportFactory( + null, // CallCredentials + null, // Parent Channel + mockServer); + + // 3. Create Transport (triggers channel creation) + XdsTransportFactory.XdsTransport transport = factory.create( + Bootstrapper.ServerInfo.create("localhost:8080", InsecureChannelCredentials.create())); + + // 4. Verify Configurer was accessed and applied + verify(mockServer).getChildChannelConfigurer(); + verify(mockConfigurer).accept(any(ManagedChannelBuilder.class)); + + transport.shutdown(); + } } diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 7098a4d315d..d7058438a00 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -27,7 +27,9 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -68,6 +70,7 @@ import io.grpc.NameResolver.ServiceConfigParser; import io.grpc.NoopClientCall; import io.grpc.NoopClientCall.NoopClientCallListener; +import io.grpc.ProxyDetector; import io.grpc.Server; import io.grpc.Status; import io.grpc.Status.Code; @@ -2951,4 +2954,64 @@ void deliverErrorStatus() { listener.onClose(Status.UNAVAILABLE, new Metadata()); } } + + @Test + public void start_passesParentChannelToClientPoolFactory() { + // Create a mock Parent Channel + ManagedChannel mockParentChannel = mock(ManagedChannel.class); + + // Build NameResolver.Args containing the parent channel + NameResolver.Args args = NameResolver.Args.newBuilder() + .setDefaultPort(8080) + .setProxyDetector(mock(ProxyDetector.class)) + .setSynchronizationContext(syncContext) + .setServiceConfigParser(serviceConfigParser) + .setChannelLogger(mock(ChannelLogger.class)) + .setParentChannel(mockParentChannel) + .build(); + + // Mock the XdsClientPoolFactory + XdsClientPoolFactory mockPoolFactory = mock(XdsClientPoolFactory.class); + @SuppressWarnings("unchecked") + ObjectPool mockObjectPool = mock(ObjectPool.class); + XdsClient mockXdsClient = mock(XdsClient.class); + when(mockObjectPool.getObject()).thenReturn(mockXdsClient); + when(mockXdsClient.getBootstrapInfo()).thenReturn(bootstrapInfo); + + when(mockPoolFactory.getOrCreate( + anyString(), + any(BootstrapInfo.class), + any(MetricRecorder.class), + any(ManagedChannel.class), + any())) + .thenReturn(mockObjectPool); + + XdsNameResolver resolver = new XdsNameResolver( + URI.create(AUTHORITY), + null, // targetAuthority (nullable) + AUTHORITY, // name + null, // overrideAuthority (nullable) + serviceConfigParser, + syncContext, + scheduler, + mockPoolFactory, + mockRandom, + FilterRegistry.getDefaultRegistry(), + rawBootstrap, + metricRecorder, + args); + + // Start the resolver (this triggers the factory call) + resolver.start(mockListener); + + // Ensure the factory's getOrCreate method was called with parent channel + verify(mockPoolFactory).getOrCreate( + eq(AUTHORITY), + any(BootstrapInfo.class), + eq(metricRecorder), + eq(mockParentChannel), + isNull()); + + resolver.shutdown(); + } } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index ac990226259..8ca0a96d0d2 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -30,15 +30,18 @@ import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.SettableFuture; import io.grpc.BindableService; +import io.grpc.ChildChannelConfigurer; import io.grpc.InsecureServerCredentials; import io.grpc.ServerServiceDefinition; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.StatusOr; +import io.grpc.internal.ObjectPool; import io.grpc.testing.GrpcCleanupRule; import io.grpc.xds.XdsListenerResource.LdsUpdate; import io.grpc.xds.XdsServerTestHelper.FakeXdsClient; import io.grpc.xds.XdsServerTestHelper.FakeXdsClientPoolFactory; +import io.grpc.xds.client.XdsClient; import io.grpc.xds.internal.security.CommonTlsContextTestsUtil; import java.io.IOException; import java.net.InetSocketAddress; @@ -325,4 +328,28 @@ public void testOverrideBootstrap() throws Exception { assertThat(xdsClientPoolFactory.savedBootstrapInfo.node().getId()) .isEqualTo(XdsServerTestHelper.BOOTSTRAP_INFO.node().getId()); } + + @Test + public void start_passesParentServerToClientPoolFactory() throws Exception { + ChildChannelConfigurer mockConfigurer = mock(ChildChannelConfigurer.class); + XdsClientPoolFactory mockPoolFactory = mock(XdsClientPoolFactory.class); + @SuppressWarnings("unchecked") + ObjectPool mockPool = mock(ObjectPool.class); + when(mockPool.getObject()).thenReturn(xdsClient); + when(mockPoolFactory.getOrCreate(any(), any(), any(), any(), any())).thenReturn(mockPool); + + buildBuilder(null); + builder.childChannelConfigurer(mockConfigurer); + builder.xdsClientPoolFactory(mockPoolFactory); + xdsServer = cleanupRule.register((XdsServerWrapper) builder.build()); + + Future unused = startServerAsync(); + + // Verify getOrCreate called with the server instance + verify(mockPoolFactory).getOrCreate( + any(), any(), any(), org.mockito.ArgumentMatchers.isNull(), + org.mockito.ArgumentMatchers.eq(xdsServer)); + + assertThat(xdsServer.getChildChannelConfigurer()).isSameInstanceAs(mockConfigurer); + } } From eab10a9cbe9fbfc0ba3c1c12b73cb7bce43c26f8 Mon Sep 17 00:00:00 2001 From: agravator Date: Sat, 14 Mar 2026 02:04:56 +0530 Subject: [PATCH 07/10] fix: pass the configurer --- .../java/io/grpc/ChildChannelConfigurer.java | 6 +- .../java/io/grpc/ChildChannelConfigurers.java | 99 ---------------- .../io/grpc/ForwardingChannelBuilder.java | 11 -- .../io/grpc/ForwardingChannelBuilder2.java | 11 -- .../java/io/grpc/ForwardingServerBuilder.java | 6 - api/src/main/java/io/grpc/ManagedChannel.java | 17 --- .../java/io/grpc/ManagedChannelBuilder.java | 31 ----- api/src/main/java/io/grpc/NameResolver.java | 23 ++-- api/src/main/java/io/grpc/Server.java | 16 --- api/src/main/java/io/grpc/ServerBuilder.java | 15 --- .../io/grpc/ChildChannelConfigurersTest.java | 58 --------- .../test/java/io/grpc/NameResolverTest.java | 17 ++- .../internal/ForwardingManagedChannel.java | 6 - .../io/grpc/internal/ManagedChannelImpl.java | 19 +-- .../internal/ManagedChannelImplBuilder.java | 65 ++-------- .../ManagedChannelImplBuilderTest.java | 19 ++- .../grpc/internal/ManagedChannelImplTest.java | 111 ------------------ .../io/grpc/xds/GrpcXdsTransportFactory.java | 22 ++-- .../InternalSharedXdsClientPoolProvider.java | 2 +- .../grpc/xds/SharedXdsClientPoolProvider.java | 32 ++--- .../io/grpc/xds/XdsClientPoolFactory.java | 5 +- .../java/io/grpc/xds/XdsNameResolver.java | 14 +-- .../java/io/grpc/xds/XdsServerBuilder.java | 3 +- .../java/io/grpc/xds/XdsServerWrapper.java | 24 +--- .../java/io/grpc/xds/CsdsServiceTest.java | 5 +- .../grpc/xds/GrpcXdsClientImplTestBase.java | 2 +- .../grpc/xds/GrpcXdsTransportFactoryTest.java | 58 ++++++--- .../io/grpc/xds/LoadReportClientTest.java | 2 +- .../xds/SharedXdsClientPoolProviderTest.java | 61 +++++++++- .../io/grpc/xds/XdsClientFallbackTest.java | 6 +- .../java/io/grpc/xds/XdsNameResolverTest.java | 20 ++-- .../io/grpc/xds/XdsServerBuilderTest.java | 11 +- .../java/io/grpc/xds/XdsServerTestHelper.java | 5 +- 33 files changed, 206 insertions(+), 596 deletions(-) delete mode 100644 api/src/main/java/io/grpc/ChildChannelConfigurers.java delete mode 100644 api/src/test/java/io/grpc/ChildChannelConfigurersTest.java diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurer.java b/api/src/main/java/io/grpc/ChildChannelConfigurer.java index 36b67098797..4392c7f7bae 100644 --- a/api/src/main/java/io/grpc/ChildChannelConfigurer.java +++ b/api/src/main/java/io/grpc/ChildChannelConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 The gRPC Authors + * Copyright 2026 The gRPC Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -60,8 +60,8 @@ public interface ChildChannelConfigurer extends ConsumerNote: The provided {@code builder} is generic (`?`). Implementations should use - * universal configuration methods (like {@code intercept()}, {@code userAgent()}) rather - * than casting to specific implementation types. + * universal configuration methods (like {@code intercept()}, {@code userAgent()}) on the + * builder rather than casting it to specific implementation types. * * @param builder the mutable channel builder for the new child channel */ diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurers.java b/api/src/main/java/io/grpc/ChildChannelConfigurers.java deleted file mode 100644 index 92508f5e71c..00000000000 --- a/api/src/main/java/io/grpc/ChildChannelConfigurers.java +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright 2025 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc; - -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * Utilities for working with {@link ChildChannelConfigurer}. - * - * @since 1.79.0 - */ -@ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") -public final class ChildChannelConfigurers { - private static final Logger logger = Logger.getLogger(ChildChannelConfigurers.class.getName()); - - // Singleton no-op instance to avoid object churn - private static final ChildChannelConfigurer NO_OP = builder -> { - }; - - private ChildChannelConfigurers() { // Prevent instantiation - } - - /** - * Returns a configurer that does nothing. - * Useful as a default value to avoid null checks in internal code. - */ - public static ChildChannelConfigurer noOp() { - return NO_OP; - } - - /** - * Returns a configurer that applies all the given configurers in sequence. - * - *

If any configurer in the chain throws an exception, the remaining ones are skipped - * (unless wrapped in {@link #safe(ChildChannelConfigurer)}). - * - * @param configurers the configurers to apply in order. Null elements are ignored. - */ - public static ChildChannelConfigurer compose(ChildChannelConfigurer... configurers) { - checkNotNull(configurers, "configurers"); - return builder -> { - for (ChildChannelConfigurer configurer : configurers) { - if (configurer != null) { - configurer.accept(builder); - } - } - }; - } - - /** - * Returns a configurer that applies the delegate but catches and logs any exceptions. - * - *

This prevents a buggy configurer (e.g., one that fails metric setup) from crashing - * the critical path of channel creation. - * - * @param delegate the configurer to wrap. - */ - public static ChildChannelConfigurer safe(ChildChannelConfigurer delegate) { - checkNotNull(delegate, "delegate"); - return builder -> { - try { - delegate.accept(builder); - } catch (Exception e) { - logger.log(Level.WARNING, "Failed to apply child channel configuration", e); - } - }; - } - - /** - * Returns a configurer that applies the delegate only if the given condition is true. - * - *

Useful for applying interceptors only in specific environments (e.g., Debug/Test). - * - * @param condition true to apply the delegate, false to do nothing. - * @param delegate the configurer to apply if condition is true. - */ - public static ChildChannelConfigurer conditional(boolean condition, - ChildChannelConfigurer delegate) { - checkNotNull(delegate, "delegate"); - return condition ? delegate : NO_OP; - } -} diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java index 44facd8796b..6e69b493992 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java @@ -242,17 +242,6 @@ public T disableServiceConfigLookUp() { return thisT(); } - @Override - public T configureChannel(ManagedChannel parentChannel) { - delegate().configureChannel(parentChannel); - return thisT(); - } - - @Override - public T configureChannel(Server parentServer) { - delegate().configureChannel(parentServer); - return thisT(); - } @Override public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java index eac1d824145..2fa60dd6ce1 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java @@ -269,17 +269,6 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { return thisT(); } - @Override - public T configureChannel(ManagedChannel parentChannel) { - delegate().configureChannel(parentChannel); - return thisT(); - } - - @Override - public T configureChannel(Server parentServer) { - delegate().configureChannel(parentServer); - return thisT(); - } @Override public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { diff --git a/api/src/main/java/io/grpc/ForwardingServerBuilder.java b/api/src/main/java/io/grpc/ForwardingServerBuilder.java index d7f5be03ea2..15ce31bb909 100644 --- a/api/src/main/java/io/grpc/ForwardingServerBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingServerBuilder.java @@ -192,12 +192,6 @@ public T setBinaryLog(BinaryLog binaryLog) { return thisT(); } - @Override - public T configureChannel(Server parentServer) { - delegate().configureChannel(parentServer); - return thisT(); - } - @Override public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { delegate().childChannelConfigurer(childChannelConfigurer); diff --git a/api/src/main/java/io/grpc/ManagedChannel.java b/api/src/main/java/io/grpc/ManagedChannel.java index a2498097833..7875fdb57f2 100644 --- a/api/src/main/java/io/grpc/ManagedChannel.java +++ b/api/src/main/java/io/grpc/ManagedChannel.java @@ -85,23 +85,6 @@ public ConnectivityState getState(boolean requestConnection) { throw new UnsupportedOperationException("Not implemented"); } - /** - * Returns the configurer for child channels. - * - *

This method is intended for use by the internal gRPC infrastructure (specifically - * load balancers and the channel builder) to propagate configuration to child channels. - * Application code should not call this method. - * - * @return the configurer, or {@code noOp()} if none is set. - * @since 1.79.0 - */ - @Internal - public ChildChannelConfigurer getChildChannelConfigurer() { - // Return noOP() by default so we don't break existing custom ManagedChannel implementations - // (like wrappers or mocks) that don't override this method. - return ChildChannelConfigurers.noOp(); - } - /** * Registers a one-off callback that will be run if the connectivity state of the channel diverges * from the given {@code source}, which is typically what has just been returned by {@link diff --git a/api/src/main/java/io/grpc/ManagedChannelBuilder.java b/api/src/main/java/io/grpc/ManagedChannelBuilder.java index 94daa47d83b..59b294ddbc4 100644 --- a/api/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -657,37 +657,6 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { throw new UnsupportedOperationException(); } - /** - * Configures this builder using settings derived from an existing parent channel. - * - *

This method is typically used by internal components (like LoadBalancers) when creating - * child channels to ensure they inherit relevant configuration (like the - * {@link ChildChannelConfigurer}) from the parent. - * - * @param parentChannel the channel to inherit configuration from - * @return this - * @since 1.79.0 - */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") - public T configureChannel(ManagedChannel parentChannel) { - throw new UnsupportedOperationException(); - } - - /** - * Configures this builder using settings derived from an existing parent server. - * - *

This method is typically used by internal components (like LoadBalancers) when creating - * child channels to ensure they inherit relevant configuration (like the - * {@link ChildChannelConfigurer}) from the parent. - * - * @param parentServer the server to inherit configuration from - * @return this - * @since 1.79.0 - */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") - public T configureChannel(Server parentServer) { - throw new UnsupportedOperationException(); - } /** * Sets a configurer that will be applied to all internal child channels created by this channel. diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index d6e6c563823..ec820824bbc 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -323,7 +323,7 @@ public static final class Args { @Nullable private final MetricRecorder metricRecorder; @Nullable private final NameResolverRegistry nameResolverRegistry; @Nullable private final IdentityHashMap, Object> customArgs; - @Nullable private final ManagedChannel parentChannel; + @Nullable private final ChildChannelConfigurer childChannelConfigurer; private Args(Builder builder) { this.defaultPort = checkNotNull(builder.defaultPort, "defaultPort not set"); @@ -338,7 +338,7 @@ private Args(Builder builder) { this.metricRecorder = builder.metricRecorder; this.nameResolverRegistry = builder.nameResolverRegistry; this.customArgs = cloneCustomArgs(builder.customArgs); - this.parentChannel = builder.parentChannel; + this.childChannelConfigurer = builder.childChannelConfigurer; } /** @@ -438,11 +438,14 @@ public ChannelLogger getChannelLogger() { } /** - * Returns the parent {@link ManagedChannel} served by this NameResolver. + * Returns the configurer for child channels. + * + * @since 1.81.0 */ + @Nullable @Internal - public ManagedChannel getParentChannel() { - return parentChannel; + public ChildChannelConfigurer getChildChannelConfigurer() { + return childChannelConfigurer; } /** @@ -554,7 +557,7 @@ public static final class Builder { private MetricRecorder metricRecorder; private NameResolverRegistry nameResolverRegistry; private IdentityHashMap, Object> customArgs; - private ManagedChannel parentChannel; + private ChildChannelConfigurer childChannelConfigurer; Builder() { } @@ -671,12 +674,12 @@ public Builder setNameResolverRegistry(NameResolverRegistry registry) { } /** - * See {@link Args#parentChannel}. This is an optional field. + * See {@link Args#getChildChannelConfigurer()}. This is an optional field. * - * @since 1.79.0 + * @since 1.81.0 */ - public Builder setParentChannel(ManagedChannel parentChannel) { - this.parentChannel = parentChannel; + public Builder setChildChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + this.childChannelConfigurer = childChannelConfigurer; return this; } diff --git a/api/src/main/java/io/grpc/Server.java b/api/src/main/java/io/grpc/Server.java index 92868cb6569..d744752ecc5 100644 --- a/api/src/main/java/io/grpc/Server.java +++ b/api/src/main/java/io/grpc/Server.java @@ -179,20 +179,4 @@ public List getMutableServices() { */ public abstract void awaitTermination() throws InterruptedException; - /** - * Returns the configurer for child channels. - * - *

This method is intended for use by the internal gRPC infrastructure - * to propagate configuration to child channels. - * Application code should not call this method. - * - * @return the configurer, or {@code noOp()} if none is set. - * @since 1.79.0 - */ - @Internal - public ChildChannelConfigurer getChildChannelConfigurer() { - // Return noOp() by default so we don't break existing custom ManagedChannel implementations - // (like wrappers or mocks) that don't override this method. - return ChildChannelConfigurers.noOp(); - } } diff --git a/api/src/main/java/io/grpc/ServerBuilder.java b/api/src/main/java/io/grpc/ServerBuilder.java index 7f929dba15b..834f6928c92 100644 --- a/api/src/main/java/io/grpc/ServerBuilder.java +++ b/api/src/main/java/io/grpc/ServerBuilder.java @@ -424,21 +424,6 @@ public T setBinaryLog(BinaryLog binaryLog) { throw new UnsupportedOperationException(); } - /** - * Configures this builder using settings derived from an existing parent server. - * - *

This method is typically used by internal components when creating - * child channels to ensure they inherit relevant configuration (like the - * {@link ChildChannelConfigurer}) from the parent. - * - * @param parentServer the server to inherit configuration from - * @return this - * @since 1.79.0 - */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") - public T configureChannel(Server parentServer) { - throw new UnsupportedOperationException(); - } /** * Sets a configurer that will be applied to all internal child channels created by this server. diff --git a/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java b/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java deleted file mode 100644 index ffcb88d289e..00000000000 --- a/api/src/test/java/io/grpc/ChildChannelConfigurersTest.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2025 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class ChildChannelConfigurersTest { - - @Test - public void noOp_doesNothing() { - ManagedChannelBuilder builder = mock(ManagedChannelBuilder.class); - ChildChannelConfigurers.noOp().accept(builder); - verifyNoInteractions(builder); - } - - @Test - public void compose_runsInOrder() { - ManagedChannelBuilder builder = mock(ManagedChannelBuilder.class); - ChildChannelConfigurer configurer1 = b -> b.userAgent("agent1"); - ChildChannelConfigurer configurer2 = b -> b.maxInboundMessageSize(123); - - ChildChannelConfigurers.compose(configurer1, configurer2).accept(builder); - - verify(builder).userAgent("agent1"); - verify(builder).maxInboundMessageSize(123); - } - - @Test - public void compose_ignoresNulls() { - ManagedChannelBuilder builder = mock(ManagedChannelBuilder.class); - ChildChannelConfigurer configurer = b -> b.userAgent("agent1"); - - ChildChannelConfigurers.compose(null, configurer, null).accept(builder); - - verify(builder).userAgent("agent1"); - } -} diff --git a/api/src/test/java/io/grpc/NameResolverTest.java b/api/src/test/java/io/grpc/NameResolverTest.java index ead5c462286..d2219a1525f 100644 --- a/api/src/test/java/io/grpc/NameResolverTest.java +++ b/api/src/test/java/io/grpc/NameResolverTest.java @@ -105,7 +105,7 @@ public void args() { } private NameResolver.Args createArgs() { - ManagedChannel parent = mock(ManagedChannel.class); + ChildChannelConfigurer childChannelConfigurer = mock(ChildChannelConfigurer.class); return NameResolver.Args.newBuilder() .setDefaultPort(defaultPort) .setProxyDetector(proxyDetector) @@ -117,13 +117,13 @@ private NameResolver.Args createArgs() { .setOverrideAuthority(overrideAuthority) .setMetricRecorder(metricRecorder) .setArg(FOO_ARG_KEY, customArgValue) - .setParentChannel(parent) + .setChildChannelConfigurer(childChannelConfigurer) .build(); } @Test - public void args_parentChannel() { - ManagedChannel parent = mock(ManagedChannel.class); + public void args_childChannelConfigurer() { + ChildChannelConfigurer childChannelConfigurer = mock(ChildChannelConfigurer.class); // Create a real SynchronizationContext instead of mocking it SynchronizationContext realSyncContext = new SynchronizationContext( @@ -140,10 +140,15 @@ public void uncaughtException(Thread t, Throwable e) { .setSynchronizationContext(realSyncContext) .setServiceConfigParser(mock(NameResolver.ServiceConfigParser.class)) .setChannelLogger(mock(ChannelLogger.class)) - .setParentChannel(parent) + .setChildChannelConfigurer(childChannelConfigurer) .build(); - assertThat(args.getParentChannel()).isSameInstanceAs(parent); + assertThat(args.getChildChannelConfigurer()).isSameInstanceAs(childChannelConfigurer); + + // Validate configurer accepts builders + ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); + args.getChildChannelConfigurer().accept(mockBuilder); + verify(childChannelConfigurer).accept(mockBuilder); } @Test diff --git a/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java b/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java index ae09785258b..7ef4ce42e97 100644 --- a/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java +++ b/core/src/main/java/io/grpc/internal/ForwardingManagedChannel.java @@ -18,7 +18,6 @@ import com.google.common.base.MoreObjects; import io.grpc.CallOptions; -import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ConnectivityState; import io.grpc.ManagedChannel; @@ -93,9 +92,4 @@ public void enterIdle() { public String toString() { return MoreObjects.toStringHelper(this).add("delegate", delegate).toString(); } - - @Override - public ChildChannelConfigurer getChildChannelConfigurer() { - return delegate.getChildChannelConfigurer(); - } } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 016ad96daa6..c19a8f49221 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -41,7 +41,6 @@ import io.grpc.ChannelLogger; import io.grpc.ChannelLogger.ChannelLogLevel; import io.grpc.ChildChannelConfigurer; -import io.grpc.ChildChannelConfigurers; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -156,14 +155,7 @@ public Result selectConfig(PickSubchannelArgs args) { private static final LoadBalancer.PickDetailsConsumer NOOP_PICK_DETAILS_CONSUMER = new LoadBalancer.PickDetailsConsumer() {}; - /** - * Stores the user-provided configuration function for internal child channels. - * - *

This is intended for use by gRPC internal components - * that are responsible for creating auxiliary {@code ManagedChannel} instances. - * guaranteed to be not null (defaults to no-op). - */ - private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); + private ChildChannelConfigurer childChannelConfigurer = builder -> {}; private final InternalLogId logId; private final String target; @@ -614,7 +606,7 @@ ClientStream newSubstream( .setOverrideAuthority(this.authorityOverride) .setMetricRecorder(this.metricRecorder) .setNameResolverRegistry(builder.nameResolverRegistry) - .setParentChannel(this); + .setChildChannelConfigurer(this.childChannelConfigurer); builder.copyAllNameResolverCustomArgsTo(nameResolverArgsBuilder); this.nameResolverArgs = nameResolverArgsBuilder.build(); this.nameResolver = getNameResolver( @@ -698,10 +690,6 @@ public CallTracer create() { * * @return the ChildChannelConfigurer, guaranteed to be not null (defaults to no-op). */ - @Override - public ChildChannelConfigurer getChildChannelConfigurer() { - return childChannelConfigurer; - } @VisibleForTesting static NameResolver getNameResolver( @@ -1592,8 +1580,7 @@ protected ManagedChannelBuilder delegate() { // Note that we follow the global configurator pattern and try to fuse the configurations as // soon as the builder gets created - ManagedChannel parentChannel = ManagedChannelImpl.this; - builder.configureChannel(parentChannel); + builder.childChannelConfigurer(childChannelConfigurer); return builder // TODO(zdapeng): executors should not outlive the parent channel. diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index e56d04ad9eb..6309ef3340b 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -46,7 +46,6 @@ import io.grpc.NameResolverProvider; import io.grpc.NameResolverRegistry; import io.grpc.ProxyDetector; -import io.grpc.Server; import io.grpc.StatusOr; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -126,7 +125,14 @@ public static ManagedChannelBuilder forTarget(String target) { private static final Method GET_CLIENT_INTERCEPTOR_METHOD; - ChildChannelConfigurer childChannelConfigurer; + ChildChannelConfigurer childChannelConfigurer = builder -> {}; + + @Override + public ManagedChannelImplBuilder childChannelConfigurer( + ChildChannelConfigurer childChannelConfigurer) { + this.childChannelConfigurer = checkNotNull(childChannelConfigurer, "childChannelConfigurer"); + return this; + } static { Method getClientInterceptorMethod = null; @@ -718,62 +724,7 @@ public ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { return this; } - /** - * Applies the configuration logic from the given parent channel to this builder. - * - *

This mechanism allows properties (like metrics, tracing, or interceptors) to propagate - * automatically from a parent channel to any child channels it creates - * (e.g., Subchannels or OOB channels). - * - * @param parentChannel the channel whose child's configuration logic - * should be applied to this builder. - */ - @Override - public ManagedChannelImplBuilder configureChannel(ManagedChannel parentChannel) { - if (parentChannel != null) { - ChildChannelConfigurer childChannelConfigurer = parentChannel.getChildChannelConfigurer(); - if (childChannelConfigurer != null) { - childChannelConfigurer.accept(this); - } - } - return this; - } - - /** - * Applies the configuration logic from the given parent server to this builder. - * - *

This mechanism allows properties (like metrics, tracing, or interceptors) to propagate - * automatically from a parent server to any child channels it creates - * (e.g., xDS). - * - * @param parentServer the server whose child's configuration logic - * should be applied to this builder. - */ - @Override - public ManagedChannelImplBuilder configureChannel(Server parentServer) { - if (parentServer != null) { - ChildChannelConfigurer childChannelConfigurer = parentServer.getChildChannelConfigurer(); - if (childChannelConfigurer != null) { - childChannelConfigurer.accept(this); - } - } - return this; - } - /** - * Sets the configurer that will be stored in the channel built by this builder. - * - *

This configurer will subsequently be used to configure any descendants (children) - * created by that channel. - * - * @param childChannelConfigurer the configurer to store in the channel. - */ - @Override - public ManagedChannelImplBuilder childChannelConfigurer( - ChildChannelConfigurer childChannelConfigurer) { - this.childChannelConfigurer = childChannelConfigurer; - return this; - } @Override public ManagedChannel build() { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java index a1e7f7f3e64..4faae81cc04 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java @@ -765,6 +765,13 @@ public void setNameResolverExtArgs() { assertThat(builder.nameResolverCustomArgs.get(testKey)).isEqualTo(42); } + @Test + public void childChannelConfigurer_setsField() { + ChildChannelConfigurer configurer = mock(ChildChannelConfigurer.class); + assertSame(builder, builder.childChannelConfigurer(configurer)); + assertSame(configurer, builder.childChannelConfigurer); + } + @Test public void childChannelConfigurer_propagatesMetricsAndInterceptors_xdsTarget() { // Setup Mocks @@ -812,14 +819,14 @@ public void childChannelConfigurer_propagatesMetricsAndInterceptors_xdsTarget() // Verify that newNameResolver was called verify(mockNameResolverFactory).newNameResolver(any(), any()); - // Extract the parent channel from Args + // Extract the childChannelConfigurer from Args NameResolver.Args args = argsCaptor.getValue(); - ManagedChannel parentChannelInArgs = args.getParentChannel(); - assertNotNull("Parent channel should be present in NameResolver.Args", - parentChannelInArgs); + ChildChannelConfigurer childChannelConfigurerInArgs = args.getChildChannelConfigurer(); + assertNotNull("Child channel configurer should be present in NameResolver.Args", + childChannelConfigurerInArgs); - // Verify the configurer on the parent channel is the one we passed - assertThat(parentChannelInArgs.getChildChannelConfigurer()).isSameInstanceAs(configurer); + // Verify the configurer is the one we passed + assertThat(childChannelConfigurerInArgs).isSameInstanceAs(configurer); // Verify the configurer logically applies (by running it on a mock) ManagedChannelBuilder mockChildBuilder = mock(ManagedChannelBuilder.class); diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 8312e15d98d..87abd17c632 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -3775,7 +3775,6 @@ public void channelsAndSubchannels_oob_instrumented_state() throws Exception { assertEquals(SHUTDOWN, getStats(channel).state); assertEquals(SHUTDOWN, getStats(oobChannel).state); } - @Test public void binaryLogInstalled() throws Exception { final SettableFuture intercepted = SettableFuture.create(); @@ -5144,115 +5143,5 @@ private static ManagedChannelServiceConfig createManagedChannelServiceConfig( .fromServiceConfig(rawServiceConfig, true, 3, 4, policySelection); } - @Test - public void getChildChannelConfigurer_returnsConfiguredValue() { - ManagedChannelImplBuilder builder = new ManagedChannelImplBuilder(TARGET, - new ClientTransportFactoryBuilder() { - @Override - public ClientTransportFactory buildClientTransportFactory() { - return mockTransportFactory; - } - }, - new FixedPortProvider(DEFAULT_PORT)); - - when(mockTransportFactory.getSupportedSocketAddressTypes()) - .thenReturn(Collections.singleton(InetSocketAddress.class)); - - ManagedChannel channel = builder - .nameResolverFactory(new FakeNameResolverFactory( - Collections.singletonList(URI.create(TARGET)), - Collections.emptyList(), - true, - null)) - .childChannelConfigurer(mockChildChannelConfigurer) - .build(); - - assertThat(channel.getChildChannelConfigurer()).isSameInstanceAs(mockChildChannelConfigurer); - channel.shutdownNow(); - } - - @Test - public void configureChannel_propagatesConfigurerToNewBuilder_endToEnd() { - when(mockTransportFactory.getSupportedSocketAddressTypes()) - .thenReturn(Collections.singleton(InetSocketAddress.class)); - - // 1. Setup Interceptor - final AtomicInteger interceptorCalls = new AtomicInteger(0); - final ClientInterceptor trackingInterceptor = new ClientInterceptor() { - @Override - public ClientCall interceptCall( - MethodDescriptor method, CallOptions callOptions, Channel next) { - interceptorCalls.incrementAndGet(); - return next.newCall(method, callOptions); - } - }; - - // 2. Setup Configurer - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { - @Override - public void accept(ManagedChannelBuilder builder) { - builder.intercept(trackingInterceptor); - } - }; - // 3. Create Parent Channel - ManagedChannelImplBuilder parentBuilder = new ManagedChannelImplBuilder("fake://parent-target", - new ClientTransportFactoryBuilder() { - @Override - public ClientTransportFactory buildClientTransportFactory() { - return mockTransportFactory; - } - }, - new FixedPortProvider(DEFAULT_PORT)); - - ManagedChannel parentChannel = parentBuilder - // MATCH THE CONSTRUCTOR SIGNATURE: (List, List, boolean, Status) - .nameResolverFactory(new FakeNameResolverFactory( - Collections.singletonList(URI.create("fake://parent-target")), - Collections.emptyList(), - true, - null)) - .childChannelConfigurer(configurer) - .build(); - - // 4. Create Child Channel Builder - ManagedChannelImplBuilder childBuilder = new ManagedChannelImplBuilder("fake://child-target", - new ClientTransportFactoryBuilder() { - @Override - public ClientTransportFactory buildClientTransportFactory() { - return mockTransportFactory; - } - }, - new FixedPortProvider(DEFAULT_PORT)); - - childBuilder.configureChannel(parentChannel); - - // Ensure child also has a resolver factory - childBuilder.nameResolverFactory(new FakeNameResolverFactory( - Collections.singletonList(URI.create("fake://child-target")), - Collections.emptyList(), - true, - null)); - - ManagedChannel childChannel = childBuilder.build(); - - // 5. Verification - ClientCall call = childChannel.newCall( - MethodDescriptor.newBuilder() - .setType(MethodDescriptor.MethodType.UNARY) - .setFullMethodName("service/method") - .setRequestMarshaller(new StringMarshaller()) - .setResponseMarshaller(new IntegerMarshaller()) - .build(), - CallOptions.DEFAULT); - - call.start(new ClientCall.Listener() { - }, new Metadata()); - - assertThat(interceptorCalls.get()) - .isEqualTo(1); - - childChannel.shutdownNow(); - parentChannel.shutdownNow(); - } } diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index f41ff62dc7a..d072964464a 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -22,6 +22,7 @@ import io.grpc.CallCredentials; import io.grpc.CallOptions; import io.grpc.ChannelCredentials; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.Context; import io.grpc.Grpc; @@ -29,7 +30,6 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Server; import io.grpc.Status; import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.XdsTransportFactory; @@ -38,20 +38,18 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { private final CallCredentials callCredentials; - private final ManagedChannel parentChannel; - private final Server parentServer; + private final ChildChannelConfigurer childChannelConfigurer; - GrpcXdsTransportFactory(CallCredentials callCredentials, ManagedChannel parentChannel, - Server parentServer) { + GrpcXdsTransportFactory(CallCredentials callCredentials, + ChildChannelConfigurer childChannelConfigurer) { this.callCredentials = callCredentials; - this.parentChannel = parentChannel; - this.parentServer = parentServer; + this.childChannelConfigurer = childChannelConfigurer; } @Override public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { - return new GrpcXdsTransport(serverInfo, callCredentials, parentChannel, parentServer); + return new GrpcXdsTransport(serverInfo, callCredentials, childChannelConfigurer); } @VisibleForTesting @@ -84,15 +82,13 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call } public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials, - ManagedChannel parentChannel, Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { String target = serverInfo.target(); ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig(); ManagedChannelBuilder channelBuilder = Grpc.newChannelBuilder(target, channelCredentials) .keepAliveTime(5, TimeUnit.MINUTES); - if (parentChannel != null) { - channelBuilder.configureChannel(parentChannel); - } else if (parentServer != null) { - channelBuilder.configureChannel(parentServer); + if (childChannelConfigurer != null) { + childChannelConfigurer.accept(channelBuilder); } this.channel = channelBuilder.build(); this.callCredentials = callCredentials; diff --git a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java index 2855a9a7321..06ce7eb6f53 100644 --- a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java @@ -87,7 +87,7 @@ public static XdsClientResult getOrCreate( CallCredentials transportCallCredentials) { return new XdsClientResult(SharedXdsClientPoolProvider.getDefaultProvider() .getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, - null, null)); + null)); } /** diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index fe08b594420..29af3849b20 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -22,9 +22,8 @@ import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.grpc.CallCredentials; -import io.grpc.ManagedChannel; +import io.grpc.ChildChannelConfigurer; import io.grpc.MetricRecorder; -import io.grpc.Server; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; @@ -95,22 +94,22 @@ public ObjectPool getOrCreate( bootstrapInfo = GrpcBootstrapperImpl.defaultBootstrap(); } return getOrCreate(target, bootstrapInfo, metricRecorder, transportCallCredentials, - null, null); + null); } @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder) { return getOrCreate(target, bootstrapInfo, metricRecorder, null, - null, null); + null); } @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel, Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { return getOrCreate(target, bootstrapInfo, metricRecorder, null, - null, parentServer); + childChannelConfigurer); } public ObjectPool getOrCreate( @@ -118,8 +117,7 @@ public ObjectPool getOrCreate( BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, CallCredentials transportCallCredentials, - ManagedChannel parentChannel, - Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { ObjectPool ref = targetToXdsClientMap.get(target); if (ref == null) { synchronized (lock) { @@ -127,8 +125,8 @@ public ObjectPool getOrCreate( if (ref == null) { ref = new RefCountedXdsClientObjectPool( - bootstrapInfo, target, metricRecorder, transportCallCredentials, parentChannel, - parentServer); + bootstrapInfo, target, metricRecorder, transportCallCredentials, + childChannelConfigurer); targetToXdsClientMap.put(target, ref); } } @@ -153,8 +151,7 @@ class RefCountedXdsClientObjectPool implements ObjectPool { private final String target; // The target associated with the xDS client. private final MetricRecorder metricRecorder; private final CallCredentials transportCallCredentials; - private final ManagedChannel parentChannel; - private final Server parentServer; + private final ChildChannelConfigurer childChannelConfigurer; private final Object lock = new Object(); @GuardedBy("lock") private ScheduledExecutorService scheduler; @@ -168,8 +165,7 @@ class RefCountedXdsClientObjectPool implements ObjectPool { @VisibleForTesting RefCountedXdsClientObjectPool( BootstrapInfo bootstrapInfo, String target, MetricRecorder metricRecorder) { - this(bootstrapInfo, target, metricRecorder, null, null, - null); + this(bootstrapInfo, target, metricRecorder, null, null); } @VisibleForTesting @@ -178,14 +174,12 @@ class RefCountedXdsClientObjectPool implements ObjectPool { String target, MetricRecorder metricRecorder, CallCredentials transportCallCredentials, - ManagedChannel parentChannel, - Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { this.bootstrapInfo = checkNotNull(bootstrapInfo, "bootstrapInfo"); this.target = target; this.metricRecorder = checkNotNull(metricRecorder, "metricRecorder"); this.transportCallCredentials = transportCallCredentials; - this.parentChannel = parentChannel; - this.parentServer = parentServer; + this.childChannelConfigurer = childChannelConfigurer; } @Override @@ -198,7 +192,7 @@ public XdsClient getObject() { scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE); metricReporter = new XdsClientMetricReporterImpl(metricRecorder, target); GrpcXdsTransportFactory xdsTransportFactory = - new GrpcXdsTransportFactory(transportCallCredentials, parentChannel, parentServer); + new GrpcXdsTransportFactory(transportCallCredentials, childChannelConfigurer); xdsClient = new XdsClientImpl( xdsTransportFactory, diff --git a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java index 5d9c63301c7..4984fdf9f74 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java @@ -16,9 +16,8 @@ package io.grpc.xds; -import io.grpc.ManagedChannel; +import io.grpc.ChildChannelConfigurer; import io.grpc.MetricRecorder; -import io.grpc.Server; import io.grpc.internal.ObjectPool; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; import io.grpc.xds.client.XdsClient; @@ -34,7 +33,7 @@ ObjectPool getOrCreate( ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel, Server parentServer); + ChildChannelConfigurer childChannelConfigurer); List getTargets(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 6f60008151d..47b2dee2e50 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -31,6 +31,7 @@ import io.grpc.Attributes; import io.grpc.CallOptions; import io.grpc.Channel; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -39,7 +40,6 @@ import io.grpc.InternalConfigSelector; import io.grpc.InternalLogId; import io.grpc.LoadBalancer.PickSubchannelArgs; -import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MetricRecorder; @@ -184,7 +184,7 @@ final class XdsNameResolver extends NameResolver { checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); this.xdsClientPool = new BootstrappingXdsClientPool( xdsClientPoolFactory, target, bootstrapOverride, metricRecorder, - nameResolverArgs.getParentChannel()); + nameResolverArgs.getChildChannelConfigurer()); } this.random = checkNotNull(random, "random"); this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry"); @@ -1056,19 +1056,19 @@ private static final class BootstrappingXdsClientPool implements XdsClientPool { private final @Nullable Map bootstrapOverride; private final @Nullable MetricRecorder metricRecorder; private ObjectPool xdsClientPool; - private ManagedChannel parentChannel; + private ChildChannelConfigurer childChannelConfigurer; BootstrappingXdsClientPool( XdsClientPoolFactory xdsClientPoolFactory, String target, @Nullable Map bootstrapOverride, @Nullable MetricRecorder metricRecorder, - @Nullable ManagedChannel parentChannel) { + @Nullable ChildChannelConfigurer childChannelConfigurer) { this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); this.target = checkNotNull(target, "target"); this.bootstrapOverride = bootstrapOverride; this.metricRecorder = metricRecorder; - this.parentChannel = parentChannel; + this.childChannelConfigurer = childChannelConfigurer; } @Override @@ -1081,8 +1081,8 @@ public XdsClient getObject() throws XdsInitializationException { bootstrapInfo = new GrpcBootstrapperImpl().bootstrap(bootstrapOverride); } this.xdsClientPool = - xdsClientPoolFactory.getOrCreate(target, bootstrapInfo, metricRecorder, parentChannel, - null); + xdsClientPoolFactory.getOrCreate( + target, bootstrapInfo, metricRecorder, childChannelConfigurer); } return xdsClientPool.getObject(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index bbfa12eb26f..cccc0161d3d 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -26,7 +26,6 @@ import com.google.errorprone.annotations.DoNotCall; import io.grpc.Attributes; import io.grpc.ChildChannelConfigurer; -import io.grpc.ChildChannelConfigurers; import io.grpc.ExperimentalApi; import io.grpc.ForwardingServerBuilder; import io.grpc.Internal; @@ -60,7 +59,7 @@ public final class XdsServerBuilder extends ForwardingServerBuilder bootstrapOverride; private long drainGraceTime = 10; private TimeUnit drainGraceTimeUnit = TimeUnit.MINUTES; - private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); + private ChildChannelConfigurer childChannelConfigurer = builder -> { }; private XdsServerBuilder(NettyServerBuilder nettyDelegate, int port) { this.delegate = nettyDelegate; diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index 54a33752ed2..e9362d251ce 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -30,7 +30,6 @@ import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; import io.grpc.Attributes; import io.grpc.ChildChannelConfigurer; -import io.grpc.ChildChannelConfigurers; import io.grpc.InternalServerInterceptors; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -130,14 +129,7 @@ public void uncaughtException(Thread t, Throwable e) { // NamedFilterConfig.filterStateKey -> filter_instance. private final HashMap activeFiltersDefaultChain = new HashMap<>(); - /** - * Stores the user-provided configuration function for internal child channels. - * - *

This is intended for use by gRPC internal components - * that are responsible for creating auxiliary {@code ManagedChannel} instances. - * guaranteed to be not null (defaults to no-op). - */ - private ChildChannelConfigurer childChannelConfigurer = ChildChannelConfigurers.noOp(); + private ChildChannelConfigurer childChannelConfigurer = builder -> { }; XdsServerWrapper( String listenerAddress, @@ -238,7 +230,7 @@ private void internalStart() { } xdsClientPool = xdsClientPoolFactory.getOrCreate( "#server", bootstrapInfo, new MetricRecorder() {}, - null, this); + childChannelConfigurer); } catch (Exception e) { StatusException statusException = Status.UNAVAILABLE.withDescription( "Failed to initialize xDS").withCause(e).asException(); @@ -264,18 +256,6 @@ private void internalStart() { discoveryState = new DiscoveryState(listenerTemplate.replaceAll("%s", replacement)); } - /** - * Retrieves the user-provided configuration function for internal child channels. - * - *

This method is intended for use by gRPC internal components (NameResolvers, LoadBalancers) - * that are responsible for creating auxiliary {@code ManagedChannel} instances. - * - * @return the ChildChannelConfigurer, guaranteed to be not null (defaults to no-op). - */ - @Override - public ChildChannelConfigurer getChildChannelConfigurer() { - return childChannelConfigurer; - } @Override public Server shutdown() { diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 74410b8f01c..3dbbc802db2 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -37,11 +37,10 @@ import io.envoyproxy.envoy.service.status.v3.ClientStatusRequest; import io.envoyproxy.envoy.service.status.v3.ClientStatusResponse; import io.envoyproxy.envoy.type.matcher.v3.NodeMatcher; +import io.grpc.ChildChannelConfigurer; import io.grpc.Deadline; import io.grpc.InsecureChannelCredentials; -import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; -import io.grpc.Server; import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusRuntimeException; @@ -523,7 +522,7 @@ public ObjectPool getOrCreate( @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel, Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { throw new UnsupportedOperationException("Should not be called"); } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index ff9b4076c0a..caad48a54e9 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -5068,7 +5068,7 @@ public void serverFailureMetricReport_forRetryAndBackoff() { private XdsClientImpl createXdsClient(String serverUri) { BootstrapInfo bootstrapInfo = buildBootStrap(serverUri); return new XdsClientImpl( - new GrpcXdsTransportFactory(null, null, null), + new GrpcXdsTransportFactory(null, null), bootstrapInfo, fakeClock.getScheduledExecutorService(), backoffPolicyProvider, diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index 36b91120bf3..4569a000962 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -34,7 +34,6 @@ import io.grpc.Grpc; import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureServerCredentials; -import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import io.grpc.MethodDescriptor; import io.grpc.Server; @@ -102,7 +101,7 @@ public void onCompleted() { @Test public void callApis() throws Exception { XdsTransportFactory.XdsTransport xdsTransport = - new GrpcXdsTransportFactory(null, null, null) + new GrpcXdsTransportFactory(null, null) .create( Bootstrapper.ServerInfo.create( "localhost:" + server.getPort(), InsecureChannelCredentials.create())); @@ -162,15 +161,10 @@ public void verifyConfigApplied_interceptor() { // Create Configurer that adds the interceptor ChildChannelConfigurer configurer = (builder) -> builder.intercept(mockInterceptor); - // Mock Parent Channel - ManagedChannel mockParentChannel = mock(ManagedChannel.class); - when(mockParentChannel.getChildChannelConfigurer()).thenReturn(configurer); - // Create Factory GrpcXdsTransportFactory factory = new GrpcXdsTransportFactory( null, - mockParentChannel, - null); + configurer); // Create Transport XdsTransportFactory.XdsTransport transport = factory.create( @@ -196,27 +190,55 @@ public void verifyConfigApplied_interceptor() { } @Test - public void useParentServerConfig() { - // 1. Mock Server and Configurer - Server mockServer = mock(Server.class); + public void useChildChannelConfigurer() { + // Mock Configurer ChildChannelConfigurer mockConfigurer = mock(ChildChannelConfigurer.class); - when(mockServer.getChildChannelConfigurer()).thenReturn(mockConfigurer); - // 2. Create Factory with Parent Server + // Create Factory GrpcXdsTransportFactory factory = new GrpcXdsTransportFactory( null, // CallCredentials - null, // Parent Channel - mockServer); + mockConfigurer); - // 3. Create Transport (triggers channel creation) + // Create Transport (triggers channel creation) XdsTransportFactory.XdsTransport transport = factory.create( Bootstrapper.ServerInfo.create("localhost:8080", InsecureChannelCredentials.create())); - // 4. Verify Configurer was accessed and applied - verify(mockServer).getChildChannelConfigurer(); + // Verify Configurer was accessed and applied verify(mockConfigurer).accept(any(ManagedChannelBuilder.class)); transport.shutdown(); } + + @Test + public void verifyConfigApplied_maxInboundMessageSize() { + // Create a mock Builder + ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); + + // Create Configurer that modifies message size + ChildChannelConfigurer configurer = (builder) -> builder.maxInboundMessageSize(1024); + + // Apply configurer to builder + configurer.accept(mockBuilder); + + // Verify builder was modified + verify(mockBuilder).maxInboundMessageSize(1024); + } + + @Test + public void verifyConfigApplied_interceptors() { + ClientInterceptor interceptor1 = mock(ClientInterceptor.class); + ClientInterceptor interceptor2 = mock(ClientInterceptor.class); + + ChildChannelConfigurer configurer = builder -> { + builder.intercept(interceptor1); + builder.intercept(interceptor2); + }; + + ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); + configurer.accept(mockBuilder); + + verify(mockBuilder).intercept(interceptor1); + verify(mockBuilder).intercept(interceptor2); + } } diff --git a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java index 55dc862cc38..bfc7db19750 100644 --- a/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java +++ b/xds/src/test/java/io/grpc/xds/LoadReportClientTest.java @@ -181,7 +181,7 @@ public void cancelled(Context context) { lrsClient = new LoadReportClient( loadStatsManager, - new GrpcXdsTransportFactory(null, null, null).createForTest(channel), + new GrpcXdsTransportFactory(null, null).createForTest(channel), NODE, syncContext, fakeClock.getScheduledExecutorService(), diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index c775037e0d5..c3963dbac74 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -28,7 +28,11 @@ import com.google.auth.oauth2.OAuth2Credentials; import com.google.common.util.concurrent.SettableFuture; import io.grpc.CallCredentials; +import io.grpc.ChildChannelConfigurer; +import io.grpc.ClientInterceptor; import io.grpc.Grpc; +import io.grpc.Grpc; +import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureServerCredentials; import io.grpc.Metadata; @@ -208,7 +212,7 @@ public void xdsClient_usesCallCredentials() throws Exception { // Create xDS client that uses the CallCredentials on the transport ObjectPool xdsClientPool = provider.getOrCreate("target", bootstrapInfo, metricRecorder, sampleCreds, - null, null); + null); XdsClient xdsClient = xdsClientPool.getObject(); xdsClient.watchXdsResource( XdsListenerResource.getInstance(), "someLDSresource", ldsResourceWatcher); @@ -221,4 +225,59 @@ public void xdsClient_usesCallCredentials() throws Exception { xdsClientPool.returnObject(xdsClient); xdsServer.shutdownNow(); } + + @Test + public void xdsClient_usesChildChannelConfigurer() throws Exception { + // Set up fake xDS server + XdsTestControlPlaneService fakeXdsService = new XdsTestControlPlaneService(); + CallCredsServerInterceptor callInterceptor = new CallCredsServerInterceptor(); + Server xdsServer = + Grpc.newServerBuilderForPort(0, InsecureServerCredentials.create()) + .addService(fakeXdsService) + .intercept(callInterceptor) + .build() + .start(); + String xdsServerUri = "localhost:" + xdsServer.getPort(); + + // Set up bootstrap & xDS client pool provider + ServerInfo server = ServerInfo.create(xdsServerUri, InsecureChannelCredentials.create()); + BootstrapInfo bootstrapInfo = + BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); + SharedXdsClientPoolProvider provider = new SharedXdsClientPoolProvider(); + + // Create a client interceptor that actually just injects a test token + ClientInterceptor testInterceptor = new ClientInterceptor() { + @Override + public io.grpc.ClientCall interceptCall( + io.grpc.MethodDescriptor method, + io.grpc.CallOptions callOptions, + io.grpc.Channel next) { + return new io.grpc.ForwardingClientCall.SimpleForwardingClientCall( + next.newCall(method, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + headers.put(AUTHORIZATION_METADATA_KEY, "Bearer test-configurer-token"); + super.start(responseListener, headers); + } + }; + } + }; + + ChildChannelConfigurer configurer = builder -> builder.intercept(testInterceptor); + + // Create xDS client that uses the ChildChannelConfigurer on the transport + ObjectPool xdsClientPool = + provider.getOrCreate("target", bootstrapInfo, metricRecorder, null, configurer); + XdsClient xdsClient = xdsClientPool.getObject(); + xdsClient.watchXdsResource( + XdsListenerResource.getInstance(), "someLDSresource", ldsResourceWatcher); + + // Wait for xDS server to get the request and verify that it received the token from configurer + assertThat(callInterceptor.getTokenWithTimeout(5, TimeUnit.SECONDS)) + .isEqualTo("Bearer test-configurer-token"); + + // Clean up + xdsClientPool.returnObject(xdsClient); + xdsServer.shutdownNow(); + } } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java index 09b330a2d43..4d5e7d09ad4 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java @@ -484,7 +484,7 @@ public void fallbackFromBadUrlToGoodOne() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(garbageUri, validUri), - new GrpcXdsTransportFactory(null, null, null), + new GrpcXdsTransportFactory(null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, @@ -509,7 +509,7 @@ public void testGoodUrlFollowedByBadUrl() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(validUri, garbageUri), - new GrpcXdsTransportFactory(null, null, null), + new GrpcXdsTransportFactory(null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, @@ -536,7 +536,7 @@ public void testTwoBadUrl() { XdsClientImpl client = CommonBootstrapperTestUtils.createXdsClient( Arrays.asList(garbageUri1, garbageUri2), - new GrpcXdsTransportFactory(null, null, null), + new GrpcXdsTransportFactory(null, null), fakeClock, new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index d7058438a00..11c961c68ac 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -29,7 +29,6 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -50,6 +49,7 @@ import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ChannelLogger; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -59,7 +59,6 @@ import io.grpc.InternalConfigSelector.Result; import io.grpc.LoadBalancer.PickDetailsConsumer; import io.grpc.LoadBalancer.PickSubchannelArgs; -import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; @@ -71,7 +70,6 @@ import io.grpc.NoopClientCall; import io.grpc.NoopClientCall.NoopClientCallListener; import io.grpc.ProxyDetector; -import io.grpc.Server; import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusOr; @@ -2501,7 +2499,7 @@ public XdsClient returnObject(Object object) { @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel, Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { targets.add(target); return new ObjectPool() { @Override @@ -2957,17 +2955,16 @@ void deliverErrorStatus() { @Test public void start_passesParentChannelToClientPoolFactory() { - // Create a mock Parent Channel - ManagedChannel mockParentChannel = mock(ManagedChannel.class); + ChildChannelConfigurer mockChildChannelConfigurer = mock(ChildChannelConfigurer.class); - // Build NameResolver.Args containing the parent channel + // Build NameResolver.Args containing the child channel configurer NameResolver.Args args = NameResolver.Args.newBuilder() .setDefaultPort(8080) .setProxyDetector(mock(ProxyDetector.class)) .setSynchronizationContext(syncContext) .setServiceConfigParser(serviceConfigParser) .setChannelLogger(mock(ChannelLogger.class)) - .setParentChannel(mockParentChannel) + .setChildChannelConfigurer(mockChildChannelConfigurer) .build(); // Mock the XdsClientPoolFactory @@ -2982,8 +2979,7 @@ public void start_passesParentChannelToClientPoolFactory() { anyString(), any(BootstrapInfo.class), any(MetricRecorder.class), - any(ManagedChannel.class), - any())) + any(ChildChannelConfigurer.class))) .thenReturn(mockObjectPool); XdsNameResolver resolver = new XdsNameResolver( @@ -3004,13 +3000,11 @@ public void start_passesParentChannelToClientPoolFactory() { // Start the resolver (this triggers the factory call) resolver.start(mockListener); - // Ensure the factory's getOrCreate method was called with parent channel verify(mockPoolFactory).getOrCreate( eq(AUTHORITY), any(BootstrapInfo.class), eq(metricRecorder), - eq(mockParentChannel), - isNull()); + eq(mockChildChannelConfigurer)); resolver.shutdown(); } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index 8ca0a96d0d2..5fe1c7feaeb 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -324,7 +324,7 @@ public void testOverrideBootstrap() throws Exception { buildBuilder(null); builder.overrideBootstrapForTest(b); xdsServer = cleanupRule.register((XdsServerWrapper) builder.build()); - Future unused = startServerAsync(); + Future unused = startServerAsync(); assertThat(xdsClientPoolFactory.savedBootstrapInfo.node().getId()) .isEqualTo(XdsServerTestHelper.BOOTSTRAP_INFO.node().getId()); } @@ -336,20 +336,17 @@ public void start_passesParentServerToClientPoolFactory() throws Exception { @SuppressWarnings("unchecked") ObjectPool mockPool = mock(ObjectPool.class); when(mockPool.getObject()).thenReturn(xdsClient); - when(mockPoolFactory.getOrCreate(any(), any(), any(), any(), any())).thenReturn(mockPool); + when(mockPoolFactory.getOrCreate(any(), any(), any(), any())).thenReturn(mockPool); buildBuilder(null); builder.childChannelConfigurer(mockConfigurer); builder.xdsClientPoolFactory(mockPoolFactory); xdsServer = cleanupRule.register((XdsServerWrapper) builder.build()); - Future unused = startServerAsync(); + Future unused = startServerAsync(); // Verify getOrCreate called with the server instance verify(mockPoolFactory).getOrCreate( - any(), any(), any(), org.mockito.ArgumentMatchers.isNull(), - org.mockito.ArgumentMatchers.eq(xdsServer)); - - assertThat(xdsServer.getChildChannelConfigurer()).isSameInstanceAs(mockConfigurer); + any(), any(), any(), org.mockito.ArgumentMatchers.eq(mockConfigurer)); } } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index 9f32d0aab4b..f611e013b3a 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -22,10 +22,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; +import io.grpc.ChildChannelConfigurer; import io.grpc.InsecureChannelCredentials; -import io.grpc.ManagedChannel; import io.grpc.MetricRecorder; -import io.grpc.Server; import io.grpc.Status; import io.grpc.StatusOr; import io.grpc.internal.ObjectPool; @@ -187,7 +186,7 @@ public XdsClient returnObject(Object object) { @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ManagedChannel parentChannel, Server parentServer) { + ChildChannelConfigurer childChannelConfigurer) { this.savedBootstrapInfo = bootstrapInfo; return new ObjectPool() { @Override From d8398f51673792db97285b468fdfdddea8e5e9d2 Mon Sep 17 00:00:00 2001 From: agravator Date: Mon, 16 Mar 2026 12:31:10 +0530 Subject: [PATCH 08/10] expose APIs similar to internalConfigurator --- .../java/io/grpc/ChildChannelConfigurer.java | 24 +++--- .../io/grpc/ChildChannelConfigurerTest.java | 41 ++++++++++ .../test/java/io/grpc/NameResolverTest.java | 4 +- .../io/grpc/internal/ManagedChannelImpl.java | 6 +- .../internal/ManagedChannelImplBuilder.java | 2 +- .../ManagedChannelImplBuilderTest.java | 15 ++-- .../grpc/internal/ManagedChannelImplTest.java | 5 +- .../io/grpc/xds/GrpcXdsTransportFactory.java | 2 +- .../java/io/grpc/xds/XdsServerBuilder.java | 2 +- .../java/io/grpc/xds/XdsServerWrapper.java | 2 +- .../FakeControlPlaneXdsIntegrationTest.java | 74 +++++++++++++++++++ .../grpc/xds/GrpcXdsTransportFactoryTest.java | 29 ++++++-- .../xds/SharedXdsClientPoolProviderTest.java | 10 ++- 13 files changed, 177 insertions(+), 39 deletions(-) create mode 100644 api/src/test/java/io/grpc/ChildChannelConfigurerTest.java diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurer.java b/api/src/main/java/io/grpc/ChildChannelConfigurer.java index 4392c7f7bae..821162809dd 100644 --- a/api/src/main/java/io/grpc/ChildChannelConfigurer.java +++ b/api/src/main/java/io/grpc/ChildChannelConfigurer.java @@ -16,7 +16,7 @@ package io.grpc; -import java.util.function.Consumer; + /** * A configurer for child channels created by gRPC's internal infrastructure. @@ -44,14 +44,13 @@ * .build(); * } * - *

Implementations must be thread-safe as {@link #accept} may be invoked concurrently + *

Implementations must be thread-safe as the configure methods may be invoked concurrently * by multiple internal components. * * @since 1.79.0 */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") -@FunctionalInterface -public interface ChildChannelConfigurer extends Consumer> { +public interface ChildChannelConfigurer { /** * Configures a builder for a new child channel. @@ -59,12 +58,17 @@ public interface ChildChannelConfigurer extends ConsumerThis method is invoked synchronously during the creation of the child channel, * before {@link ManagedChannelBuilder#build()} is called. * - *

Note: The provided {@code builder} is generic (`?`). Implementations should use - * universal configuration methods (like {@code intercept()}, {@code userAgent()}) on the - * builder rather than casting it to specific implementation types. - * * @param builder the mutable channel builder for the new child channel */ - @Override - void accept(ManagedChannelBuilder builder); + default void configureChannelBuilder(ManagedChannelBuilder builder) {} + + /** + * Configures a builder for a new child server. + * + *

This method is invoked synchronously during the creation of the child server, + * before {@link ServerBuilder#build()} is called. + * + * @param builder the mutable server builder for the new child server + */ + default void configureServerBuilder(ServerBuilder builder) {} } diff --git a/api/src/test/java/io/grpc/ChildChannelConfigurerTest.java b/api/src/test/java/io/grpc/ChildChannelConfigurerTest.java new file mode 100644 index 00000000000..cfafe7b8958 --- /dev/null +++ b/api/src/test/java/io/grpc/ChildChannelConfigurerTest.java @@ -0,0 +1,41 @@ +/* + * Copyright 2026 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ChildChannelConfigurerTest { + + @Test + public void defaultMethods_doNothing() { + ChildChannelConfigurer configurer = new ChildChannelConfigurer() {}; + + ManagedChannelBuilder mockChannelBuilder = mock(ManagedChannelBuilder.class); + configurer.configureChannelBuilder(mockChannelBuilder); + verifyNoInteractions(mockChannelBuilder); + + ServerBuilder mockServerBuilder = mock(ServerBuilder.class); + configurer.configureServerBuilder(mockServerBuilder); + verifyNoInteractions(mockServerBuilder); + } +} diff --git a/api/src/test/java/io/grpc/NameResolverTest.java b/api/src/test/java/io/grpc/NameResolverTest.java index d2219a1525f..85a3e45a6bd 100644 --- a/api/src/test/java/io/grpc/NameResolverTest.java +++ b/api/src/test/java/io/grpc/NameResolverTest.java @@ -147,8 +147,8 @@ public void uncaughtException(Thread t, Throwable e) { // Validate configurer accepts builders ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); - args.getChildChannelConfigurer().accept(mockBuilder); - verify(childChannelConfigurer).accept(mockBuilder); + args.getChildChannelConfigurer().configureChannelBuilder(mockBuilder); + verify(childChannelConfigurer).configureChannelBuilder(mockBuilder); } @Test diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index c19a8f49221..18e5ad333fc 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -155,7 +155,7 @@ public Result selectConfig(PickSubchannelArgs args) { private static final LoadBalancer.PickDetailsConsumer NOOP_PICK_DETAILS_CONSUMER = new LoadBalancer.PickDetailsConsumer() {}; - private ChildChannelConfigurer childChannelConfigurer = builder -> {}; + private ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; private final InternalLogId logId; private final String target; @@ -1580,7 +1580,9 @@ protected ManagedChannelBuilder delegate() { // Note that we follow the global configurator pattern and try to fuse the configurations as // soon as the builder gets created - builder.childChannelConfigurer(childChannelConfigurer); + if (childChannelConfigurer != null) { + childChannelConfigurer.configureChannelBuilder(builder); + } return builder // TODO(zdapeng): executors should not outlive the parent channel. diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 6309ef3340b..f33c4141320 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -125,7 +125,7 @@ public static ManagedChannelBuilder forTarget(String target) { private static final Method GET_CLIENT_INTERCEPTOR_METHOD; - ChildChannelConfigurer childChannelConfigurer = builder -> {}; + ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; @Override public ManagedChannelImplBuilder childChannelConfigurer( diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java index 4faae81cc04..c954a05b21b 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java @@ -786,11 +786,14 @@ public void childChannelConfigurer_propagatesMetricsAndInterceptors_xdsTarget() ClientInterceptor mockInterceptor = mock(ClientInterceptor.class); // Define the Configurer - ChildChannelConfigurer configurer = (builder) -> { - builder.addMetricSink(mockMetricSink); - - // Assuming InternalInterceptorFactory is also accessible - builder.interceptWithTarget(target -> mockInterceptor); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + builder.addMetricSink(mockMetricSink); + + // Assuming InternalInterceptorFactory is also accessible + builder.interceptWithTarget(target -> mockInterceptor); + } }; // Mock NameResolver.Factory to capture Args @@ -833,7 +836,7 @@ public void childChannelConfigurer_propagatesMetricsAndInterceptors_xdsTarget() // Stub addMetricSink to return the builder to avoid generic return type issues doReturn(mockChildBuilder).when(mockChildBuilder).addMetricSink(any()); - configurer.accept(mockChildBuilder); + configurer.configureChannelBuilder(mockChildBuilder); verify(mockChildBuilder).addMetricSink(mockMetricSink); verify(mockChildBuilder).interceptWithTarget(any()); } diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 87abd17c632..4977bb249e6 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -69,7 +69,6 @@ import io.grpc.Channel; import io.grpc.ChannelCredentials; import io.grpc.ChannelLogger; -import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -102,7 +101,6 @@ import io.grpc.LoadBalancerRegistry; import io.grpc.LongCounterMetricInstrument; import io.grpc.ManagedChannel; -import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; @@ -306,8 +304,6 @@ public String getPolicyName() { private ArgumentCaptor resolvedAddressCaptor; private ArgumentCaptor streamListenerCaptor = ArgumentCaptor.forClass(ClientStreamListener.class); - @Mock - private ChildChannelConfigurer mockChildChannelConfigurer; private void createChannel(ClientInterceptor... interceptors) { @@ -3775,6 +3771,7 @@ public void channelsAndSubchannels_oob_instrumented_state() throws Exception { assertEquals(SHUTDOWN, getStats(channel).state); assertEquals(SHUTDOWN, getStats(oobChannel).state); } + @Test public void binaryLogInstalled() throws Exception { final SettableFuture intercepted = SettableFuture.create(); diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index d072964464a..c5f0ff6bdc3 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -88,7 +88,7 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call ManagedChannelBuilder channelBuilder = Grpc.newChannelBuilder(target, channelCredentials) .keepAliveTime(5, TimeUnit.MINUTES); if (childChannelConfigurer != null) { - childChannelConfigurer.accept(channelBuilder); + childChannelConfigurer.configureChannelBuilder(channelBuilder); } this.channel = channelBuilder.build(); this.callCredentials = callCredentials; diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index cccc0161d3d..5628367f502 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -59,7 +59,7 @@ public final class XdsServerBuilder extends ForwardingServerBuilder bootstrapOverride; private long drainGraceTime = 10; private TimeUnit drainGraceTimeUnit = TimeUnit.MINUTES; - private ChildChannelConfigurer childChannelConfigurer = builder -> { }; + private ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; private XdsServerBuilder(NettyServerBuilder nettyDelegate, int port) { this.delegate = nettyDelegate; diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index e9362d251ce..0c69fda7b3e 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -129,7 +129,7 @@ public void uncaughtException(Thread t, Throwable e) { // NamedFilterConfig.filterStateKey -> filter_instance. private final HashMap activeFiltersDefaultChain = new HashMap<>(); - private ChildChannelConfigurer childChannelConfigurer = builder -> { }; + private ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; XdsServerWrapper( String listenerAddress, diff --git a/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java b/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java index c8f2b8932ef..fd856a51995 100644 --- a/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java +++ b/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java @@ -22,6 +22,13 @@ import static io.grpc.xds.XdsTestControlPlaneService.ADS_TYPE_URL_CDS; import static io.grpc.xds.XdsTestControlPlaneService.ADS_TYPE_URL_EDS; import static org.junit.Assert.assertEquals; +import static org.mockito.AdditionalAnswers.delegatesTo; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; import com.github.xds.type.v3.TypedStruct; import com.google.common.collect.ImmutableMap; @@ -47,15 +54,23 @@ import io.envoyproxy.envoy.extensions.load_balancing_policies.wrr_locality.v3.WrrLocality; import io.grpc.CallOptions; import io.grpc.Channel; +import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientStreamTracer; import io.grpc.ForwardingClientCall.SimpleForwardingClientCall; import io.grpc.ForwardingClientCallListener; +import io.grpc.Grpc; +import io.grpc.InsecureChannelCredentials; +import io.grpc.InsecureServerCredentials; import io.grpc.LoadBalancerRegistry; import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.MetricSink; +import io.grpc.NoopMetricSink; +import io.grpc.Server; import io.grpc.testing.protobuf.SimpleRequest; import io.grpc.testing.protobuf.SimpleResponse; import io.grpc.testing.protobuf.SimpleServiceGrpc; @@ -339,4 +354,63 @@ public void pingPong_logicalDns_authorityOverride() { System.clearProperty("GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE"); } } + + @Test + public void childChannelConfigurer_passesMetricSinkToChannel_E2E() { + MetricSink mockSink = mock(MetricSink.class, delegatesTo(new NoopMetricSink())); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + builder.addMetricSink(mockSink); + } + }; + + ManagedChannel channel = Grpc.newChannelBuilder("test-xds:///test-server", + InsecureChannelCredentials.create()) + .childChannelConfigurer(configurer) + .build(); + + try { + SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = SimpleServiceGrpc.newBlockingStub( + channel); + blockingStub.unaryRpc(SimpleRequest.getDefaultInstance()); + + // The xDS client inside the channel configurer will have created an ADS stream. + // The metric sink should have received attempt or connection metrics. + verify(mockSink, timeout(5000).atLeastOnce()) + .addLongCounter(any(), anyLong(), anyList(), anyList()); + } finally { + channel.shutdownNow(); + } + } + + @Test + public void childChannelConfigurer_passesMetricSinkToServer_E2E() throws Exception { + MetricSink mockSink = mock(MetricSink.class, delegatesTo(new NoopMetricSink())); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + // Child channels (xDS client connections) created by this server get the sink. + builder.addMetricSink(mockSink); + } + }; + + // We start an XdsServer manually. + // XdsServer needs RDS, LDS, etc. from control plane. + XdsServerBuilder serverBuilder = XdsServerBuilder.forPort( + 0, InsecureServerCredentials.create()) + .addService(new SimpleServiceGrpc.SimpleServiceImplBase() {}) + .overrideBootstrapForTest(controlPlane.defaultBootstrapOverride()) + .childChannelConfigurer(configurer); + + Server childServer = serverBuilder.build().start(); + + try { + // The server xDS client will connect to control plane to get LDS. + verify(mockSink, timeout(5000).atLeastOnce()) + .addLongCounter(any(), anyLong(), anyList(), anyList()); + } finally { + childServer.shutdownNow(); + } + } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index 4569a000962..f851935c200 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -159,7 +159,12 @@ public void verifyConfigApplied_interceptor() { .thenReturn(new io.grpc.NoopClientCall<>()); // Create Configurer that adds the interceptor - ChildChannelConfigurer configurer = (builder) -> builder.intercept(mockInterceptor); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + builder.intercept(mockInterceptor); + } + }; // Create Factory GrpcXdsTransportFactory factory = new GrpcXdsTransportFactory( @@ -204,7 +209,7 @@ public void useChildChannelConfigurer() { Bootstrapper.ServerInfo.create("localhost:8080", InsecureChannelCredentials.create())); // Verify Configurer was accessed and applied - verify(mockConfigurer).accept(any(ManagedChannelBuilder.class)); + verify(mockConfigurer).configureChannelBuilder(any(ManagedChannelBuilder.class)); transport.shutdown(); } @@ -215,10 +220,15 @@ public void verifyConfigApplied_maxInboundMessageSize() { ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); // Create Configurer that modifies message size - ChildChannelConfigurer configurer = (builder) -> builder.maxInboundMessageSize(1024); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + builder.maxInboundMessageSize(1024); + } + }; // Apply configurer to builder - configurer.accept(mockBuilder); + configurer.configureChannelBuilder(mockBuilder); // Verify builder was modified verify(mockBuilder).maxInboundMessageSize(1024); @@ -229,13 +239,16 @@ public void verifyConfigApplied_interceptors() { ClientInterceptor interceptor1 = mock(ClientInterceptor.class); ClientInterceptor interceptor2 = mock(ClientInterceptor.class); - ChildChannelConfigurer configurer = builder -> { - builder.intercept(interceptor1); - builder.intercept(interceptor2); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + builder.intercept(interceptor1); + builder.intercept(interceptor2); + } }; ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); - configurer.accept(mockBuilder); + configurer.configureChannelBuilder(mockBuilder); verify(mockBuilder).intercept(interceptor1); verify(mockBuilder).intercept(interceptor2); diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index c3963dbac74..bcf97066361 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -31,10 +31,9 @@ import io.grpc.ChildChannelConfigurer; import io.grpc.ClientInterceptor; import io.grpc.Grpc; -import io.grpc.Grpc; -import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureChannelCredentials; import io.grpc.InsecureServerCredentials; +import io.grpc.ManagedChannelBuilder; import io.grpc.Metadata; import io.grpc.MetricRecorder; import io.grpc.Server; @@ -263,7 +262,12 @@ public void start(Listener responseListener, Metadata headers) { } }; - ChildChannelConfigurer configurer = builder -> builder.intercept(testInterceptor); + ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + @Override + public void configureChannelBuilder(ManagedChannelBuilder builder) { + builder.intercept(testInterceptor); + } + }; // Create xDS client that uses the ChildChannelConfigurer on the transport ObjectPool xdsClientPool = From 367640ecbda868207a780a5556dbdac5652f8d67 Mon Sep 17 00:00:00 2001 From: agravator Date: Tue, 17 Mar 2026 20:42:57 +0530 Subject: [PATCH 09/10] fix: refactor from ChildChannelConfigurer to ChannelConfigurer --- ...Configurer.java => ChannelConfigurer.java} | 7 ++--- .../io/grpc/ForwardingChannelBuilder.java | 4 +-- .../io/grpc/ForwardingChannelBuilder2.java | 6 ++-- .../java/io/grpc/ForwardingServerBuilder.java | 4 +-- .../java/io/grpc/ManagedChannelBuilder.java | 11 +++---- api/src/main/java/io/grpc/NameResolver.java | 14 ++++----- api/src/main/java/io/grpc/ServerBuilder.java | 6 ++-- ...erTest.java => ChannelConfigurerTest.java} | 4 +-- .../test/java/io/grpc/NameResolverTest.java | 13 ++++----- .../io/grpc/internal/ManagedChannelImpl.java | 29 +++++++++---------- .../internal/ManagedChannelImplBuilder.java | 11 ++++--- .../ManagedChannelImplBuilderTest.java | 24 +++++++-------- .../grpc/internal/ManagedChannelImplTest.java | 3 +- .../io/grpc/xds/GrpcXdsTransportFactory.java | 19 ++++++------ .../grpc/xds/SharedXdsClientPoolProvider.java | 18 ++++++------ .../io/grpc/xds/XdsClientPoolFactory.java | 4 +-- .../java/io/grpc/xds/XdsNameResolver.java | 10 +++---- .../java/io/grpc/xds/XdsServerBuilder.java | 12 ++++---- .../java/io/grpc/xds/XdsServerWrapper.java | 13 ++++----- .../java/io/grpc/xds/CsdsServiceTest.java | 4 +-- .../FakeControlPlaneXdsIntegrationTest.java | 6 ++-- .../grpc/xds/GrpcXdsTransportFactoryTest.java | 12 ++++---- .../xds/SharedXdsClientPoolProviderTest.java | 8 ++--- .../java/io/grpc/xds/XdsNameResolverTest.java | 12 ++++---- .../io/grpc/xds/XdsServerBuilderTest.java | 4 +-- .../java/io/grpc/xds/XdsServerTestHelper.java | 4 +-- 26 files changed, 127 insertions(+), 135 deletions(-) rename api/src/main/java/io/grpc/{ChildChannelConfigurer.java => ChannelConfigurer.java} (93%) rename api/src/test/java/io/grpc/{ChildChannelConfigurerTest.java => ChannelConfigurerTest.java} (91%) diff --git a/api/src/main/java/io/grpc/ChildChannelConfigurer.java b/api/src/main/java/io/grpc/ChannelConfigurer.java similarity index 93% rename from api/src/main/java/io/grpc/ChildChannelConfigurer.java rename to api/src/main/java/io/grpc/ChannelConfigurer.java index 821162809dd..78288a87ffe 100644 --- a/api/src/main/java/io/grpc/ChildChannelConfigurer.java +++ b/api/src/main/java/io/grpc/ChannelConfigurer.java @@ -32,8 +32,7 @@ *

Usage Example: *

{@code
  * // 1. Define the configurer
- * ChildChannelConfigurer configurer = builder -> {
- *   builder.intercept(new MyAuthInterceptor());
+ * ChannelConfigurer configurer = builder -> {
  *   builder.maxInboundMessageSize(4 * 1024 * 1024);
  * };
  *
@@ -47,10 +46,10 @@
  * 

Implementations must be thread-safe as the configure methods may be invoked concurrently * by multiple internal components. * - * @since 1.79.0 + * @since 1.81.0 */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") -public interface ChildChannelConfigurer { +public interface ChannelConfigurer { /** * Configures a builder for a new child channel. diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java index 6e69b493992..33276d2fdd3 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder.java @@ -244,8 +244,8 @@ public T disableServiceConfigLookUp() { @Override - public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { - delegate().childChannelConfigurer(childChannelConfigurer); + public T childChannelConfigurer(ChannelConfigurer channelConfigurer) { + delegate().childChannelConfigurer(channelConfigurer); return thisT(); } diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java index 2fa60dd6ce1..0fd3f1fb209 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java @@ -95,7 +95,7 @@ public T intercept(ClientInterceptor... interceptors) { } @Override - public T interceptWithTarget(InterceptorFactory factory) { + protected T interceptWithTarget(InterceptorFactory factory) { delegate().interceptWithTarget(factory); return thisT(); } @@ -271,8 +271,8 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { @Override - public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { - delegate().childChannelConfigurer(childChannelConfigurer); + public T childChannelConfigurer(ChannelConfigurer channelConfigurer) { + delegate().childChannelConfigurer(channelConfigurer); return thisT(); } diff --git a/api/src/main/java/io/grpc/ForwardingServerBuilder.java b/api/src/main/java/io/grpc/ForwardingServerBuilder.java index 15ce31bb909..a0478ddad49 100644 --- a/api/src/main/java/io/grpc/ForwardingServerBuilder.java +++ b/api/src/main/java/io/grpc/ForwardingServerBuilder.java @@ -193,8 +193,8 @@ public T setBinaryLog(BinaryLog binaryLog) { } @Override - public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { - delegate().childChannelConfigurer(childChannelConfigurer); + public T childChannelConfigurer(ChannelConfigurer channelConfigurer) { + delegate().childChannelConfigurer(channelConfigurer); return thisT(); } diff --git a/api/src/main/java/io/grpc/ManagedChannelBuilder.java b/api/src/main/java/io/grpc/ManagedChannelBuilder.java index 59b294ddbc4..bf00854f30a 100644 --- a/api/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -160,10 +160,11 @@ public T offloadExecutor(Executor executor) { public abstract T intercept(ClientInterceptor... interceptors); /** - * Adds a factory that will construct an interceptor based on the channel's target. + * Internal-only: Adds a factory that will construct an interceptor based on the channel's target. * This can be used to work around nameResolverFactory() changing the target string. */ - public T interceptWithTarget(InterceptorFactory factory) { + @Internal + protected T interceptWithTarget(InterceptorFactory factory) { throw new UnsupportedOperationException(); } @@ -664,12 +665,12 @@ public T setNameResolverArg(NameResolver.Args.Key key, X value) { *

This allows injecting configuration (like credentials, interceptors, or flow control) * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections. * - * @param childChannelConfigurer the configurer to apply. + * @param channelConfigurer the configurer to apply. * @return this - * @since 1.79.0 + * @since 1.81.0 */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") - public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + public T childChannelConfigurer(ChannelConfigurer channelConfigurer) { throw new UnsupportedOperationException("Not implemented"); } diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index b87508cc9cc..0d70c20dc0b 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -358,7 +358,7 @@ public static final class Args { @Nullable private final MetricRecorder metricRecorder; @Nullable private final NameResolverRegistry nameResolverRegistry; @Nullable private final IdentityHashMap, Object> customArgs; - @Nullable private final ChildChannelConfigurer childChannelConfigurer; + @Nullable private final ChannelConfigurer channelConfigurer; private Args(Builder builder) { this.defaultPort = checkNotNull(builder.defaultPort, "defaultPort not set"); @@ -373,7 +373,7 @@ private Args(Builder builder) { this.metricRecorder = builder.metricRecorder; this.nameResolverRegistry = builder.nameResolverRegistry; this.customArgs = cloneCustomArgs(builder.customArgs); - this.childChannelConfigurer = builder.childChannelConfigurer; + this.channelConfigurer = builder.channelConfigurer; } /** @@ -479,8 +479,8 @@ public ChannelLogger getChannelLogger() { */ @Nullable @Internal - public ChildChannelConfigurer getChildChannelConfigurer() { - return childChannelConfigurer; + public ChannelConfigurer getChildChannelConfigurer() { + return channelConfigurer; } /** @@ -592,7 +592,7 @@ public static final class Builder { private MetricRecorder metricRecorder; private NameResolverRegistry nameResolverRegistry; private IdentityHashMap, Object> customArgs; - private ChildChannelConfigurer childChannelConfigurer; + private ChannelConfigurer channelConfigurer = new ChannelConfigurer() {}; Builder() { } @@ -713,8 +713,8 @@ public Builder setNameResolverRegistry(NameResolverRegistry registry) { * * @since 1.81.0 */ - public Builder setChildChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { - this.childChannelConfigurer = childChannelConfigurer; + public Builder setChildChannelConfigurer(ChannelConfigurer channelConfigurer) { + this.channelConfigurer = channelConfigurer; return this; } diff --git a/api/src/main/java/io/grpc/ServerBuilder.java b/api/src/main/java/io/grpc/ServerBuilder.java index 834f6928c92..54f69d32e0b 100644 --- a/api/src/main/java/io/grpc/ServerBuilder.java +++ b/api/src/main/java/io/grpc/ServerBuilder.java @@ -432,12 +432,12 @@ public T setBinaryLog(BinaryLog binaryLog) { * into auxiliary channels created by gRPC infrastructure, such as xDS control plane connections * or OOB load balancing channels. * - * @param childChannelConfigurer the configurer to apply. + * @param channelConfigurer the configurer to apply. * @return this - * @since 1.79.0 + * @since 1.81.0 */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/12574") - public T childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { + public T childChannelConfigurer(ChannelConfigurer channelConfigurer) { throw new UnsupportedOperationException("Not implemented"); } diff --git a/api/src/test/java/io/grpc/ChildChannelConfigurerTest.java b/api/src/test/java/io/grpc/ChannelConfigurerTest.java similarity index 91% rename from api/src/test/java/io/grpc/ChildChannelConfigurerTest.java rename to api/src/test/java/io/grpc/ChannelConfigurerTest.java index cfafe7b8958..f6e56c81a64 100644 --- a/api/src/test/java/io/grpc/ChildChannelConfigurerTest.java +++ b/api/src/test/java/io/grpc/ChannelConfigurerTest.java @@ -24,11 +24,11 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class ChildChannelConfigurerTest { +public class ChannelConfigurerTest { @Test public void defaultMethods_doNothing() { - ChildChannelConfigurer configurer = new ChildChannelConfigurer() {}; + ChannelConfigurer configurer = new ChannelConfigurer() {}; ManagedChannelBuilder mockChannelBuilder = mock(ManagedChannelBuilder.class); configurer.configureChannelBuilder(mockChannelBuilder); diff --git a/api/src/test/java/io/grpc/NameResolverTest.java b/api/src/test/java/io/grpc/NameResolverTest.java index 85a3e45a6bd..e3864a7665a 100644 --- a/api/src/test/java/io/grpc/NameResolverTest.java +++ b/api/src/test/java/io/grpc/NameResolverTest.java @@ -105,7 +105,7 @@ public void args() { } private NameResolver.Args createArgs() { - ChildChannelConfigurer childChannelConfigurer = mock(ChildChannelConfigurer.class); + ChannelConfigurer channelConfigurer = mock(ChannelConfigurer.class); return NameResolver.Args.newBuilder() .setDefaultPort(defaultPort) .setProxyDetector(proxyDetector) @@ -117,15 +117,14 @@ private NameResolver.Args createArgs() { .setOverrideAuthority(overrideAuthority) .setMetricRecorder(metricRecorder) .setArg(FOO_ARG_KEY, customArgValue) - .setChildChannelConfigurer(childChannelConfigurer) + .setChildChannelConfigurer(channelConfigurer) .build(); } @Test public void args_childChannelConfigurer() { - ChildChannelConfigurer childChannelConfigurer = mock(ChildChannelConfigurer.class); + ChannelConfigurer channelConfigurer = mock(ChannelConfigurer.class); - // Create a real SynchronizationContext instead of mocking it SynchronizationContext realSyncContext = new SynchronizationContext( new Thread.UncaughtExceptionHandler() { @Override @@ -140,15 +139,15 @@ public void uncaughtException(Thread t, Throwable e) { .setSynchronizationContext(realSyncContext) .setServiceConfigParser(mock(NameResolver.ServiceConfigParser.class)) .setChannelLogger(mock(ChannelLogger.class)) - .setChildChannelConfigurer(childChannelConfigurer) + .setChildChannelConfigurer(channelConfigurer) .build(); - assertThat(args.getChildChannelConfigurer()).isSameInstanceAs(childChannelConfigurer); + assertThat(args.getChildChannelConfigurer()).isSameInstanceAs(channelConfigurer); // Validate configurer accepts builders ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); args.getChildChannelConfigurer().configureChannelBuilder(mockBuilder); - verify(childChannelConfigurer).configureChannelBuilder(mockBuilder); + verify(channelConfigurer).configureChannelBuilder(mockBuilder); } @Test diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 26a9095f788..e5e4332b70f 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -37,10 +37,10 @@ import io.grpc.CallCredentials; import io.grpc.CallOptions; import io.grpc.Channel; +import io.grpc.ChannelConfigurer; import io.grpc.ChannelCredentials; import io.grpc.ChannelLogger; import io.grpc.ChannelLogger.ChannelLogLevel; -import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -156,7 +156,13 @@ public Result selectConfig(PickSubchannelArgs args) { private static final LoadBalancer.PickDetailsConsumer NOOP_PICK_DETAILS_CONSUMER = new LoadBalancer.PickDetailsConsumer() {}; - private ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; + /** + * Retrieves the user-provided configuration function for internal child channels. + * + *

This is intended for use by gRPC internal components + * that are responsible for creating auxiliary {@code ManagedChannel} instances. + */ + private ChannelConfigurer channelConfigurer = new ChannelConfigurer() {}; private final InternalLogId logId; private final String target; @@ -548,8 +554,8 @@ ClientStream newSubstream( Supplier stopwatchSupplier, List interceptors, final TimeProvider timeProvider) { - if (builder.childChannelConfigurer != null) { - this.childChannelConfigurer = builder.childChannelConfigurer; + if (builder.channelConfigurer != null) { + this.channelConfigurer = builder.channelConfigurer; } this.target = checkNotNull(builder.target, "target"); this.logId = InternalLogId.allocate("Channel", target); @@ -596,7 +602,7 @@ ClientStream newSubstream( .setOverrideAuthority(this.authorityOverride) .setMetricRecorder(this.metricRecorder) .setNameResolverRegistry(builder.nameResolverRegistry) - .setChildChannelConfigurer(this.childChannelConfigurer); + .setChildChannelConfigurer(this.channelConfigurer); builder.copyAllNameResolverCustomArgsTo(nameResolverArgsBuilder); this.nameResolverArgs = nameResolverArgsBuilder.build(); this.nameResolver = getNameResolver( @@ -672,15 +678,6 @@ public CallTracer create() { } } - /** - * Retrieves the user-provided configuration function for internal child channels. - * - *

This method is intended for use by gRPC internal components - * that are responsible for creating auxiliary {@code ManagedChannel} instances. - * - * @return the ChildChannelConfigurer, guaranteed to be not null (defaults to no-op). - */ - @VisibleForTesting static NameResolver getNameResolver( UriWrapper targetUri, @Nullable final String overrideAuthority, @@ -1504,8 +1501,8 @@ protected ManagedChannelBuilder delegate() { // Note that we follow the global configurator pattern and try to fuse the configurations as // soon as the builder gets created - if (childChannelConfigurer != null) { - childChannelConfigurer.configureChannelBuilder(builder); + if (channelConfigurer != null) { + channelConfigurer.configureChannelBuilder(builder); } return builder diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 1d4c5df616e..2d09d4b495e 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -29,8 +29,8 @@ import io.grpc.CallCredentials; import io.grpc.CallOptions; import io.grpc.Channel; +import io.grpc.ChannelConfigurer; import io.grpc.ChannelCredentials; -import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientTransportFilter; @@ -128,12 +128,13 @@ public static ManagedChannelBuilder forTarget(String target) { private static final Method GET_CLIENT_INTERCEPTOR_METHOD; - ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; + ChannelConfigurer channelConfigurer = new ChannelConfigurer() {}; @Override public ManagedChannelImplBuilder childChannelConfigurer( - ChildChannelConfigurer childChannelConfigurer) { - this.childChannelConfigurer = checkNotNull(childChannelConfigurer, "childChannelConfigurer"); + ChannelConfigurer channelConfigurer) { + this.channelConfigurer = checkNotNull(channelConfigurer, + "childChannelConfigurer"); return this; } @@ -727,8 +728,6 @@ public ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { return this; } - - @Override public ManagedChannel build() { ClientTransportFactory clientTransportFactory = diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java index 90f2623b6a4..5f935deddbe 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java @@ -35,7 +35,7 @@ import com.google.common.util.concurrent.MoreExecutors; import io.grpc.CallOptions; import io.grpc.Channel; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.CompressorRegistry; @@ -44,6 +44,7 @@ import io.grpc.InternalConfigurator; import io.grpc.InternalConfiguratorRegistry; import io.grpc.InternalFeatureFlags; +import io.grpc.InternalManagedChannelBuilder; import io.grpc.InternalManagedChannelBuilder.InternalInterceptorFactory; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; @@ -73,7 +74,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; @@ -786,9 +786,9 @@ public void setNameResolverExtArgs() { @Test public void childChannelConfigurer_setsField() { - ChildChannelConfigurer configurer = mock(ChildChannelConfigurer.class); + ChannelConfigurer configurer = mock(ChannelConfigurer.class); assertSame(builder, builder.childChannelConfigurer(configurer)); - assertSame(configurer, builder.childChannelConfigurer); + assertSame(configurer, builder.channelConfigurer); } @Test @@ -805,13 +805,12 @@ public void childChannelConfigurer_propagatesMetricsAndInterceptors_xdsTarget() ClientInterceptor mockInterceptor = mock(ClientInterceptor.class); // Define the Configurer - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { builder.addMetricSink(mockMetricSink); - // Assuming InternalInterceptorFactory is also accessible - builder.interceptWithTarget(target -> mockInterceptor); + InternalManagedChannelBuilder.interceptWithTarget(builder, target -> mockInterceptor); } }; @@ -821,7 +820,7 @@ public void configureChannelBuilder(ManagedChannelBuilder builder) { NameResolver mockNameResolver = mock(NameResolver.class); when(mockNameResolver.getServiceAuthority()).thenReturn("foo.authority"); ArgumentCaptor argsCaptor = ArgumentCaptor.forClass(NameResolver.Args.class); - when(mockNameResolverFactory.newNameResolver(any(), + when(mockNameResolverFactory.newNameResolver((URI) any(), argsCaptor.capture())).thenReturn(mockNameResolver); // Use the configurer and the mock factory @@ -839,16 +838,16 @@ public void configureChannelBuilder(ManagedChannelBuilder builder) { grpcCleanupRule.register(channel); // Verify that newNameResolver was called - verify(mockNameResolverFactory).newNameResolver(any(), any()); + verify(mockNameResolverFactory).newNameResolver((URI) any(), any()); // Extract the childChannelConfigurer from Args NameResolver.Args args = argsCaptor.getValue(); - ChildChannelConfigurer childChannelConfigurerInArgs = args.getChildChannelConfigurer(); + ChannelConfigurer channelConfigurerInArgs = args.getChildChannelConfigurer(); assertNotNull("Child channel configurer should be present in NameResolver.Args", - childChannelConfigurerInArgs); + channelConfigurerInArgs); // Verify the configurer is the one we passed - assertThat(childChannelConfigurerInArgs).isSameInstanceAs(configurer); + assertThat(channelConfigurerInArgs).isSameInstanceAs(configurer); // Verify the configurer logically applies (by running it on a mock) ManagedChannelBuilder mockChildBuilder = mock(ManagedChannelBuilder.class); @@ -857,7 +856,6 @@ public void configureChannelBuilder(ManagedChannelBuilder builder) { configurer.configureChannelBuilder(mockChildBuilder); verify(mockChildBuilder).addMetricSink(mockMetricSink); - verify(mockChildBuilder).interceptWithTarget(any()); } @Test diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 2f6357c074f..d55288be357 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -299,6 +299,7 @@ public String getPolicyName() { private boolean panicExpected; @Captor private ArgumentCaptor resolvedAddressCaptor; + private ArgumentCaptor streamListenerCaptor = ArgumentCaptor.forClass(ClientStreamListener.class); @@ -4748,6 +4749,4 @@ private static ManagedChannelServiceConfig createManagedChannelServiceConfig( return ManagedChannelServiceConfig .fromServiceConfig(rawServiceConfig, true, 3, 4, policySelection); } - - } diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index c5f0ff6bdc3..7da696ac49e 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -21,8 +21,8 @@ import com.google.common.annotations.VisibleForTesting; import io.grpc.CallCredentials; import io.grpc.CallOptions; +import io.grpc.ChannelConfigurer; import io.grpc.ChannelCredentials; -import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.Context; import io.grpc.Grpc; @@ -38,18 +38,18 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { private final CallCredentials callCredentials; - private final ChildChannelConfigurer childChannelConfigurer; + private final ChannelConfigurer channelConfigurer; GrpcXdsTransportFactory(CallCredentials callCredentials, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { this.callCredentials = callCredentials; - this.childChannelConfigurer = childChannelConfigurer; + this.channelConfigurer = channelConfigurer; } @Override public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { - return new GrpcXdsTransport(serverInfo, callCredentials, childChannelConfigurer); + return new GrpcXdsTransport(serverInfo, callCredentials, channelConfigurer); } @VisibleForTesting @@ -81,14 +81,15 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials call this.callCredentials = callCredentials; } - public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, CallCredentials callCredentials, - ChildChannelConfigurer childChannelConfigurer) { + public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, + CallCredentials callCredentials, + ChannelConfigurer channelConfigurer) { String target = serverInfo.target(); ChannelCredentials channelCredentials = (ChannelCredentials) serverInfo.implSpecificConfig(); ManagedChannelBuilder channelBuilder = Grpc.newChannelBuilder(target, channelCredentials) .keepAliveTime(5, TimeUnit.MINUTES); - if (childChannelConfigurer != null) { - childChannelConfigurer.configureChannelBuilder(channelBuilder); + if (channelConfigurer != null) { + channelConfigurer.configureChannelBuilder(channelBuilder); } this.channel = channelBuilder.build(); this.callCredentials = callCredentials; diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index 29af3849b20..bccf63c475f 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -22,7 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.concurrent.GuardedBy; import io.grpc.CallCredentials; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.MetricRecorder; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; @@ -107,9 +107,9 @@ public ObjectPool getOrCreate( @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { return getOrCreate(target, bootstrapInfo, metricRecorder, null, - childChannelConfigurer); + channelConfigurer); } public ObjectPool getOrCreate( @@ -117,7 +117,7 @@ public ObjectPool getOrCreate( BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, CallCredentials transportCallCredentials, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { ObjectPool ref = targetToXdsClientMap.get(target); if (ref == null) { synchronized (lock) { @@ -126,7 +126,7 @@ public ObjectPool getOrCreate( ref = new RefCountedXdsClientObjectPool( bootstrapInfo, target, metricRecorder, transportCallCredentials, - childChannelConfigurer); + channelConfigurer); targetToXdsClientMap.put(target, ref); } } @@ -151,7 +151,7 @@ class RefCountedXdsClientObjectPool implements ObjectPool { private final String target; // The target associated with the xDS client. private final MetricRecorder metricRecorder; private final CallCredentials transportCallCredentials; - private final ChildChannelConfigurer childChannelConfigurer; + private final ChannelConfigurer channelConfigurer; private final Object lock = new Object(); @GuardedBy("lock") private ScheduledExecutorService scheduler; @@ -174,12 +174,12 @@ class RefCountedXdsClientObjectPool implements ObjectPool { String target, MetricRecorder metricRecorder, CallCredentials transportCallCredentials, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { this.bootstrapInfo = checkNotNull(bootstrapInfo, "bootstrapInfo"); this.target = target; this.metricRecorder = checkNotNull(metricRecorder, "metricRecorder"); this.transportCallCredentials = transportCallCredentials; - this.childChannelConfigurer = childChannelConfigurer; + this.channelConfigurer = channelConfigurer; } @Override @@ -192,7 +192,7 @@ public XdsClient getObject() { scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE); metricReporter = new XdsClientMetricReporterImpl(metricRecorder, target); GrpcXdsTransportFactory xdsTransportFactory = - new GrpcXdsTransportFactory(transportCallCredentials, childChannelConfigurer); + new GrpcXdsTransportFactory(transportCallCredentials, channelConfigurer); xdsClient = new XdsClientImpl( xdsTransportFactory, diff --git a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java index 4984fdf9f74..5e16d1225fa 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java @@ -16,7 +16,7 @@ package io.grpc.xds; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.MetricRecorder; import io.grpc.internal.ObjectPool; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; @@ -33,7 +33,7 @@ ObjectPool getOrCreate( ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ChildChannelConfigurer childChannelConfigurer); + ChannelConfigurer channelConfigurer); List getTargets(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 47b2dee2e50..5c27263f6f2 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -31,7 +31,7 @@ import io.grpc.Attributes; import io.grpc.CallOptions; import io.grpc.Channel; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -1056,19 +1056,19 @@ private static final class BootstrappingXdsClientPool implements XdsClientPool { private final @Nullable Map bootstrapOverride; private final @Nullable MetricRecorder metricRecorder; private ObjectPool xdsClientPool; - private ChildChannelConfigurer childChannelConfigurer; + private ChannelConfigurer channelConfigurer; BootstrappingXdsClientPool( XdsClientPoolFactory xdsClientPoolFactory, String target, @Nullable Map bootstrapOverride, @Nullable MetricRecorder metricRecorder, - @Nullable ChildChannelConfigurer childChannelConfigurer) { + @Nullable ChannelConfigurer channelConfigurer) { this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); this.target = checkNotNull(target, "target"); this.bootstrapOverride = bootstrapOverride; this.metricRecorder = metricRecorder; - this.childChannelConfigurer = childChannelConfigurer; + this.channelConfigurer = channelConfigurer; } @Override @@ -1082,7 +1082,7 @@ public XdsClient getObject() throws XdsInitializationException { } this.xdsClientPool = xdsClientPoolFactory.getOrCreate( - target, bootstrapInfo, metricRecorder, childChannelConfigurer); + target, bootstrapInfo, metricRecorder, channelConfigurer); } return xdsClientPool.getObject(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index 5628367f502..febc9567829 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -25,7 +25,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.errorprone.annotations.DoNotCall; import io.grpc.Attributes; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.ExperimentalApi; import io.grpc.ForwardingServerBuilder; import io.grpc.Internal; @@ -59,7 +59,7 @@ public final class XdsServerBuilder extends ForwardingServerBuilder bootstrapOverride; private long drainGraceTime = 10; private TimeUnit drainGraceTimeUnit = TimeUnit.MINUTES; - private ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; + private ChannelConfigurer channelConfigurer = new ChannelConfigurer() {}; private XdsServerBuilder(NettyServerBuilder nettyDelegate, int port) { this.delegate = nettyDelegate; @@ -108,11 +108,11 @@ public XdsServerBuilder drainGraceTime(long drainGraceTime, TimeUnit drainGraceT *

This configurer will subsequently be used to configure any child channels * created by that server. * - * @param childChannelConfigurer the configurer to store in the channel. + * @param channelConfigurer the configurer to store in the channel. */ @Override - public XdsServerBuilder childChannelConfigurer(ChildChannelConfigurer childChannelConfigurer) { - this.childChannelConfigurer = childChannelConfigurer; + public XdsServerBuilder childChannelConfigurer(ChannelConfigurer channelConfigurer) { + this.channelConfigurer = channelConfigurer; return this; } @@ -145,7 +145,7 @@ public Server build() { InternalNettyServerBuilder.eagAttributes(delegate, builder.build()); return new XdsServerWrapper("0.0.0.0:" + port, delegate, xdsServingStatusListener, filterChainSelectorManager, xdsClientPoolFactory, bootstrapOverride, filterRegistry, - this.childChannelConfigurer); + this.channelConfigurer); } @VisibleForTesting diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index 0c69fda7b3e..7e633d75242 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -29,7 +29,7 @@ import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; import io.grpc.Attributes; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.InternalServerInterceptors; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -129,7 +129,7 @@ public void uncaughtException(Thread t, Throwable e) { // NamedFilterConfig.filterStateKey -> filter_instance. private final HashMap activeFiltersDefaultChain = new HashMap<>(); - private ChildChannelConfigurer childChannelConfigurer = new ChildChannelConfigurer() {}; + private ChannelConfigurer channelConfigurer = new ChannelConfigurer() {}; XdsServerWrapper( String listenerAddress, @@ -159,7 +159,7 @@ public void uncaughtException(Thread t, Throwable e) { XdsClientPoolFactory xdsClientPoolFactory, @Nullable Map bootstrapOverride, FilterRegistry filterRegistry, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { this( listenerAddress, delegateBuilder, @@ -170,8 +170,8 @@ public void uncaughtException(Thread t, Throwable e) { filterRegistry, SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE)); sharedTimeService = true; - if (childChannelConfigurer != null) { - this.childChannelConfigurer = childChannelConfigurer; + if (channelConfigurer != null) { + this.channelConfigurer = channelConfigurer; } } @@ -230,7 +230,7 @@ private void internalStart() { } xdsClientPool = xdsClientPoolFactory.getOrCreate( "#server", bootstrapInfo, new MetricRecorder() {}, - childChannelConfigurer); + channelConfigurer); } catch (Exception e) { StatusException statusException = Status.UNAVAILABLE.withDescription( "Failed to initialize xDS").withCause(e).asException(); @@ -256,7 +256,6 @@ private void internalStart() { discoveryState = new DiscoveryState(listenerTemplate.replaceAll("%s", replacement)); } - @Override public Server shutdown() { if (!shutdown.compareAndSet(false, true)) { diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 3dbbc802db2..70ddd8ba795 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -37,7 +37,7 @@ import io.envoyproxy.envoy.service.status.v3.ClientStatusRequest; import io.envoyproxy.envoy.service.status.v3.ClientStatusResponse; import io.envoyproxy.envoy.type.matcher.v3.NodeMatcher; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.Deadline; import io.grpc.InsecureChannelCredentials; import io.grpc.MetricRecorder; @@ -522,7 +522,7 @@ public ObjectPool getOrCreate( @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { throw new UnsupportedOperationException("Should not be called"); } } diff --git a/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java b/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java index fd856a51995..1de47cf2b73 100644 --- a/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java +++ b/xds/src/test/java/io/grpc/xds/FakeControlPlaneXdsIntegrationTest.java @@ -54,7 +54,7 @@ import io.envoyproxy.envoy.extensions.load_balancing_policies.wrr_locality.v3.WrrLocality; import io.grpc.CallOptions; import io.grpc.Channel; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientStreamTracer; @@ -358,7 +358,7 @@ public void pingPong_logicalDns_authorityOverride() { @Test public void childChannelConfigurer_passesMetricSinkToChannel_E2E() { MetricSink mockSink = mock(MetricSink.class, delegatesTo(new NoopMetricSink())); - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { builder.addMetricSink(mockSink); @@ -387,7 +387,7 @@ public void configureChannelBuilder(ManagedChannelBuilder builder) { @Test public void childChannelConfigurer_passesMetricSinkToServer_E2E() throws Exception { MetricSink mockSink = mock(MetricSink.class, delegatesTo(new NoopMetricSink())); - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { // Child channels (xDS client connections) created by this server get the sink. diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index f851935c200..a97490ed9bb 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -29,7 +29,7 @@ import io.grpc.BindableService; import io.grpc.CallOptions; import io.grpc.Channel; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.ClientInterceptor; import io.grpc.Grpc; import io.grpc.InsecureChannelCredentials; @@ -159,7 +159,7 @@ public void verifyConfigApplied_interceptor() { .thenReturn(new io.grpc.NoopClientCall<>()); // Create Configurer that adds the interceptor - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { builder.intercept(mockInterceptor); @@ -195,9 +195,9 @@ public void configureChannelBuilder(ManagedChannelBuilder builder) { } @Test - public void useChildChannelConfigurer() { + public void useChannelConfigurer() { // Mock Configurer - ChildChannelConfigurer mockConfigurer = mock(ChildChannelConfigurer.class); + ChannelConfigurer mockConfigurer = mock(ChannelConfigurer.class); // Create Factory GrpcXdsTransportFactory factory = new GrpcXdsTransportFactory( @@ -220,7 +220,7 @@ public void verifyConfigApplied_maxInboundMessageSize() { ManagedChannelBuilder mockBuilder = mock(ManagedChannelBuilder.class); // Create Configurer that modifies message size - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { builder.maxInboundMessageSize(1024); @@ -239,7 +239,7 @@ public void verifyConfigApplied_interceptors() { ClientInterceptor interceptor1 = mock(ClientInterceptor.class); ClientInterceptor interceptor2 = mock(ClientInterceptor.class); - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { builder.intercept(interceptor1); diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index bcf97066361..89a094046fe 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -28,7 +28,7 @@ import com.google.auth.oauth2.OAuth2Credentials; import com.google.common.util.concurrent.SettableFuture; import io.grpc.CallCredentials; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.ClientInterceptor; import io.grpc.Grpc; import io.grpc.InsecureChannelCredentials; @@ -226,7 +226,7 @@ public void xdsClient_usesCallCredentials() throws Exception { } @Test - public void xdsClient_usesChildChannelConfigurer() throws Exception { + public void xdsClient_usesChannelConfigurer() throws Exception { // Set up fake xDS server XdsTestControlPlaneService fakeXdsService = new XdsTestControlPlaneService(); CallCredsServerInterceptor callInterceptor = new CallCredsServerInterceptor(); @@ -262,14 +262,14 @@ public void start(Listener responseListener, Metadata headers) { } }; - ChildChannelConfigurer configurer = new ChildChannelConfigurer() { + ChannelConfigurer configurer = new ChannelConfigurer() { @Override public void configureChannelBuilder(ManagedChannelBuilder builder) { builder.intercept(testInterceptor); } }; - // Create xDS client that uses the ChildChannelConfigurer on the transport + // Create xDS client that uses the ChannelConfigurer on the transport ObjectPool xdsClientPool = provider.getOrCreate("target", bootstrapInfo, metricRecorder, null, configurer); XdsClient xdsClient = xdsClientPool.getObject(); diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 11c961c68ac..af65c4d9687 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -48,8 +48,8 @@ import com.google.re2j.Pattern; import io.grpc.CallOptions; import io.grpc.Channel; +import io.grpc.ChannelConfigurer; import io.grpc.ChannelLogger; -import io.grpc.ChildChannelConfigurer; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.ClientInterceptors; @@ -2499,7 +2499,7 @@ public XdsClient returnObject(Object object) { @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { targets.add(target); return new ObjectPool() { @Override @@ -2955,7 +2955,7 @@ void deliverErrorStatus() { @Test public void start_passesParentChannelToClientPoolFactory() { - ChildChannelConfigurer mockChildChannelConfigurer = mock(ChildChannelConfigurer.class); + ChannelConfigurer mockChannelConfigurer = mock(ChannelConfigurer.class); // Build NameResolver.Args containing the child channel configurer NameResolver.Args args = NameResolver.Args.newBuilder() @@ -2964,7 +2964,7 @@ public void start_passesParentChannelToClientPoolFactory() { .setSynchronizationContext(syncContext) .setServiceConfigParser(serviceConfigParser) .setChannelLogger(mock(ChannelLogger.class)) - .setChildChannelConfigurer(mockChildChannelConfigurer) + .setChildChannelConfigurer(mockChannelConfigurer) .build(); // Mock the XdsClientPoolFactory @@ -2979,7 +2979,7 @@ public void start_passesParentChannelToClientPoolFactory() { anyString(), any(BootstrapInfo.class), any(MetricRecorder.class), - any(ChildChannelConfigurer.class))) + any(ChannelConfigurer.class))) .thenReturn(mockObjectPool); XdsNameResolver resolver = new XdsNameResolver( @@ -3004,7 +3004,7 @@ public void start_passesParentChannelToClientPoolFactory() { eq(AUTHORITY), any(BootstrapInfo.class), eq(metricRecorder), - eq(mockChildChannelConfigurer)); + eq(mockChannelConfigurer)); resolver.shutdown(); } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index 5fe1c7feaeb..770bdae1eac 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -30,7 +30,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.SettableFuture; import io.grpc.BindableService; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.InsecureServerCredentials; import io.grpc.ServerServiceDefinition; import io.grpc.Status; @@ -331,7 +331,7 @@ public void testOverrideBootstrap() throws Exception { @Test public void start_passesParentServerToClientPoolFactory() throws Exception { - ChildChannelConfigurer mockConfigurer = mock(ChildChannelConfigurer.class); + ChannelConfigurer mockConfigurer = mock(ChannelConfigurer.class); XdsClientPoolFactory mockPoolFactory = mock(XdsClientPoolFactory.class); @SuppressWarnings("unchecked") ObjectPool mockPool = mock(ObjectPool.class); diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index f611e013b3a..56d5021691a 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -22,7 +22,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol; -import io.grpc.ChildChannelConfigurer; +import io.grpc.ChannelConfigurer; import io.grpc.InsecureChannelCredentials; import io.grpc.MetricRecorder; import io.grpc.Status; @@ -186,7 +186,7 @@ public XdsClient returnObject(Object object) { @Override public ObjectPool getOrCreate( String target, BootstrapInfo bootstrapInfo, MetricRecorder metricRecorder, - ChildChannelConfigurer childChannelConfigurer) { + ChannelConfigurer channelConfigurer) { this.savedBootstrapInfo = bootstrapInfo; return new ObjectPool() { @Override From 3e385366d7f7e40343027610ce42f1d3c04afae6 Mon Sep 17 00:00:00 2001 From: agravator Date: Wed, 18 Mar 2026 15:21:23 +0530 Subject: [PATCH 10/10] fix: merge conflict --- .../java/io/grpc/xds/GrpcXdsTransportFactory.java | 12 +++--------- .../io/grpc/xds/GrpcXdsTransportFactoryTest.java | 4 ++-- .../test/java/io/grpc/xds/XdsNameResolverTest.java | 6 +++--- .../test/java/io/grpc/xds/XdsServerBuilderTest.java | 4 ++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index 89a367d3e37..0f467f3ae0d 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -54,9 +54,8 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { private final CallCredentials callCredentials; -<<<<<<< child-channel-plugin private final ChannelConfigurer channelConfigurer; -======= + // The map of xDS server info to its corresponding gRPC xDS transport. // This enables reusing and sharing the same underlying gRPC channel. // @@ -65,8 +64,6 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { // for reference counting of each GrpcXdsTransport instance. private static final Map xdsServerInfoToTransportMap = new ConcurrentHashMap<>(); ->>>>>>> master - GrpcXdsTransportFactory(CallCredentials callCredentials, ChannelConfigurer channelConfigurer) { @@ -76,19 +73,15 @@ final class GrpcXdsTransportFactory implements XdsTransportFactory { @Override public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { -<<<<<<< child-channel-plugin - return new GrpcXdsTransport(serverInfo, callCredentials, channelConfigurer); -======= return xdsServerInfoToTransportMap.compute( serverInfo, (info, transport) -> { if (transport == null) { - transport = new GrpcXdsTransport(serverInfo, callCredentials); + transport = new GrpcXdsTransport(serverInfo, callCredentials, channelConfigurer); } ++transport.refCount; return transport; }); ->>>>>>> master } @VisibleForTesting @@ -136,6 +129,7 @@ public GrpcXdsTransport(Bootstrapper.ServerInfo serverInfo, } this.channel = channelBuilder.build(); this.callCredentials = callCredentials; + this.serverInfo = serverInfo; } @VisibleForTesting diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java index 69808eb3d1a..786112c1cd4 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsTransportFactoryTest.java @@ -136,7 +136,7 @@ public void refCountedXdsTransport_sameXdsServerAddress_returnsExistingTransport Bootstrapper.ServerInfo xdsServerInfo = Bootstrapper.ServerInfo.create( "localhost:" + server.getPort(), InsecureChannelCredentials.create()); - GrpcXdsTransportFactory xdsTransportFactory = new GrpcXdsTransportFactory(null); + GrpcXdsTransportFactory xdsTransportFactory = new GrpcXdsTransportFactory(null, null); // Calling create() for the first time creates a new GrpcXdsTransport instance. // The ref count was previously 0 and now is 1. XdsTransportFactory.XdsTransport transport1 = xdsTransportFactory.create(xdsServerInfo); @@ -168,7 +168,7 @@ public void refCountedXdsTransport_differentXdsServerAddress_returnsDifferentTra Bootstrapper.ServerInfo xdsServerInfo2 = Bootstrapper.ServerInfo.create( "localhost:" + server2.getPort(), InsecureChannelCredentials.create()); - GrpcXdsTransportFactory xdsTransportFactory = new GrpcXdsTransportFactory(null); + GrpcXdsTransportFactory xdsTransportFactory = new GrpcXdsTransportFactory(null, null); // Calling create() to the first xDS server creates a new GrpcXdsTransport instance. // The ref count was previously 0 and now is 1. XdsTransportFactory.XdsTransport transport1 = xdsTransportFactory.create(xdsServerInfo1); diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 25c95e27a00..0cee1b77a56 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -2946,10 +2946,10 @@ void deliverErrorStatus() { } @Test - public void start_passesParentChannelToClientPoolFactory() { + public void start_passesChannelConfigurerToClientPoolFactory() { ChannelConfigurer mockChannelConfigurer = mock(ChannelConfigurer.class); - // Build NameResolver.Args containing the child channel configurer + // Build NameResolver.Args containing the channel configurer NameResolver.Args args = NameResolver.Args.newBuilder() .setDefaultPort(8080) .setProxyDetector(mock(ProxyDetector.class)) @@ -2975,7 +2975,7 @@ public void start_passesParentChannelToClientPoolFactory() { .thenReturn(mockObjectPool); XdsNameResolver resolver = new XdsNameResolver( - URI.create(AUTHORITY), + targetUri, null, // targetAuthority (nullable) AUTHORITY, // name null, // overrideAuthority (nullable) diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index 770bdae1eac..a01d88262fc 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -330,7 +330,7 @@ public void testOverrideBootstrap() throws Exception { } @Test - public void start_passesParentServerToClientPoolFactory() throws Exception { + public void start_passesChannelConfigurerToClientPoolFactory() throws Exception { ChannelConfigurer mockConfigurer = mock(ChannelConfigurer.class); XdsClientPoolFactory mockPoolFactory = mock(XdsClientPoolFactory.class); @SuppressWarnings("unchecked") @@ -345,7 +345,7 @@ public void start_passesParentServerToClientPoolFactory() throws Exception { Future unused = startServerAsync(); - // Verify getOrCreate called with the server instance + // Verify getOrCreate called with the ChannelConfigurer instance verify(mockPoolFactory).getOrCreate( any(), any(), any(), org.mockito.ArgumentMatchers.eq(mockConfigurer)); }