NameResolverRegistry with multiple registrations no longer working as of gRPC 1.60+

[clairemcginty]

As of #10590, the behavior of ManagedChannelImplBuilder#nameResolverFactory has changed -- it now wraps the provided resolverFactory into an NameResolverFactoryToProviderFacade that returns the first registered NameResolverProvider's scheme as its defaultScheme(). As a result, lookups of any other registered schemes fail.

Here's a code sample of something that used to work pre-1.60.0 (apologies for the Scala code):

case class Resolver(scheme: String) extends NameResolverProvider {
  override def isAvailable: Boolean = true
  override def priority(): Int = 5
  override def newNameResolver(targetUri: URI, args: NameResolver.Args): NameResolver = new NameResolver {
    override def getServiceAuthority: String = "foo"
    override def shutdown(): Unit = {}
  }
  override def getDefaultScheme: String = scheme
}

val registry = new NameResolverRegistry()
registry.register(Resolver("s1"))
registry.register(Resolver("s2"))
registry.register(Resolver("s3"))

NettyChannelBuilder
  .forTarget("s2://test")
  .usePlaintext
  .nameResolverFactory(registry.asFactory())
  .build()

After upgrading to GRPC 1.60, .build() throws the following exception:

java.lang.IllegalArgumentException: Could not find a NameResolverProvider for s2://test
	at io.grpc.internal.ManagedChannelImpl.getNameResolver(ManagedChannelImpl.java:741)
	at io.grpc.internal.ManagedChannelImpl.getNameResolver(ManagedChannelImpl.java:771)
	at io.grpc.internal.ManagedChannelImpl.<init>(ManagedChannelImpl.java:635)
	at io.grpc.internal.ManagedChannelImplBuilder.build(ManagedChannelImplBuilder.java:662)
	at io.grpc.ForwardingChannelBuilder2.build(ForwardingChannelBuilder2.java:254)

Setting an IDE breakpoint in ManagedChannelImpl#getNameResolver shows:

So what's happening is that there's technically no longer a provider for s2, because the singular wrapper class NameResolverFactoryToProviderFacade has a registered scheme of s1.

Is there anything I can do to work around this? thanks!

[ejona86]

We were mainly trying to preserve compatibility with NameResolver.Factory while propagating NameResolverRegistry deeper through the code. registry.asFactory() existed for gRPC to convert, but admit I wasn't expecting others to use it like that. (That's very similar to what gRPC does, but because we were supporting the old way.)

Can you explain why you aren't registering in the global registry? Could you also explain why you need the multiple NameResolverProviders since this builder is for only a single target string? (I know of multiple reasons for these answers, but am interested in your situation.)

I don't think there's a workaround, other than registering in the global registry. To restore the behavior we'd either need to expose some goo to convert NameResolverRegistry's factory back to a registry, or we'd need to add a builder.nameResolverRegistry() method for you to use instead of providing a factory.

Ref: #7133

[clairemcginty]

We were mainly trying to preserve compatibility with NameResolver.Factory while propagating NameResolverRegistry deeper through the code. registry.asFactory() existed for gRPC to convert, but admit I wasn't expecting others to use it like that. (That's very similar to what gRPC does, but because we were supporting the old way.)

Can you explain why you aren't registering in the global registry? Could you also explain why you need the multiple NameResolverProviders since this builder is for only a single target string? (I know of multiple reasons for these answers, but am interested in your situation.)

I don't think there's a workaround, other than registering in the global registry. To restore the behavior we'd either need to expose some goo to convert NameResolverRegistry's factory back to a registry, or we'd need to add a builder.nameResolverRegistry() method for you to use instead of providing a factory.

Ref: #7133

Thanks for the quick response @ejona86! Basically, the use case here is for a library function (thus the need to support multiple possible target URI schemes) exported for use in map-reduce jobs. The code here would be executed in relatively isolated container environments, which are difficult to inject startup/shutdown hooks into for the global registry route (unless I'm misunderstanding how that works - I couldn't find too many examples of custom NameResolvers.)

[ejona86]

Quick reply only partly responding:
If your resolver doesn't need injected configuration, then you just create a META-INF/services/ resource with the class name inside. For example, ours for DNS. gRPC looks for those when loading using ServiceLoader

[clairemcginty]

Quick reply only partly responding: If your resolver doesn't need injected configuration, then you just create a META-INF/services/ resource with the class name inside. For example, ours for DNS. gRPC looks for those when loading using ServiceLoader

Thanks @ejona86 ! Unfortunately some of our resolvers do need extra config. As a workaround, the best thing I can think of for right now is a targetUri-aware NameResolverProvider, something like:

class TargetAwareNameResolverProvider extends NameResolverProvider {
   private final String targetScheme;
   private final NameResolver nameResolver;

   TargetAwareNameResolverProvider(URI targetUri, NameResolver.Args args) {
      this.targetScheme = targetUri.getScheme();
      switch (this.targetScheme) {
         case "s1":
            this.nameResolver = Resolver("s1")
            break;
         // etc
      }
   }

   @Override
   public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
     // assert targetUri scheme matches expected...
     return this.nameResolver;
   }
 
   @Override
    public String getDefaultScheme() {
      return this.targetScheme;
    } 
}

Also - do you think it makes sense to add validation to .nameResolverFactory(...) to check that the supplied factory only provides a single resolver?

[ejona86]

How is the library function you are exposing used? Something like (?):

builder = ManagedChannelBuilder.forTarget("foo://");
yourLibrary.configure(builder);
builder.build();

How much of a problem would it be if we solved this by adding a new method builder.nameResolverRegistry()? Since that method wouldn't exist on older versions of gRPC.

Also - do you think it makes sense to add validation to .nameResolverFactory(...) to check that the supplied factory only provides a single resolver?

There's no way to check via the Factory API.

[clairemcginty]

How is the library function you are exposing used?

It's pretty opinionated -- it accepts a targetUri and some other params (retry, serviceConfig etc) and constructs the Channel opaquely to the user.

How much of a problem would it be if we solved this by adding a new method builder.nameResolverRegistry()? Since that method wouldn't exist on older versions of gRPC.

So it would directly set nameResolverRegistry to the provided value rather than wrapping it in a NameResolverFactoryToProviderFacade? Yes, that should solve it! We're pretty strict on compatible GRPC versions, so it being an added method shouldn't be an issue.

[ejona86]

It's pretty opinionated -- it accepts a targetUri and some other params (retry, serviceConfig etc) and constructs the Channel opaquely to the user.

targetUri is passed in, so you could know which name resolver should be used. And you are in control of channel construction, which gives options.

Unfortunately some of our resolvers do need extra config.

Is that state stable for the life of the process? And is the config simply data; nothing that needs to be cleaned up/shut down? Where I'm going with this: Could you add the resolvers to the registry on first use of your library function?

---

I'm simultaneously trying to figure out the best workaround and long-term approach.

One thing I have been considering is passing NameResolverRegistry when creating the channel builder. The problem with the current nameResolverFactory is it changes the semantics of the target string in the middle of the building process. For things like OpenTelemetry, we think people may need configuration based on the target string but it is hard to implement if the target string can change in the middle of building.

[clairemcginty]

It's pretty opinionated -- it accepts a targetUri and some other params (retry, serviceConfig etc) and constructs the Channel opaquely to the user.

targetUri is passed in, so you could know which name resolver should be used. And you are in control of channel construction, which gives options.

Yeah! so I could do something like the snippet above, which isn't too much trouble, it just adds an extra layer of URI/scheme parsing on top of what the channel lookup is already doing.

Is that state stable for the life of the process? And is the config simply data; nothing that needs to be cleaned up/shut down? Where I'm going with this: Could you add the resolvers to the registry on first use of your library function?

Yes, it's simply data! would you mind expanding a bit on how this would work? i.e. would it be done by the library user?

[ejona86]

would you mind expanding a bit on how this would work?

This only works if you are just injecting data (yay!) and if the configuration is stable for the life of the process (dunno). Basically, it would probably work if "you always use the same configuration in the registry" today.

class YourLibraryClient {
  private static final Object lock = new Object();
  private static boolean inited;

  public YourLibraryClient create(String target, String serviceConfig) {
    synchronized (lock) {
      if (!inited) {
        inited = true;
        NameResolverRegistry.getDefault().register(Resolver("s1"));
        NameResolverRegistry.getDefault().register(Resolver("s2"));
        NameResolverRegistry.getDefault().register(Resolver("s3"));
      }
    }
    // your existing code
  }

[ejona86]

API review meeting:

• Introduce ability to pass NameResolverRegistry to the channel builder when being built (Grpc.newManagedChannel(String, ChannelCredentials, NameResolverRegistry))
• That would also be a solution to many cases that are using channelBuilder.nameResolverFactory()
• May want to delay this until after we split "default scheme" from provider selection. This hasn't really been discussed anywhere, but we should allow setting a default prefix (e.g., "dns://1.1.1.1/" or "xds://my.control.plane/"), and that seems it should be a separate decision than the priority order for a particular scheme. (The application author should be in control of the setting, not the name resolver library authors.)