aspnet / Hosting

[Archived] Code for hosting and starting up an ASP.NET Core application. Project moved to https://github.com/aspnet/Extensions and https://github.com/aspnet/AspNetCore
Apache License 2.0
552 stars 312 forks source link

Question on Generic Host logging setup #1563

Closed guardrex closed 5 years ago

guardrex commented 5 years ago

@Tratcher In the Generic Host sample app, we have both AddLogging in service config and AddConsole+AddDebug in ConfigureLogging. I noticed that the call to AddLogging isn't required, and I speculated that perhaps AddConsole and AddDebug call AddLogging on their own.

We don't remark at all on AddLogging; and for ConfigureLogging all we say is ...

ConfigureLogging adds a delegate for configuring the provided ILoggingBuilder. ConfigureLogging may be called multiple times with additive results.

Do we need to make any changes or call anything out in the topic/sample regarding logging activation?

.ConfigureServices((hostContext, services) =>
{
    services.AddLogging();
    ...
})
.ConfigureLogging((hostContext, configLogging) =>
{
    configLogging.AddConsole();
    configLogging.AddDebug();
})
Tratcher commented 5 years ago

ConfigureLogging embeds AddLogging.

Edit: Also, Logging is a default service.

guardrex commented 5 years ago

Ok, thanks.

How do you want to handle it in the topic+sample? We can add a remark to the ConfigureLogging section of the topic, but that doesn't address the sample app including the call to AddLogging when it isn't necessary.

Tratcher commented 5 years ago

If anything it's there as an example for ConfigureServices. Have anything else that could go there?

It would be worth mentioning somewhere what the default services are.

guardrex commented 5 years ago

Yes, we have the hosted services there because the sample for the topic is the hosted services sample. The full method is ...

.ConfigureServices((hostContext, services) =>
{
    services.AddLogging();
    services.AddHostedService<LifetimeEventsHostedService>();
    services.AddHostedService<TimedHostedService>();
})

It would be worth mentioning somewhere what the default services are.

Very good. :+1:

I've opened https://github.com/aspnet/Docs/issues/8893 to address these.