JasperFx / lamar

Fast Inversion of Control Tool and Successor to StructureMap
https://jasperfx.github.io/lamar
MIT License
572 stars 119 forks source link

ASP.net Core Logging Configuration is lost with WebHost.CreateDefaultBuilder() #80

Closed xorets closed 5 years ago

xorets commented 6 years ago

According to the documentation you must have .UseLamar() before everything else after new WebHostBuilder(). But nowadays there is that WebHost.CreateDefaultBuilder() helper that everybody is likely to use, so you can't actually initialize Lamare before everything else. It could look only like this:

public static IWebHostBuilder CreateWebHostBuilder( string[] args ) =>
    WebHost.CreateDefaultBuilder( args )
        .UseLamar()
        .UseStartup<Startup>();

Unfortunately, this doesn't work well - at least logging configuration gets lost. CreateDefaultBuilder() configures logging in this way:

.ConfigureLogging( ( hostingContext, logging ) =>
{
    logging.AddConfiguration( hostingContext.Configuration.GetSection( "Logging" ) );
    logging.AddConsole();
    logging.AddDebug();
} )

And all this configuration doesn't reach Startup.Configure method. I don't know if something else also gets corrupted.

xorets commented 6 years ago

Update: Testing shows that it's not enough to just put .UseLamar() before everything else. Second factor that affects logging configuration is the existence of public void ConfigureContainer( ServiceRegistry services ) method in your Startup class.

If even empty ConfigureContainer exitst, then logger factory will have empty rules. Remove the method and rules from "appsettings.json" are there.

jeremydmiller commented 6 years ago

@AndrewMayorov Hey man, it's not terribly clear what the actual problem is here. Can you add some context about what exactly doesn't work?

xorets commented 6 years ago

@jeremydmiller, as I understood so far Lamar "clears" the logger rules. Say you have this logger configufation:

 "Logging": {
    "IncludeScopes": false,
    "LogLevel": {
      "Default": "Error"
    }
  }

Without Lamar you see only error messages in your console/debug outputs. Add Lamar and you start to see every trace message.

To check add ILoggerFactory to Configure:

public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
{
   // Evaluate loggerFactory here. 
   // Normally loggerFactory._filterOptions.Rules should contain entries configured from appsettings.json
   // With Lamar it's empty.
}
jeremydmiller commented 5 years ago

Renamed the title because it was misleading. Still next up for research.

jeremydmiller commented 5 years ago

@AndrewMayorov @CodingGorilla Finally had time today to look at this. Wasn't even remotely what I thought it would end up being. Fix will pop up tonight.

CodingGorilla commented 5 years ago

Can't wait to see the fix, I was digging around trying to figure out what it was yesterday, thinking maybe it was related to my issues with Jasper and IOptions<>, but I never was able to track it down.

jeremydmiller commented 5 years ago

LoggerFactory got us. Weird confluence of things made Lamar choose a constructor function that took in LoggerFilterOptions instead of IOptions<LoggerFilterOptions>. In that case, Lamar used a new LoggerFilterOptions() build method instead of bouncing through the IOptions.

Not exactly sure what I want to do to fix this permanently, but I've got at least a couple options.

CodingGorilla commented 5 years ago

That was what I was looking at actually, so I wasn't far off. I just couldn't figure out which constructor it would "normally" be using without Lamar, I was digging through the actual ASP.Net code trying to figure that out, but ran out of "free time". Well at least I know I was on the right track. :)

CodingGorilla commented 5 years ago

On a related note, the constructor selection seems a bit odd to me. Lamar will attempt to use the constructor with the most parameters assuming it can provide all the parameters, rather than the least parameters or the simplest constructor.

What's the reasoning behind that decision?