grpc-java icon indicating copy to clipboard operation
grpc-java copied to clipboard

Expose service config parsing logic

Open onyn opened this issue 1 year ago • 1 comments

Is your feature request related to a problem?

DNSNameResolver have service config parsing logic which is hidden from outer world.

Please expose json string to ConfigOrError parser. This simplifies implementation of A2 proposal by custom name resolvers.

Describe the solution you'd like

Implementation may be moved to NameResolver class with protected visibility level.

Also consider providing two methods:

  1. One for parsing A2's canarying changes json format (list of objects).
  2. Second for parsing just single service config itself.

Additional context

This feature request inspired by https://github.com/grpc-ecosystem/grpc-spring/pull/1046

onyn avatar Mar 13 '24 11:03 onyn

@ejona86 - need your opinion on this.

sergiitk avatar Mar 18 '24 16:03 sergiitk

Second for parsing just single service config itself.

"parse" can mean a few things here. The only way to get a ConfigOrError from within a NameResolver is args.getServiceConfigParser().parseServiceConfig(Map<String,?>). That's the method you want to use.

Now to parse the JSON into Map<String,?>, we purposefully don't have anything in our API. We don't want to expose a JSON implementation. But you should be able to use the JSON implementation of your choice. For Gson, it's about as easy as Gson.fromJson(json, Map.class). That's what one example does.

So I don't see much need to do anything here.

One for parsing A2's canarying changes json format (list of objects).

This is specific to DNS. We could maybe expand it to others, but DNS still doesn't have service config enabled by default... We were expecting other name resolvers to use some of their own methods for canarying. But all clients receiving the same results does seem like it would be frequent enough that other NRs might want it to select the config to use in the same way. The implementation interesting to other NRs would mostly be maybeChooseServiceConfig().

But the current implementation also suffers from https://github.com/grpc/grpc-java/issues/6579 . We'd need to resolve that before having more people use it.

ejona86 avatar Mar 21 '24 00:03 ejona86