Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow minimal host to be created without default HostBuilder behavior #32485

Closed
davidfowl opened this issue May 7, 2021 · 59 comments · Fixed by #46040
Closed

Allow minimal host to be created without default HostBuilder behavior #32485

davidfowl opened this issue May 7, 2021 · 59 comments · Fixed by #46040
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks feature-minimal-hosting Priority:2 Work that is important, but not critical for the release triage-focus Add this label to flag the issue for focus at triage
Milestone

Comments

@davidfowl
Copy link
Member

davidfowl commented May 7, 2021

Background and Motivation

The default host builder pulls in a ton of dependencies that are rooted for trimming by default. We should have a mode where the minimal host is dependency minimal as well.

These numbers are native AOT net8.0 compiled on linx-x64:

  1. WebApplication.CreateBuilder() = 18.7 MB
  2. With prototype changes of the proposed API - 12.9 MB

We can stick to the current naming convention and use the existing Create/CreateBuilder methods as the one with the defaults and CreateEmptyBuilder to be the empty one:

Proposed API

namespace Microsoft.AspNetCore.Builder
{
    public class WebApplication
    {
       public static WebApplication Create(string[] args);
       public static WebApplicationBuilder CreateBuilder();
       public static WebApplicationBuilder CreateBuilder(string[] args);
       public static WebApplicationBuilder CreateBuilder(WebApplicationOptions options);
+      public static WebApplicationBuilder CreateEmptyBuilder();
+      public static WebApplicationBuilder CreateEmptyBuilder(string[] args);
+      public static WebApplicationBuilder CreateEmptyBuilder(WebApplicationOptions options);
    }
}

namespace  Microsoft.Extensions.Hosting
{
    public class GenericHostWebHostBuilderExtensions
    {
       public static IHostBuilder ConfigureWebHost(this IHostBuilder builder, Action<IWebHostBuilder> configure)
       public static IHostBuilder ConfigureWebHost(this IHostBuilder builder, Action<IWebHostBuilder> configure, Action<WebHostBuilderOptions> configureWebHostBuilder)
+      public static IHostBuilder ConfigureEmptyWebHost(this IHostBuilder builder, Action<IWebHostBuilder> configure, Action<WebHostBuilderOptions> configureWebHostBuilder)
    }
}

API Usage

WebApplicationBuilder builder = WebApplication.CreateEmptyBuilder();
builder.Logging.AddConsole();

WebApplication app = builder.Build();

app.MapGet("/", () => "Hello World");

app.Run();

Features

When creating an "empty" web application, the following features will be on (✅) or off (❌) by default. Note that these features can be enabled by the app explicitly after creating the builder/application. They just won't be enabled by default, in order to reduce app size.

  • Features
    • StaticWebAssets ❌
    • IHostingStartup ❌
  • Configuration
    • command line args ✅
    • appsetttings.json ❌
    • User secrets ❌
    • env variables ✅
  • Middleware
    • Routing ✅
    • Auth ❌
    • HostFiltering ❌
    • ForwardedHeaders ❌
  • Logging
    • Logging configuration support ❌
    • ConsoleLogger ❌
    • DebugLogger ❌
    • EventSourceLogger ❌
  • Servers
    • IIS in proc ❌
    • IIS out of proc ❌
    • Kestrel
      • HTTP 1 ✅
      • HTTPS ❌
      • HTTP 2 ❌
      • HTTP 3 ❌

Alternative Designs

We could create whole new class names for another type of "Application" than a "WebApplication". Something like:

var apiBuilder = ApiApplication.CreateBuilder(args);
var apiApp = builder.Builder();

Other possible names include:

  • HttpApplication
  • ContainerApplication
  • CloudApplication
  • WorkerApplication

Risks

Potential confusion about which one to call. The template should make it obvious that the "empty" builder doesn't have anything in it, and you need to add things like appsettings, console logging, etc.

@davidfowl davidfowl added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews feature-minimal-hosting labels May 7, 2021
@Pilchie
Copy link
Member

Pilchie commented May 7, 2021

4 and 5 appear to be the same config above?

@davidfowl
Copy link
Member Author

Fixed

@BrennanConroy BrennanConroy added this to the Next sprint planning milestone May 7, 2021
@ghost
Copy link

