Register Logging Services Correctly

[mark-monteiro]

Register logging services correctly with the DI container so they are resolved correctly.

Changes

• Replace existing DI type registrations with a call to UseSerilog() which registers ILoggerFactory and ILogger<T>
• Keep a registration for ILogger but add a TODO note to deprecate it in favour of ILogger<T>
• Reduce logging output from Microsoft and System namespaces to a min level of Warning, which is recommended in the Serilog documentation. If we don't to make this change, we need to at least reduce the output for Microsoft.AspNetCore which clogs up the logs with every single request to the server at level Info

Still To Address
We have no way to push an update to the logging configuration for existing users. At startup, if the user's logging.json settings file already exists, then it is loaded as-is and the bundled resource settings file is ignored.

Issues
Fixes #2498

[Bond-009]

We have no way to push an update to the logging configuration for existing users. At startup, if the user's logging.json settings file already exists, then it is loaded as-is and the bundled resource settings file is ignored.

This is a non-issue imo, either the installer or the user should handle it imo

[mark-monteiro]

We have no way to push an update to the logging configuration for existing users. At startup, if the user's logging.json settings file already exists, then it is loaded as-is and the bundled resource settings file is ignored.

This is a non-issue imo, either the installer or the user should handle it imo

Fair enough, if that's the case then I'll open this up to be merged now. For some context though, here is the mess that the log turns into if the configuration isn't updated (every server request is logged):

As an alternative, I tested loading the bundled resource as a default and then overriding with the user settings file. This works to apply the updated defaults correctly, but might make managing the logging config more confusing for users since there would no longer be a single source of truth for the logging config.

new ConfigurationBuilder()
    .SetBasePath(appPaths.ConfigurationDirectoryPath)
    .AddInMemoryCollection(ConfigurationOptions.Configuration)
    .AddJsonStream(defaultLogConfigResource)  // Load resource file as defaults
    .AddJsonFile("logging.json", false, true)          // Merge user settings from file over top

[JustAMan]

Can we check if users have a default (not user-changed) pre-this PR config file and overwrite it automatically? This would handle most update scenarios.

[mark-monteiro]

Can we check if users have a default (not user-changed) pre-this PR config file and overwrite it automatically? This would handle most update scenarios.

Yeah we could do that. Would that be something more appropriate to put in the installer perhaps? We would only want that check to run a single time during the version update, not every server startup.

[JustAMan]

Can we check if users have a default (not user-changed) pre-this PR config file and overwrite it automatically? This would handle most update scenarios.

Yeah we could do that. Would that be something more appropriate to put in the installer perhaps? We would only want that check to run a single time during the version update, not every server startup.

Just a follow-up... we have very different installers and installation methods, including those where there is no installer at all (like docker images or portable installs). So I think the server should deal with it...

[mark-monteiro]

Just a follow-up... we have very different installers and installation methods, including those where there is no installer at all (like docker images or portable installs). So I think the server should deal with it...

Created a new issue to deal with this: #2511