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

Retrieve ILogger from IServiceProvider #5

Closed
adamvoss opened this issue Jun 28, 2015 · 1 comment
Closed

Retrieve ILogger from IServiceProvider #5

adamvoss opened this issue Jun 28, 2015 · 1 comment

Comments

@adamvoss
Copy link
Contributor

I read in aspnet/Logging#182 this project will use the ILogger implementation from Log.Logger if none is specified.

I understand and appreciate the need for the static/global Log; however, its seems out of place to me in cases where dependency injection is first class like in the ASP.NET 5 framework. There, it would seem more appropriate to get one from the IServiceProvider that is used for dependency resolution.

While, the current implementation works and users can choose to resolve via the DI mechanism themselves; however, I am wondering whether it would be worthwhile to have an API where serilog-framework-logging would try to resolve/retrieve the ILogger before falling back to the static Log. This could be an extension method on IApplicationBuilder that uses IApplicationBuilder.ApplicationServices meaning usage would be along the lines of app.UseSerilog().

@nblumhardt
Copy link
Member

Thanks for the feedback; I'm keen to see how things fall out from usage. The Log class is always going to be around, so no API like this could be watertight - so unless Log is configured it will always be possible to call it (and preferable in many cases I think, since the overhead of a DI resolve is going to significant if it is done for every static logging call).

It'd be interesting to see a PoC if you end up with one - it'd be great if you are able to share your experiences here. I might close this for now as I don't think it's something we'll tackle in the short term in this project.

Cheers!

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

No branches or pull requests

2 participants