ghost commented May 7, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@pranavkm
Copy link
Contributor

API review will tolerate CreateEmptyBuilder as an API here given the goals here.

@webczat
Copy link

webczat commented May 12, 2021

I am one of the people who would still like it, so +1! also I'm for CreateBuilder vs CreateBuilderWithDefaults or something like that, because it explicitly says what's the intention here without the confusion. Considering that the WebApplication api is not yet in any stable version of aspnetcore, it's still open for such changes, isn't it?
CreareBuilder vs CreateEmptyBuilder looks more confusing by name.

@webczat
Copy link

webczat commented May 12, 2021

also my non trimming usecase is: for example I would like to create a local webapp, the one that listens on localhost and it makes no sense for it to be exposed or have a reverse proxy in front or actually even have most of the config knops or files. And it would have config in some user profile directory. And things like that. I don't need things like the iis integration, forwarded headers, etc, even for the theoretical future, so I would prefer not to bring all these.
Other stuff although it's just personal preference and can likely be done with non empty builder, I do not necessarily want the default logging and kestrel sections to be named "Logging" and "Kestrel". And to be fair, in case of the app I mentioned, maybe I don't want them at all at least the Kestrel one. There is limited utility in setting up kestrel to do anything beyond listening to localhost in this case. Even urls are a bit too much because exposing that would be super dangerous. But that's likely not a problem.

@davidfowl
Copy link
Member Author

@DamianEdwards suggested adding a boolean to the existing CreateBuilder API

@webczat
Copy link

webczat commented May 12, 2021

well... could be. I am not even sure my first idea wasn't like that :) but I'd be okay with such a bool.

@DamianEdwards
Copy link
Member

What I suggested:

namespace Microsoft.AspNetCore.Builder
{
    public partial class WebApplication
    {
+       public static WebApplicationBuilder CreateBuilder(string[] args = null); // Calls overload below with useDefaults=true
+       public static WebApplicationBuilder CreateBuilder(bool useDefaults, string[] args = null);
    }
}

@webczat
Copy link

webczat commented May 12, 2021

what about only one overload and useDefaults as an optional parameter set to true by default? Is that even acceptable? considering it doesn't have to be compatible with anything...

@davidfowl
Copy link
Member Author

what about only one overload and useDefaults as an optional parameter set to true by default? Is that even acceptable? considering it doesn't have to be compatible with anything...

It is.

namespace Microsoft.AspNetCore.Builder
{
    public partial class WebApplication
    {
+       public static WebApplicationBuilder CreateBuilder(string[] args = null, bool useDefaults = true);
    }
}

@pranavkm
Copy link
Contributor

By the way, a parameter might make it hard for trim-analysis for the scenario @webczat laid out. The trimmer would not be able to discern that pieces are unused unless we introduce a feature flag. Having an explicit overload helps that case.

@webczat
Copy link

webczat commented May 12, 2021

ooh. Wouldn't think about that.

@pranavkm
Copy link
Contributor

We think this still needs more work:

// Useful but minimal
WebApplication
    .CreateBuilder()

// Kitchen-sink. Needs more work
WebApplication
    .CreateMaximalBuilder()

// Full control
WebApplication
    .CreateEmptyBuilder()

@DamianEdwards
Copy link
Member

I still prefer the parameter-based approach personally:

// Useful defaults
WebApplication.CreateBuilder()

// Empty builder
WebApplication.CreateBuilder(useDefaults: false)

What is the kitchen-sink/maximal builder about? Why do I want that and what is in it?

@webczat
Copy link

webczat commented May 17, 2021

well also maximal builder name looks soooo bad to me... maybe let's just stay with two paths, not three?

@davidfowl
Copy link
Member Author

@DamianEdwards but it defeats the trimming so it's a no-go.

@webczat
We replaced Maximal with Default as a strawman because Default isn't a good name to describe what it does.

@webczat
Copy link

webczat commented May 17, 2021

on a bit unrelated note, empty host is empty, it does not even have host configuration like the normal one does? so I would need to call host's methods directly for it, then use the normal Config property for the app one?

@webczat
Copy link

webczat commented May 17, 2021

@webczat
We replaced Maximal with Default as a strawman because Default isn't a good name to describe what it does.

Well but here I see three variants of CreateBuilder. so not sure...

@DamianEdwards
Copy link
Member

@davidfowl I thought the trimming issue was only with params that have a default value? I'm not suggesting that, I was suggesting concrete overloads.

@BrennanConroy
Copy link
Member

API review: rejected, we'll look into linker flags

@davidfowl
Copy link
Member Author

@eerhardt we need some assistance with linker flags to understand what's possible to do here.

@webczat
Copy link

webczat commented May 24, 2021

what does that mean?

@eerhardt
Copy link
Member

we need some assistance with linker flags to understand what's possible to do here.

Take a look at https://github.com/dotnet/designs/blob/main/accepted/2020/feature-switch.md and https://github.com/dotnet/designs/blob/main/accepted/2020/linking-libraries.md#feature-switches to get an idea on what is possible to do with feature switches.

Although, they are mainly used when you don't have a public API that can control what is pulled in, and what isn't. A concrete example is the InvariantGlobalization mode, which allows for culture aware dependencies (like libicu) to be trimmed. See https://github.com/dotnet/runtime/blob/main/docs/workflow/trimming/feature-switches.md for the current list of feature switches available in the runtime, which you can model after.

My opinion would be to try to figure out a new public method that could be added that did the intended behavior. That seems like the more appropriate design here. Feature switches are hard to discover, and they aren't part of the code. It is less surprising for developers if they called method Foo and got the same behavior, no matter what settings the application was published with.

@eerhardt eerhardt added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Dec 6, 2022
@ghost
Copy link

ghost commented Dec 6, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@eerhardt
Copy link
Member

eerhardt commented Dec 6, 2022

I've updated the top post with my current API proposal and marked this "ready for review". Please take a look and let me know if you have any feedback.

@halter73
Copy link
Member

halter73 commented Dec 9, 2022

API Review Notes:

  • Why cannot we not have a useDefaults parameter or property on the options? Adding new variants of CreateBuilder could be confusing. The trimmer cannot "see" this.
  • Can we make the "empty" builder more empty by removing the default command line and environment config sources?
    • We already removed all the default loggers.
    • In addition to removing the default config sources, can we also remove routing middleware?
      • We would have to blow up at runtime (and/or have analyzers) if you call MapGet/MapPost/etc... without adding routing middleware.
    • Can we have a single method that adds back the normal defaults? Possibly, as a follow up.
  • Do we have a better word than "empty" since there are default services, config sources, and middleware. Just less.
    • Minimal? 😛 Small? Optimizable?
    • Slim? We kinda like it. SemaphoreSlim is prior art.
  • IHostingStartup is removed because it requires using reflection to scan the assembly. Do we need a way to opt into this? Are you forced to add things like App Service logging manually? (e.g. AddAzureWebAppDiagnostics). It looks like it. IHostingStartup is unlikely to play well with AOT and trimming.
  • Are we going to need new Kestrel APIs to add it without TLS, HTTP/2 and/or HTTP/3? Yes. Later.
  • Can we add the console logger and debug logger back? They're big because of the generic explosion on TState structs. Without appsettings, the console is a lot noisier because of per-request info-level logs from hosting.
  • What about HostFiltering and ForwardedHeaders? You can add it manually. Without the appsettings from our template, HostFiltering will throw by default for having no allowed hosts. Not having the middleware is equivalent to the "*" behavior we get with the templated appsettings.
namespace Microsoft.AspNetCore.Builder
{
    public class WebApplication
    {
       public static WebApplication Create(string[] args);
       public static WebApplicationBuilder CreateBuilder();
       public static WebApplicationBuilder CreateBuilder(string[] args);
       public static WebApplicationBuilder CreateBuilder(WebApplicationOptions options);
+      public static WebApplicationBuilder CreateSlimBuilder();
+      public static WebApplicationBuilder CreateSlimBuilder(string[] args);
+      public static WebApplicationBuilder CreateSlimBuilder(WebApplicationOptions options);
    }
}

namespace  Microsoft.Extensions.Hosting
{
    public class GenericHostWebHostBuilderExtensions
    {
       public static IHostBuilder ConfigureWebHost(this IHostBuilder builder, Action<IWebHostBuilder> configure)
       public static IHostBuilder ConfigureWebHost(this IHostBuilder builder, Action<IWebHostBuilder> configure, Action<WebHostBuilderOptions> configureWebHostBuilder)
+      public static IHostBuilder ConfigureSlimWebHost(this IHostBuilder builder, Action<IWebHostBuilder> configure, Action<WebHostBuilderOptions> configureWebHostBuilder)
    }
}

When creating an "slim" web application, the following features will be on (✅) or off (❌) by default:

  • Features
    • StaticWebAssets ❌
    • IHostingStartup ❌
  • Configuration
    • command line args ✅
    • appsettings.json ❌
    • User secrets ❌
    • env variables ✅
  • Middleware
    • Routing ✅
    • Auth ❌
    • HostFiltering ❌
    • ForwardedHeaders ❌
  • Logging
    • Logging configuration support ❌
    • ConsoleLogger ❌
    • DebugLogger ❌
    • EventSourceLogger ❌
  • Servers
    • IIS in proc ❌
    • IIS out of proc ❌
    • Kestrel
      • HTTP 1 ✅
      • HTTP 2 ❌
      • HTTP 3 ❌
      • HTTPS/TLS ❌
      • Sockets ✅
      • QUIC ❌
      • Named Pipes ❌

API Approved for preview1! We should have a lot of time to collect feedback from the community.

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Dec 9, 2022
@davidfowl
Copy link
Member Author

davidfowl commented Dec 9, 2022

SLIM 😄

@DamianEdwards
Copy link
Member

DamianEdwards commented Dec 9, 2022

Note on IHostingStartup: it's used to enable various inner-loop features (i.e. in VS features during dev time). We might want to think about how we could enable those to still work during inner-loop given the app is still running on CoreCLR there, e.g. ideas:

  • a cmd-line arg/env-var when the app is launched that enables IHostingStartup again
  • a partial Program.dev.cs that contains a dev-time bootstrap method with a body that's behind a #if DEBUG compiler switch that adds services required for inner-loop experiences to work, the method would be called from Progam.cs

Similar story for UserSecrets which is only intended for use during inner-loop and is the foundation of other dev-time experiences like dotnet user-jwts.

@webczat
Copy link

webczat commented Dec 9, 2022

about routing middleware, isn't it that it's not added unless you call the map family of functions? or maybe it's only not added if you add it manually.
Anyway, you also mean you don't add routing services?

@davidfowl
Copy link
Member Author

@webczat

about routing middleware, isn't it that it's not added unless you call the map family of functions? or maybe it's only not added if you add it manually.

Chicken and egg problem. The Map functions are defined on the application and routing needs to be added so that they can be used.

@davidfowl
Copy link
Member Author

@eerhardt It should be possible to turn on these features after the fact.

@eerhardt
Copy link
Member

eerhardt commented Dec 9, 2022

It should be possible to turn on these features after the fact.

I've updated the top proposal's Features section to make this more explicit.

eerhardt added a commit to eerhardt/aspnetcore that referenced this issue Jan 19, 2023
This adds a new Hosting API to reduce startup and app size, and ensures the default behavior is NativeAOT compatible.

Fix dotnet#32485
eerhardt added a commit that referenced this issue Jan 21, 2023
…#46040)

* Allow minimal host to be created without default HostBuilder behavior

This adds a new Hosting API to reduce startup and app size, and ensures the default behavior is NativeAOT compatible.

Fix #32485

* Use the new slim hosting API in the api template.

Refactor the WebHostBuilder classes to share more code.
@ghost ghost locked as resolved and limited conversation to collaborators Feb 20, 2023
@davidfowl
Copy link
Member Author

@eerhardt I still think we should do CreateEmptyBuilder

@DamianEdwards
Copy link
Member

@eerhardt I still think we should do CreateEmptyBuilder

💯

@eerhardt
Copy link
Member

Agreed. Especially now that we added stuff back to "SlimBuilder" to make it more convenient. I can open a new issue specifically for it.

@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
@eerhardt
Copy link
Member

I opened #48811 for CreateEmptyBuilder.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks feature-minimal-hosting Priority:2 Work that is important, but not critical for the release triage-focus Add this label to flag the issue for focus at triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.