A110: Child Channel Options

[AgraVator]

No description provided.

[markdroth]

The overall design looks good, but I think the doc needs work.

My main concern is that we need a much better description of the overall design in a language-agnostic way, rather than just saying that we have a different design for each language. It's true that each language has its own APIs for how per-channel options are set, but the overall goal here is still to have a way to pass a set of options to be used in child channels. The semantics for that should be the same in all languages.

Please let me know if you have any questions. Thanks!

[markdroth]

This is looking much better!

Please let me know if you have any questions. Thanks!

[markdroth]

This is looking really good! The only really substantive remaining comment from me is the one from my last review pass about making sure we can plumb this into channels created via an LB policy or resolver.

[markdroth]

Just one small remaining comment, otherwise looks great!

[ejona86]

Given the current ManagedChannelBuilder.childChannelConfigurator() API, I suspect we'd want "whatever is calling build() should be applying the child config." Otherwise, that component can't compose its own configuration to child channels; if it calls childChannelConfigurator() it will overwrite any previous configuration.

Either we could move the responsibility to whatever component calls build(), or we could have managedChannelBuilder.childChannelConfigurator() append the provided configurator. It currently overwrites the previous configurator. (Note that we don't have to have a list, we could just make a new configurator that calls the old configurator first, then the newly-passed configurator.)

Note that Go and C's approaches here allow a component to supplement the child configuration; only Java and only createResolvingOobChannelBuilder() has the limitation.

@kannanjgithub, thoughts?

[AgraVator]

Yeah, we could do something like below for composition

@Override
public ManagedChannelImplBuilder childChannelConfigurator(ChannelConfigurator channelConfigurator) {
  checkNotNull(channelConfigurator, "channelConfigurator");
  ChannelConfigurator previous = this.channelConfigurator;
  this.channelConfigurator = builder -> {
    previous.configureChannelBuilder(builder);
    channelConfigurator.configureChannelBuilder(builder);
  };
  return this;
}