Update EFLogging sample code to not use obsolete functions

[0xced]

The current Logging documentation uses a now obsolete function as of Microsoft.Extensions.Logging.Console 2.2.0. This pull request adapts the sample code to the recommended replacement method.

[divega]

@ajcvickers are you aware of what parts of the logging API were made obsolete in 2.2 or is this something we need to investigate/follow up on?

I am assigning this to you, hoping that you have the relevant information and can review the PR, but otherwise let me know.

[ajcvickers]

@divega Not really. @davidfowl mentioned it, but my request for details from @pakrym was rejected.

[pakrym]

@divega Not really. @davidfowl mentioned it, but my request for details from @pakrym was rejected.

Sorry, I thought I'd fix EF docs along with writing migration guide.

As you might know, we moved logging to use DI in 2.2 and added a centralized way to do filtering and configuration. So obsolete members mostly were:

1. Extension methods for ILoggerFactory (they moved to DI aware ILoggingBuilder)
2. Logger specific configuration and filtering constructors - console logger had the ability to pass both IConfiguration and filter Func (all these features moved to ILoggingBuilder too)
3. Some old LoggerSettings classes that were converted into Options objects.

[ajcvickers]

@divega I think we'll want to add some sugar here. It's super sucky that the code requires using D.I. even when the application is not using D.I. at all and the developer may not have a good understanding of how D.I. works. Makes me wonder if we should get away from exposing this at all in EF and instead have our own logging configuration API that is more aligned to how logging is used in EF.

[0xced]

It's super sucky that the code requires using D.I. even when the application is not using D.I. at all

Absolutely agree. I also tried a non-DI solution, but it’s even uglier than with DI! 🙁

var configureNamedOptions = new ConfigureNamedOptions<ConsoleLoggerOptions>("", null);
var optionsFactory = new OptionsFactory<ConsoleLoggerOptions>(new []{ configureNamedOptions }, Enumerable.Empty<IPostConfigureOptions<ConsoleLoggerOptions>>());
var optionsMonitor = new OptionsMonitor<ConsoleLoggerOptions>(optionsFactory, Enumerable.Empty<IOptionsChangeTokenSource<ConsoleLoggerOptions>>(), new OptionsCache<ConsoleLoggerOptions>());
var loggerFactory = new LoggerFactory(new[] { new ConsoleLoggerProvider(optionsMonitor) }, new LoggerFilterOptions { MinLevel = LogLevel.Information });

[divega]

@ajcvickers I created https://github.com/aspnet/EntityFrameworkCore/issues/14136 so that we can discuss this.

Thanks @0xced for bringing this to our attention.

[divega]

@ajcvickers @0xced @pakrym

From https://github.com/aspnet/Extensions/issues/615#issuecomment-447061661, it seems that there is going to be a replacement in 3.0. Unless I am missing something, the replacement isn't the exact same API that existed before, but should allow similar usage.

So, what do you think we should do now with this PR?

My take is that we should leave the sample code as is. It is using obsolete API but hopefully it will be easier to make the switch to the new API in 3.0 than to rewrite everything now to use use DI in an application that doesn't use DI.

On the other hand, the fact that the ILoggerFactory extensions are completely gone in 3.0 is going to make it hard to discover how to do the transition.

[ajcvickers]

@divega It seems to me that the obsolete process was done incorrectly, because Obsolete was added before there was anything acceptable to replace it with. I honestly don't know what to do here--all options seem to suck.

[divega]

@pakrym @eilon could the ILoggerFactory extension methods be added back with ObsoleteAttribute in 3.0? I get that this is not a very common thing to do, but even if they had IsError=true and NotSupportedException in the body, the could still carry guidance on what options there are to replace them.

The new Create overload actually looks pretty nice to me, but this is a hard break for 3.0 and I wonder if we can mitigate it somehow.

[divega]

@ajcvickers given we already discussed and decided not to change direction with the obsolete/removed APIs, what would you like to do with this PR? My take is that adding a comment in the text saying to ignore the warnings and/or adding a suppression in the code would be enough for 2.2.

[ajcvickers]

@divega I'm okay with that.

[divega]

a comment in the text saying to ignore the warnings and/or adding a suppression in the code would be enough for 2.2.

@0xced If you would like to implement that in this PR I can wait for you. Let me know.

[divega]

Thank you @0xced for bringing this up to our attention. I have created a new PR with the comments we decided to have suggesting to ignore the obsolete warnings in 2.2. I also cherry picked the your commits that updated the projects to the new format and the dependency versions.