Skip to content

Issue with this: Remove ILoggerFactory parameters and any Add*() calls on the logger factory in Startup.cs #147

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

Closed
daharmon opened this issue Nov 1, 2019 · 2 comments · Fixed by #167

Comments

@daharmon
Copy link

daharmon commented Nov 1, 2019

Hi, I'm wanting to use the serilog-aspnetcore package and am following instructions in the readme file but am stumbling on this line

Remove ILoggerFactory parameters and any Add*() calls on the logger factory in Startup.cs

I have this line in my startup.cs for registering a custom modelbinderprovider

services.AddMvc().AddMvcOptions(options => { 
  options.ModelBinderProviders.Insert(0, new GenericViewModelBinderProvider(loggerFactory));
})"

I need to pass an ILoggerFactory because GenericViewModelBinderProvider creates a new type of modelbinder which inherits from ComplexTypeModelBinder, which needs an ILoggerFactory.

My question is why do we need to remove ILoggerFactory parameters from the Startup.cs file. It would seem to me that if and what is the best method for getting the serilog logger factory within the configureservices method?

I hope this makes sense. Thank you for your time.

@nblumhardt
Copy link
Member

Hi!

I think this should work just fine, in your scenario. The advice you mention in the README is a little outdated, from a time when the default ASP.NET Core project template included a bunch of framework-specific logger setup based on ILoggerFactory, along the lines of AddConsole(), AddDebug() etc.

I think we should improve or remove this wording.

Let me know if you

@daharmon
Copy link
Author

daharmon commented Nov 4, 2019

Thank you! I figured it would work fine but I wanted to confirm there wouldn't be any hidden conflict that I wouldn't be able to find until I got into production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants