Adds Grpc.newManagedChannel(String, ChannelCredentials, NameResolverR… by AgraVator · Pull Request #11901 · grpc/grpc-java

AgraVator · 2025-02-17T08:57:53Z

Fixes #11055
Exposes a new method for channel creation which accepts NameResolverRegistry

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

netty/src/test/java/io/grpc/netty/UdsNettyChannelProviderTest.java

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

ejona86 · 2025-02-19T16:00:55Z

netty/src/test/java/io/grpc/netty/UdsNettyChannelProviderTest.java

+    assertThat(managedChannelBuilder).isNotNull();
+    ManagedChannel channel = managedChannelBuilder.build();
+    assertThat(channel).isNotNull();
+    assertThat(channel.authority()).isEqualTo("/sock.sock");


The test isn't actually working. FakeNameResolverProvider hard-codes the authority to "fake-authority". So this should be failing, because it isn't actually using your NameResolverRegistry.

And that's because the name resolver registry isn't being passed to the ManagedChannelBuilder.

grpc-java/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

Line 154 in 60f6ea7

NameResolverRegistry nameResolverRegistry = NameResolverRegistry.getDefaultRegistry();

And looking deeper, that's because the NameResolverRegistry code in ManagedChannelRegistry is a bit of a hack; it doesn't coordinate with the ManagedChannelBuilder right now and instead just assumes some later logic in ManagedChannelImpl will produce similar results. It should ideally select the NameResolverProvider and pass it to the ManagedChannelProvider. But I'm not sure how much of that we want to clean up now.

I think we will need to change the arguments of ManagedChannelProvider.newChannelBuilder(String, ChannelCredentials). That's a bit annoying though, because we can't make that change atomically everywhere (some implementations aren't in this repo) and this is needing to modify an existing code path. So we need to add a new method to ManagedChannelProvider and have it call the existing method by default. We can update all the ManagedChannelProviders in this repo at the same time (and have the old method call the new method). After this change is imported to Google, we'll update the remaining usages and then we can delete the old method.

But I need to think more about what the new signature should look like. There's some interplay between ManagedChannelRegistry and ManagedChannelImplBuilder.getNameResolverProvider(), and it'd be nice to avoid one side assuming the behavior of the other. I also need to figure out whether the NameResolverRegistry will propagate through the channel to LoadBalancer.Helper.createResolvingOobChannel() and getNameResolverRegistry().

I spoke with Doug, and I think the new signature should have NameResolverProvider (for this channel's use) and NameResolverRegistry (for child channel's use) added as arguments.

To help the migration, you can have the new method call the old method (and just throw away the new arguments).

Do you mean something like #11978 for migration ?

The extra method needs to be on ManagedChannelProvider and would be non-static. But the general idea, yeah. We won't let our users pass the NameResolverProvider to io.grpc.Grpc; they'll only pass the registry. Then io.grpc.Grpc will compute the name resolver provider and pass both provider+registry to the selected managed channel provider.

The update looks good. Why is that a separate PR? What progression of changes are you imagining for this?

I initially thought that after merging the other PR we could do the migration, because initially i had made changes to expose a new API but after the update that becomes pointless because even if we had merged it, it wouldn't be useful directly.

It must be used in this PR. This method would call the new provider.newChannelBuilder(). Right now this method discards the name resolver registry and name resolver provider being used. That's why this test should be failing, but isn't.

ejona86 · 2026-01-16T16:21:21Z

netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java

+   * Sets the registry used for looking up name resolvers.
+   */
+  @CanIgnoreReturnValue
+  public NettyChannelBuilder nameResolverRegistry(NameResolverRegistry registry) {


The purpose of us adding this new API is so that people can't change the name resolver after creating the builder. So it defeats the purpose to add new public methods.

We should use a separate constructor, because we'll want to disallow changing the nameResolverFactory when using the API to pass the registry.

ejona86 · 2026-01-16T16:26:58Z

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

+      }
    }

-    if (provider == null && !URI_PATTERN.matcher(target).matches()) {


Why go through this code block if you will ignore the result?

I still don't understand why this block was modified to check if provider is set. It looks like it can only break things.

…egistry)

2. removes visibleForTesting 3. improves the test case

…ResolverRegistry and NameResolverProvider.

ejona86

This looks much closer to ready. I think the various components are in-place, we just need to tweak things.

ejona86 · 2026-02-26T18:59:13Z

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

    return newChannelBuilder(NameResolverRegistry.getDefaultRegistry(), target, creds);
  }

-  @VisibleForTesting


Why was this removed?

ejona86 · 2026-02-26T22:25:45Z

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

+   *     or credentials
+   * @since 1.79.0
+   */
+  public static ManagedChannelBuilder<?> newChannelBuilder(


Add @ExperimentalApi

ejona86 · 2026-02-26T22:27:22Z

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

+   * Creates a channel builder with a target string, credentials, and a specific
+   * name resolver registry.
+   *
+   * <p>This method uses the {@link ManagedChannelRegistry#getDefaultRegistry()} to


Let's not mention the ManagedChannelRegistry, as users shouldn't care. We didn't mention it in the newChannelBuilder(String, ChannelCredentials) version and I don't think it hurt users at all.

ejona86 · 2026-02-26T22:29:57Z

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

+   *
+   * <p>This method allows for fine-grained control over name resolution by providing
+   * both a {@link NameResolverRegistry} and a specific {@link NameResolverProvider}.
+   * Unlike the public factory methods, this returns a {@link NewChannelBuilderResult},


It is pretty unclear what "public factory methods" means. (I figured it out, but it wasn't clear.)

ejona86 · 2026-02-26T22:33:52Z

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

  public ManagedChannelImplBuilder(
      String target, @Nullable ChannelCredentials channelCreds, @Nullable CallCredentials callCreds,
      ClientTransportFactoryBuilder clientTransportFactoryBuilder,
      @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) {
    this.target = checkNotNull(target, "target");
    this.channelCredentials = channelCreds;
    this.callCredentials = callCreds;
    this.clientTransportFactoryBuilder = checkNotNull(clientTransportFactoryBuilder,
        "clientTransportFactoryBuilder");
    this.directServerAddress = null;

    if (channelBuilderDefaultPortProvider != null) {
      this.channelBuilderDefaultPortProvider = channelBuilderDefaultPortProvider;
    } else {
      this.channelBuilderDefaultPortProvider = new ManagedChannelDefaultPortProvider();
    }
    // TODO(dnvindhya): Move configurator to all the individual builders
    InternalConfiguratorRegistry.configureChannelBuilder(this);
  }


This should just forward to the new constructor, right?

ejona86 · 2026-02-26T22:34:35Z

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

+    if (nameResolverRegistry != null) {
+      this.nameResolverRegistry = nameResolverRegistry;
+    }
+    if (nameResolverProvider != null) {


Why a null check?

ejona86 · 2026-02-26T22:40:29Z

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

+      }
    }

-    if (provider == null && !URI_PATTERN.matcher(target).matches()) {


I still don't understand why this block was modified to check if provider is set. It looks like it can only break things.

ejona86 reviewed Feb 18, 2025

View reviewed changes

api/src/main/java/io/grpc/Grpc.java Outdated Show resolved Hide resolved

netty/src/test/java/io/grpc/netty/UdsNettyChannelProviderTest.java Outdated Show resolved Hide resolved

api/src/main/java/io/grpc/Grpc.java Outdated Show resolved Hide resolved

ejona86 reviewed Feb 19, 2025

View reviewed changes

ejona86 mentioned this pull request Mar 26, 2025

feat(api): adds new channel builder with name resolver registry and provider #11978

Closed

AgraVator force-pushed the adds-new-channel-builder-with-name-resolver-registry branch from 20a6b37 to 6913156 Compare December 24, 2025 10:24

AgraVator requested a review from ejona86 December 24, 2025 10:25

AgraVator force-pushed the adds-new-channel-builder-with-name-resolver-registry branch from 4df7c4f to 4ee3512 Compare January 16, 2026 08:20

ejona86 reviewed Jan 16, 2026

View reviewed changes

AgraVator added 7 commits February 25, 2026 23:35

Adds Grpc.newManagedChannel(String, ChannelCredentials, NameResolverR…

043b468

…egistry)

1. refactors method name to make it overloaded

5e1b7b3

2. removes visibleForTesting 3. improves the test case

fix: exposes a new ManagedChannelProvider.newChannelBuilder with Name…

49f8dc4

…ResolverRegistry and NameResolverProvider.

style: fixes checkstyle

a88e430

API for nameResolverRegistry

4288b51

use the new API in channelProviders

0e436bb

use constructor instead of public method

e65d7ca

AgraVator force-pushed the adds-new-channel-builder-with-name-resolver-registry branch from 2b54502 to a627be6 Compare February 25, 2026 18:07

AgraVator requested a review from ejona86 February 25, 2026 18:08

AgraVator force-pushed the adds-new-channel-builder-with-name-resolver-registry branch 2 times, most recently from 7d98147 to e65d7ca Compare February 25, 2026 18:57

AgraVator added 3 commits February 26, 2026 00:56

refactor getNameResolverProvider()

dd007ec

fix test cases

f3f49b6

fix test cases

3859ae5

ejona86 reviewed Feb 26, 2026

View reviewed changes

Conversation

AgraVator commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ejona86 Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ejona86 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AgraVator commented Feb 17, 2025 •

edited

Loading

ejona86 Feb 19, 2025 •

edited

Loading