chore: Use logging source generators

[austindrenski]

This PR updates OpenFeature to use the latest MELA source generators to produce compile-time logging delegates.

The most obvious critique of this PR is likely to be that, much like #188, this PR changes the dependency graph by bumping our MELA dependency from [2.0.0,) => [8.0.0,).

To that^ critique, I continue to contend that depending on an ancient version of MELA is unlikely to aid many real-world consumers of this SDK, since new versions of MELA provide robust TFM coverage, as well as my personal disbelief that anyone looking to consume this library in 2024 is deploying an app that won't already have transitively referenced MELA 8.0.0 somewhere in its package graph.

(If you are a user or potential user of this SDK and have a real-world use case of a legacy app that does not and cannot reference MELA >= 8.0.0, please ping back here! Would love to hear from you and adjust my disbelief accordingly!)

(Note: if this PR lands before #188, then I'll update #188 to remove its added dependency on Microsoft.Bcl.AsyncInterfaces, since it flows transitively from MELA 8.0.0.)

Upon request, I am happy to provide a soapbox diatribe on why I think we should care about source generators, hook perf, and incidental logging allocations, especially as an SDK that wants to be trusted and baked into all kinds of consumer apps, but eliding that for now in favor of some docs refs:

• https://learn.microsoft.com/dotnet/core/extensions/high-performance-logging
• https://learn.microsoft.com/dotnet/core/extensions/logger-message-generator

[austindrenski]

This is actually an interesting side-effect of bumping the MELA dependency.

Our original (ancient) MELA dependency resolves to 2.0.2 which only includes a target for netstandard2.0, whereas starting with version 6.0.0, MELA provided targets for net461, netstandard2.0, and net6.0. This continued into the 7.x series with targets for net462, netstandard2.0, net6.0, and net7.0, as well as into the 8.x series with targets for net462, netstandard2.0, net6.0, net7.0, and net8.0.

So by bumping to a modern MELA, our transitive reference to System.ValueTuple via the netstandard2.0 reference was lost, thus requiring us to take a direct dependency on System.ValueTuple.

Now it's been a minute since I had to deal with this in a real app, but when I last encountered this^ in the wild, IIRC, you always end up with a System.ValueTuple dependency somewhere in the graph, so if you were skirting it before with a faux target of net462 -> netstandard2.0, you still needed the impl bits from System.ValueTuple available by run-time, else chaos ensued.

[austindrenski]

This is effectively the same thing, but (1) it saves two allocations (albeit insignificant ones), (2) its the common/recommended pattern, and (3) its sometimes nice to be able to drop a breakpoint inside NullLogger<T> and catch callers to it, so this makes it a little more obvious what's happening in those cases.

[austindrenski]

Note the logger name/category change here. It doesn't actually matter for the no-op logger, but prestaging the change while I'm at it because this will matter in the post-#181 world because when #181 registers IFeatureClient it injects a named logger similar to how HttpClient loggers work so that consumer apps can exercise granular control over logging sources.

dotnet-sdk/src/OpenFeature.Extensions.Hosting/OpenFeatureBuilderExtensions.cs

Lines 109 to 113 in 9821a5b

	builder.Services.TryAddKeyedSingleton(providerName, static (services, providerName) => providerName switch
	{
	null => services.GetRequiredService<ILogger<FeatureClient>>(),
	not null => services.GetRequiredService<ILoggerFactory>().CreateLogger($"OpenFeature.FeatureClient.{providerName}")
	});

Such that a consumer app could do something like this in their appsettings.json to vary the minimum log levels by provider:

{
  "Logging": {
    "LogLevel": {
      "Default": "Debug",
      "OpenFeature": "Information",
      "OpenFeature.FeatureClient": "Warning",
      "OpenFeature.FeatureClient.No-op Provider": "Warning",
      "OpenFeature.FeatureClient.flagd Provider": "Information",
    }
  }
}

[austindrenski]

This call still happens, but the arg type is an internal type (e.g. LoggerMessage.LogValues<T>) which makes call checking a little peevish since we can't reference the type, so opted to change this to just check that we made it to the IsEnabled(...) check.

If there are any strong feelings about this, I can hack together something closer to the original assertion using reflection, just let me know.

For reference, here's a snippet of what the source generator emits at compile-time:

// <auto-generated/>
#nullable enable

namespace OpenFeature
{
    partial class FeatureClient
    {
        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "8.0.9.3103")]
        private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, global::System.String, global::System.Exception?> __FlagEvaluationErrorCallback =
            global::Microsoft.Extensions.Logging.LoggerMessage.Define<global::System.String>(global::Microsoft.Extensions.Logging.LogLevel.Error, new global::Microsoft.Extensions.Logging.EventId(101, nameof(FlagEvaluationError)), "Error while evaluating flag {FlagKey}", new global::Microsoft.Extensions.Logging.LogDefineOptions() { SkipEnabledCheck = true }); 

        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "8.0.9.3103")]
        partial void FlagEvaluationError(global::System.String flagKey, global::System.Exception exception)
        {
            if (_logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Error))
            {
                __FlagEvaluationErrorCallback(_logger, flagKey, exception);
            }
        }
    }
}

[codecov]

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 94.20%. Comparing base (e654222) to head (a45f8cf).
Report is 1 commits behind head on main.

❗ Current head a45f8cf differs from pull request most recent head 160dbb9. Consider uploading reports for the commit 160dbb9 to get more accurate results

Files	Patch %	Lines
src/OpenFeature/Api.cs	0.00%	1 Missing ⚠️
src/OpenFeature/EventExecutor.cs	75.00%	1 Missing ⚠️
src/OpenFeature/OpenFeatureClient.cs	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
- Coverage   95.50%   94.20%   -1.31%     
==========================================
  Files          27       23       -4     
  Lines        1113      949     -164     
  Branches      120       99      -21     
==========================================
- Hits         1063      894     -169     
  Misses         33       33              
- Partials       17       22       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[benjiro]

I'm fine with pushing forward bumping to the latest of anything in Microsoft.Extensions they all have great TFM now. I find of late a lot of libraries are taking this approach.