Add support for custom attributes in NameResolver.Helper

[ST-DDT]

What version of gRPC are you using?

1.19

Related

• Based on Breaking change in NameResolverProvider with latest release #5556 (comment)

Feature-Request

The java service loader API has the drawback that it creates instances without context.
In my case it creates NameResolverProviders without spring's dependency injection.
With the Attributes API that present up to and including 1.18, I was able to pipe the potentially required beans/config to the actual implementations. Now this path is blocked.

Lets assume we have a config which contains the "service discovery server URL".
Somehow we need to push this tiny piece of runtime only information into the ServiceDiscoveryNameResolver(Factory).

We now (1.19 or later) have basically three options:

1. Encode everything in the URI
   This will soon become a nightmare for every maintainer as more and more properties needs to be encoded for the potential case that one provider needs them. This option cannot pass more complex instances such as a RestTemplate or HttpRequestOptions.
2. Use static fields to pass the variables
   Well you can pass anything through static variables, but using them to transfer something stateful sounds like a bad idea and fails as soon as the load order is slightly of or multiple distinct instances of gprc-services run in the same JVM (to save some memory).
3. Wrap the NameResolverProvider (registry) in another (spring-managed) registry that will first try to use the contextual name resolvers before falling back to the grpc-managed ones. Well this works but somewhat defeats the purpose of specifying priorities and availability in the first place.

Prior to 1.19 we also had a forth option:

4. Pass the context through the attributes

In yidongnan/grpc-spring-boot-starter I use the third variant, but also considered using the forth variant because it would free my code from conditional wrappers and would allow adding other contextual name resolvers more easily.

ConfigMappedNameResolverFactory (springNameResolver)

    @Nullable
    @Override
    public NameResolver newNameResolver(final URI targetUri, final Attributes params) {
        final String clientName = targetUri.toString();
        final GrpcChannelProperties clientConfig = this.config.getChannel(clientName);
        URI remappedUri = clientConfig.getAddress();
        if (remappedUri == null) {
            remappedUri = this.defaultUriMapper.apply(clientName);
            if (remappedUri == null) {
                throw new IllegalStateException("No targetUri provided for '" + clientName + "'"
                        + " and defaultUri mapper returned null.");
            }
        }
        log.debug("Remapping target URI for {} to {} via {}", clientName, remappedUri, this.delegate);
        final Attributes extendedParas = params.toBuilder() // Enrich attributes with the context
                .set(NameResolverConstants.PARAMS_CLIENT_NAME, clientName)
                .set(NameResolverConstants.PARAMS_CLIENT_CONFIG, clientConfig)
                // .set(NameResolverConstants.PARAMS_APPLICATION_CONTEXT, applicationContext) // Full spring context
                .build();
        return this.delegate.newNameResolver(remappedUri, extendedParas);
    }

I currently don't use the extra attributes, but I considered it for future simplifications.

The DiscoveryClientResolverFactory is currently my only contextual name resolver. In this example I could request the DiscoveryClient bean from the applicationContext instead of passing it in the constructor.

IMO it would be best to revert the helper back to the unspecific attributes or add an optional, mutable and unspecific attributes to the helper because:

• A single getCustomAttributes() : Object would fail as soon as another layer of context is added on top.
• At least the default port is useless when other service discovery mechanisms are used. I'm not sure whether I would call it an implementation detail but at least it's related.
• There is no factory method/forwarder class to overwrite one value with another without risking to become fragile to changes to the helper class which seems to be the case for 1.21 again.

[ejona86]

We want to have a NameResolverRegistry to mirror LoadBalancerRegistry. Notably, it would have a register(NameResolverProvider) method. If we had that, it seems like you could have Spring create the NameResolverProviders with the appropriate information and then register them.

Would that work even better than trying to plumb Attributes through the NameResolver?

[ST-DDT]

Yes, that would work perfectly fine.