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

2.1.0 Release #12

Merged
merged 9 commits into from
Oct 22, 2017
Merged

2.1.0 Release #12

merged 9 commits into from
Oct 22, 2017

Conversation

nblumhardt
Copy link
Member

@nblumhardt nblumhardt commented Oct 21, 2017

@toddlucas
Copy link

This looks like a step in the right direction. I would like to set up logging in Startup. A big part of what this method is doing is adding the factory singleton to services, which often happens inside Startup.ConfigureServices. In Core 2.0 the pattern seems to be:

ConfigureServices(IServiceCollection services) { 
    services.AddMyService();
    services.AddMvc();
} 

and in Startup.Configure:

Configure(IApplicationBuilder app) {
    app.UseMyService();
    app.UseMvc(); 
}

If the UseSerilog() extension wrapped a new AddSerilog() extension which would add the factory, then it would be possible to call services.AddSerilog() from ConfigureServices as above.

I would write my own, but right now I can't create an extension like that because SerilogLoggerFactory isn't public. I can write my own factory, since SerilogLoggerProvider is available, but it would be nice to use the one from this project. Adding the AddSerilog extension would make it possible to follow the new typical pattern. Thanks!

@nblumhardt
Copy link
Member Author

Hi @toddlucas - we're keen to keep the top-level UseSerilog() pattern for now, but thanks for the suggestion - will keep it in mind. Cheers! 👍

@nblumhardt nblumhardt merged commit 6e59a36 into master Oct 22, 2017
@toddlucas
Copy link

Thanks @nblumhardt. If you're amenable, I might create a PR to show you an example. The UseSerilog() pattern wouldn't need to change.

@nblumhardt
Copy link
Member Author

Thanks for the follow-up, Todd. Just to zoom out for a second, what't the advantage to be gained by configuring Serilog in Startup.cs? Thanks!

@toddlucas
Copy link

Actually, I've taken another look and the Add/Use way appears to be more of a 1.x convention--at least for logging. 2.x has moved default configuration and logging setup to Program.cs. It's often hidden in templates that use CreateDefaultBuilder(). It makes sense to be able to set it up earlier, before Startup is invoked. Please disregard.

@nblumhardt
Copy link
Member Author

Okay, thanks for the info 👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants