Support Microsoft.Extensions.Hosting.Abstractions

[ReubenBond]

In Orleans 2.0, we added SiloHostBuilder, which was designed to align with IHostBuilder from the Microsoft.Extensions.Hosting.Abstractions package (which wasn't yet released).

The intention was that SiloHostBuilder would be temporary and we would add support for the generic host once it became available.

It's available now, so we can consider adding support for it.

ASP.NET folks have a similar issue opened for WebHostBuilder: https://github.com/aspnet/Hosting/issues/1421. They've scheduled it for 3.0.0, so I imagine it will be a year or more before they move over.

I have a PoC for this on a branch here: master...ReubenBond:feature-generic-host

cc @galvesribeiro & @attilah who asked for this today

[ReubenBond]

There is an open question about API style:

• Do we configure everything on the top-level IHostBuilder?
• Do we have a sub-builder which wraps IHostBuilder?
• Or do we use another style?

Top-level builder

Attach all configuration methods to IHostBuilder directly.

hostBuilder
  .AddOrleans()
  .UseLocalhostClustering()
  .AddMemoryGrainStorage("MemoryStore");

Pro: It's more obvious that there is a single DI container
Con: Extension methods don't have a clear target (is UseLocalhostClustering for Orleans or for SignalR? - less obvious)

Sub-builder

Create a new type, IOrleansBuilder, which has an IHostBuilder property.

hostBuilder.AddOrleans(builder =>
  builder
    .UseLocalhostClustering()
    .AddMemoryGrainStorage("MemoryStore"));

Pro: It's clear that those methods belong to Orleans and they will appear via intellisense in the correct context.
Con: It's not clear that there is a single DI container being used, so changes to services made by the AddOrleans configuration delegate will affect the shared container. Eg, if the user thinks that they can configure logging differently for Orleans and ASP.NET Core then they are mistaken.

As a potential workaround for that con, we can create a new builder type which makes the relationship to IHostBuilder more explicit:

public interface IOrleansBuilder
{
    IHostBuilder HostBuilder { get; }
}

This can then be implemented explicitly to hide HostBuilder.

public class OrleansBuilder : IOrleansBuilder
{
    private readonly IHostBuilder builder;
    public OrleansBuilder(IHostBuilder builder) => this.builder = builder;
    IHostBuilder IOrleansBuilder.HostBuilder => this.builder;
}

That helps to hide the IHostBuilder so that end-users aren't tempted to call Build() from within the AddOrleans delegate or to try configuring logging separately for Orleans and another hosted service.

At the same time, extension writers see IHostBuilder clearly and understand that there is only one IHostBuilder, so the pattern and implications ought to be more clear to them.

Extension methods will look like this:

public static class OrleansBuilderExtensions
{
    public static TBuilder ConfigureThingy<TBuilder>(
      this TBuilder builder,
      Action<OptionsBuilder<ThingyOptions>> configure) where TBuilder : IOrleansBuilder
    {
        // Do something with HostBuilder, like configure services.
        return builder;
    }
}

Then we can expose the OrleansBuilder class (not the IOrleansBuilder interface) in the AddOrleans delegate:

public static IHostBuilder AddOrleans(
  this IHostBuilder hostBuilder,
  Action<OrleansBuilder> configure)
{
    configure(new OrleansBuilder(hostBuilder));
    return hostBuilder;
}

This way, the end-user can use extension methods defined against IOrleansBuilder without having easy access to methods on IHostBuilder such as ConfigureServices, ConfigureServiceProvider, and Build.

---

I believe that the proposed pattern with the sub-builder presents a better user experience to the top-level builder. We could consider similar changes for the client builder, but I don't have a well-formed opinion on that yet.

Suggestions welcome 😊

xref: aspnet/Hosting#1279
cc @jdom

[jdom]

My vote is for option 2 (sub builder). We talked and tried many things with option 1 and variants of 2, but we concluded that configuring in the context of an Orleans builder is very important. There are already a few things that work in that way in the Microsoft.Extensions namespace, such as when you configure Logging or use .AddOptions(), etc, those builders actually end up configuring the one shared container, even though that Intellisense isn't entirely obvious that it's not a new instance of the Logging infrastructure, etc. This is where familiarity plays in our favor, and hopefully future frameworks will implement sub-builders too (I'd double check with @davidfowl now that there is a closer roadmap to integrate ASP.NET with the generic host).

Also, all the temporary hosting work we did today in Orleans 2.0 would make that option straightforward for our users to move to (albeit some minor tweaking of 1 or 2 lines of code in their setup). In fact, the team agreed to remove all the duplicate extension methods that configure IServiceCollection directly and just kept the ones on ISiloHostBuilder particularly to make the transition to option 2 easier on our users, since we deemed the sub builder context as very important even back then.
We need to make sure that our sub builder no longer has extension methods to configure generic cross-cutting concerns such as logging (unless of course it's an Orleans-specific customization).

I'm not too convinced about hiding IHostBuilder on the sub-builder though. I haven't formed an opinion one way or the other, but at least initially I'm not seeing the confusion/harm about being transparent about it. If they happen to call .Build() on it, and then they try to continue to configure it, then it will be immediately obvious that it doesn't work.

[SebastianStehle]

Some examples for the syntax of variant 1:
MVC
Authorization
Identity
IdentityServer

But variant 1 would just return the subbuilder

[jason-bragg]

While my views on this are still quite malleable, atm, I also advocate a sub-builder. I see an Orleans silo as a hosted service, and would prefer to configure it as such using an ISiloBuilder. The move to IHostBuilder should not necessarily deprecate ISiloHostBuilder, as silo host builder can be maintained as a façade over IHostBuilder to support the common configuration pattern of a host with a single Orleans silo.

[ReubenBond]

@SebastianStehle the thing is, ASP.NET is in a transition and is still evolving. Those patterns were created for the Startup Class configuration model, which predates the Generic Host work. ASP.NET folks have said in the past that they want to get rid of that model since it has problems. For example it requires a temporary container and doesn't play well with hosting (eg, how do you pass context from Service Fabric to it?).

We deprecated the Startup Class with the 2.0 transition and are already on a model very similar to the Generic Host. Now we need to consider what the .NET ecosystem is going to look like once others make the transition.

The issue with AddX() returning IXBuilder is that it breaks continuity with the chaining style used everywhere else. Some methods return the original builder for chaining, some return a different builder, some methods accept a configuration delegate for configuration and others don't. That's not consistent.

The sub-builder model is more consistent.

Still, it's important to align with whatever the rest of the ecosystem is moving towards, so I'm fine with whatever that ends up being.

[SebastianStehle]

Thx for the long explanation. Was not aware of that

[attilah]

Generally the sub-builder is a better way to go IMHO. In real world that builder could be a no-op, without a Build method just a marker interface to stuff the extension methods on, a'la Logging and MVCs builder.

For non-breaking change I'd suggest to make the method public which registers the internal Orleans things and have that on IServiceCollection instead of ISiloHostBuilder.

This has multiple positive outcomes:

• It would enable anyone to use Generic Host in their projects and do not use the ISiloHostBuilder and similar classes.
• It would not change how Orleans' is doing its things today, would still work.

Extensions are also affected, did not check (because we've a lot), but I suspect that none or at least not to many of them registering internal classes in extension methods attached to the ISiloHostBuilder interface, if they're a modification also needed there. Which again would not be breaking anyone.

Regarding the opening up of that internal method: It could be moved into an Internal namespace within the current one, but be public. ASP.NET team also follows this pattern for things that can change in the future.

It would be good to see it in 2.1.

[ReubenBond]

If we put methods on IServiceCollection instead of IHostBuilder/ISiloHostBuilder, then we cannot access HostBuilderContext, which gives us configuration/environment/etc.

[davidfowl]

We've also landed on the sub builder design. It's time to wake this thread up! 😄

[ReubenBond]

Thanks, @davidfowl - I think we're on the same page.

To enable this earlier than 3.0, we can follow this plan:

• Implement internal class SiloBuilder : ISiloBuilder as a wrapper around IHostBuilder, largely used for scoping extension methods to Orleans.

• Add a new builder type, ISiloBuilder which takes most of the methods from ISiloHostBuilder but omits the ones which must remain on IHostBuilder (configuring service provider factory, Build(), etc.)

• Implement SiloBuilder : ISiloBuilder as a wrapper around IHostBuilder, largely used for scoping extension methods to Orleans.

• Copy existing ISiloHostBuilder extension methods, changing the scope to ISiloBuilder.

• Implement IHostBuilder AddOrleans(this IHostBuilder builder, Action<ISiloBuilder> configureDelegate) extension method for adding Orleans to the generic host builder.

• Mark all extension methods scoped to IServiceCollection as [Obsolete], pending deletion.

For 3.0, we can clean up:

• Remove ISiloHostBuilder and related classes/methods.

• Remove (already [Obsolete]) IServiceCollection methods.

This would be a breaking change, but I think the end result is better than having two entirely separate interfaces. There's only minor ease-of-upgrade benefit in keeping a separate SiloHostBuilder post-3.0. I'm in favor of unification.

The key interface:

public interface ISiloBuilder
{
  IDictionary<string, object> Properties { get; }

  ISiloBuilder ConfigureServices(Action<HostBuilderContext, IServiceCollection> configureDelegate);
}

Both methods just forward their counterparts on IHostBuilder, the interface exists largely for intellisense purposes and to hide invalid methods like Build().

---

As an alternative to removing ISiloHostBuilder, we could make it extend ISiloBuilder this would mean changing all extension methods from:

ISiloBuilder AddMyProvider(this ISiloBuilder builder, ...)

to

T AddMyProvider<T>(this T builder, ...) where T : ISiloBuilder

so that users don't lose the ability to write

var silo = new SiloHostBuilder().AddMyProvider(...).Build();

(since otherwise AddMyProvider would return ISiloBuilder which has no Build() method)

It might be worth going with this approach so that we can support adding a silo to the host builder in its own, separate container in the future, which would likely involve adding a new interface derived from ISiloBuilder.

[galvesribeiro]

Working on a PR aligned to @ReubenBond description...

[davidfowl]

cc @glennc

[SebastianStehle]

Can you not keep the ISiloHostBuilder, so that extension projects do not need an update to support GenericHost?

[galvesribeiro]

@SebastianStehle better to break then extensions than break the runtime directly IMHO

[SebastianStehle]

Yes, I had a second look to this interface and I also do not see and option to keep it.

[ReubenBond]

I assume that this proposal works for everyone. @galvesribeiro's PR looks good to me

[matthewgrimagig]

Using the existing ISiloHostBuilder, I make use of UseServiceProviderFactory, however this does not seem to be possible when using the UseOrleans extension method. Am I missing something please?

[galvesribeiro]

@matthewgrimagig instead of using that to configure the container on Orleans directly, check the Configuring Generic Host Container here.

[matthewgrimagig]

thanks @galvesribeiro, no idea how I didn't think about that. Any idea when 2.3.0 will be released?

[galvesribeiro]

Dunno. @ReubenBond perhaps have a clue.

[sergeybykov]

We can release it as early as next week. Just waiting for people to try 2.3.0-rc1 to see if we missed something.