Enable AOT compatibility for all libraries

[eerhardt]

Port #4625 to main now that the Configuration.Binder bugs are fixed in 8.0.1.

Enable AOT compatibility for all libraries and fix AOT warnings.

• Enable configuration binder source generator
• The only library that can't use the ConfigBinder SG is the HeaderParsing library.
  • Blocked by ConfigurationBinder source generator generates code that doesn't compile when using StringValues runtime#94547

Explicitly reference Microsoft.Extensions.Configuration.Binder in projects that use the source generator to ensure we get the latest version (8.0.1).

Microsoft Reviewers: Open in CodeFlow

[joperezr]

NIT: Does it hurt to set this property to true when a project is not referencing the configuration binder package? If not, then we might want to consider defaulting this to true in directory.build.props to avoid having to add it in most places. And to that same line, should we consider adding the generator by default to all projects too? I'm assuming libraries not using configuration will simply no-op for this generator (which may include a slight perf regresion in build time) but this may be beneficial for new libraries in the future.

[eerhardt]

(Totally my opinion, I'm curious to hear others')

Since this source generator is off by default, I thought it would be better to continue to keep it off by default. It isn't really "most places". It is 12 libraries. By comparison, UseLoggingGenerator is used in 13 libraries, and it isn't defaulted to be "on by default".

[joperezr]

Left minor comments, but looks good overall. Thanks Eric!

[joperezr]

🎉 Good job! The coverage increased 🎉 Update MinCodeCoverage in the project files.

Project Expected Actual
Microsoft.Extensions.Http.Diagnostics 91 94
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=520582&view=codecoverage-tab

This is being addressed via #4870

[dotnet-comment-bot]

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.AspNetCore.Diagnostics.Middleware	Line	100	99.17 🔻

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=523076&view=codecoverage-tab

[joperezr]

‼️ Found issues ‼️

Project Coverage Type Expected Actual
Microsoft.AspNetCore.Diagnostics.Middleware Line 100 99.17 🔻
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=523076&view=codecoverage-tab

hrm... Looking at the reprot it is complaining about this line:

extensions/src/Libraries/Microsoft.Extensions.Diagnostics.Testing/Metrics/CollectedMeasurement.cs

Line 109 in 7db3a7e

internal string DebuggerToString() => $"{Value} @ {Timestamp.ToString("HH:mm:ss.ffff", CultureInfo.InvariantCulture)}";

Not sure why the changes in this PR are triggering it, but sounds like we should be able to just exclude from code coverage given that it looks like that is only for debugging purposes?

[eerhardt]

hrm... Looking at the reprot it is complaining about this line:

I dug deeper and it is complaining about these 3 setters:

extensions/src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Logging/LoggingRedactionOptions.cs

Lines 58 to 82 in 7db3a7e

	#pragma warning disable CA2227 // Collection properties should be read only
	public IDictionary<string, DataClassification> RouteParameterDataClasses { get; set; } = new Dictionary<string, DataClassification>(StringComparer.OrdinalIgnoreCase);
	#pragma warning restore CA2227 // Collection properties should be read only
	/// <summary>
	/// Gets or sets a map between request headers to be logged and their data classification.
	/// </summary>
	/// <value>
	/// The default value is an empty dictionary, which means that no request header is logged by default.
	/// </value>
	[Required]
	#pragma warning disable CA2227 // Collection properties should be read only
	public IDictionary<string, DataClassification> RequestHeadersDataClasses { get; set; } = new Dictionary<string, DataClassification>(StringComparer.OrdinalIgnoreCase);
	#pragma warning restore CA2227 // Collection properties should be read only
	/// <summary>
	/// Gets or sets a map between response headers to be logged and their data classification.
	/// </summary>
	/// <value>
	/// The default value is an empty dictionary, which means that no response header is logged by default.
	/// </value>
	[Required]
	#pragma warning disable CA2227 // Collection properties should be read only
	public IDictionary<string, DataClassification> ResponseHeadersDataClasses { get; set; } = new Dictionary<string, DataClassification>(StringComparer.OrdinalIgnoreCase);
	#pragma warning restore CA2227 // Collection properties should be read only

However, when I look back at other CI runs and dig into their merged.cobertura.xml files, I'm seeing some weirdness.

I downloaded the merged xml file from https://dev.azure.com/dnceng-public/public/_build/results?buildId=522132, and that one has 100% for the class/package.

But then I download it from https://dev.azure.com/dnceng-public/public/_build/results?buildId=524014, and that one has
<package line-rate="0.9916666666666667" branch-rate="1" complexity="168" name="Microsoft.AspNetCore.Diagnostics.Middleware">, but the code coverage task passed in CI. So I'm at a loss at what is going wrong here, or why my change would cause this.

I verified that the source generator sets these properties, so I have no idea why this change is causing this to pop up now.

I looked at the buildId=524014 was against the dev branch, which already has this change. So now I believe it is the source generator change that isn't causing these setters to be hit.

[eerhardt]

I've traced down the code coverage issue to dotnet/runtime#96873.

It looks like the only test we have in this area is

extensions/test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Logging/HttpLoggingServiceExtensionsTests.cs

Lines 31 to 40 in 7db3a7e

	public void AddHttpLogging_WhenConfiguredUsingConfigurationSection_IsCorrect()
	{
	var services = new ServiceCollection();
	var builder = new ConfigurationBuilder().AddInMemoryCollection(new[]
	{
	new KeyValuePair<string, string?>("HttpLogging:RequestPathLoggingMode", "Structured"),
	new KeyValuePair<string, string?>("HttpLogging:RequestPathParameterRedactionMode","None"),
	new KeyValuePair<string, string?>("HttpLogging:ExcludePathStartsWith:[0]","/path0toexclude"),
	new KeyValuePair<string, string?>("HttpLogging:ExcludePathStartsWith:[1]","/path1toexclude"),
	});

Which isn't setting these properties. I'll add to this test to ensure the properties get set so we can keep code coverage at 100%.