-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Remove handwritten binding logic from Logging.Console; use generator instead #88067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Tagging subscribers to this area: @dotnet/area-extensions-logging Issue DetailsGenerated code (click to view)// <auto-generated/>
#nullable enable
/// <summary>Generated helper providing an AOT and linking compatible implementation for configuration binding.</summary>
internal static class GeneratedConfigurationBinder
{
/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
public static void Bind(this global::Microsoft.Extensions.Configuration.IConfiguration configuration, global::Microsoft.Extensions.Logging.Console.ConsoleFormatterOptions obj) => global::Microsoft.Extensions.Configuration.Binder.SourceGeneration.CoreBindingHelper.BindCore(configuration, ref obj, binderOptions: null);
/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
public static void Bind(this global::Microsoft.Extensions.Configuration.IConfiguration configuration, global::Microsoft.Extensions.Logging.Console.ConsoleLoggerOptions obj) => global::Microsoft.Extensions.Configuration.Binder.SourceGeneration.CoreBindingHelper.BindCore(configuration, ref obj, binderOptions: null);
/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
public static void Bind(this global::Microsoft.Extensions.Configuration.IConfiguration configuration, global::Microsoft.Extensions.Logging.Console.JsonConsoleFormatterOptions obj) => global::Microsoft.Extensions.Configuration.Binder.SourceGeneration.CoreBindingHelper.BindCore(configuration, ref obj, binderOptions: null);
/// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
public static void Bind(this global::Microsoft.Extensions.Configuration.IConfiguration configuration, global::Microsoft.Extensions.Logging.Console.SimpleConsoleFormatterOptions obj) => global::Microsoft.Extensions.Configuration.Binder.SourceGeneration.CoreBindingHelper.BindCore(configuration, ref obj, binderOptions: null);
}
namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Console;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Text.Encodings.Web;
using System.Text.Json;
/// <summary>Provide core binding logic.</summary>
internal static class CoreBindingHelper
{
public static void BindCore(IConfiguration configuration, ref ConsoleFormatterOptions obj, BinderOptions? binderOptions)
{
if (obj is null)
{
throw new ArgumentNullException(nameof(obj));
}
List<string>? temp = null;
foreach (IConfigurationSection section in configuration.GetChildren())
{
switch (section.Key)
{
case "IncludeScopes":
{
if (configuration["IncludeScopes"] is string stringValue0)
{
obj.IncludeScopes = ParseBool(stringValue0, () => section.Path)!;
}
}
break;
case "TimestampFormat":
{
obj.TimestampFormat = configuration["TimestampFormat"]!;
}
break;
case "UseUtcTimestamp":
{
if (configuration["UseUtcTimestamp"] is string stringValue2)
{
obj.UseUtcTimestamp = ParseBool(stringValue2, () => section.Path)!;
}
}
break;
default:
{
if (binderOptions?.ErrorOnUnknownConfiguration == true)
{
(temp ??= new List<string>()).Add($"'{section.Key}'");
}
}
break;
}
}
if (temp is not null)
{
throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {typeof(ConsoleFormatterOptions)}: {string.Join(", ", temp)}");
}
}
public static void BindCore(IConfiguration configuration, ref ConsoleLoggerOptions obj, BinderOptions? binderOptions)
{
if (obj is null)
{
throw new ArgumentNullException(nameof(obj));
}
List<string>? temp = null;
foreach (IConfigurationSection section in configuration.GetChildren())
{
switch (section.Key)
{
case "DisableColors":
{
if (configuration["DisableColors"] is string stringValue3)
{
obj.DisableColors = ParseBool(stringValue3, () => section.Path)!;
}
}
break;
case "Format":
{
if (configuration["Format"] is string stringValue4)
{
obj.Format = ParseConsoleLoggerFormat(stringValue4, () => section.Path)!;
}
}
break;
case "FormatterName":
{
obj.FormatterName = configuration["FormatterName"]!;
}
break;
case "IncludeScopes":
{
if (configuration["IncludeScopes"] is string stringValue6)
{
obj.IncludeScopes = ParseBool(stringValue6, () => section.Path)!;
}
}
break;
case "LogToStandardErrorThreshold":
{
if (configuration["LogToStandardErrorThreshold"] is string stringValue7)
{
obj.LogToStandardErrorThreshold = ParseLogLevel(stringValue7, () => section.Path)!;
}
}
break;
case "TimestampFormat":
{
obj.TimestampFormat = configuration["TimestampFormat"]!;
}
break;
case "UseUtcTimestamp":
{
if (configuration["UseUtcTimestamp"] is string stringValue9)
{
obj.UseUtcTimestamp = ParseBool(stringValue9, () => section.Path)!;
}
}
break;
case "QueueFullMode":
{
if (configuration["QueueFullMode"] is string stringValue10)
{
obj.QueueFullMode = ParseConsoleLoggerQueueFullMode(stringValue10, () => section.Path)!;
}
}
break;
case "MaxQueueLength":
{
if (configuration["MaxQueueLength"] is string stringValue11)
{
obj.MaxQueueLength = ParseInt(stringValue11, () => section.Path)!;
}
}
break;
default:
{
if (binderOptions?.ErrorOnUnknownConfiguration == true)
{
(temp ??= new List<string>()).Add($"'{section.Key}'");
}
}
break;
}
}
if (temp is not null)
{
throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {typeof(ConsoleLoggerOptions)}: {string.Join(", ", temp)}");
}
}
public static void BindCore(IConfiguration configuration, ref JsonWriterOptions obj, BinderOptions? binderOptions)
{
List<string>? temp = null;
foreach (IConfigurationSection section in configuration.GetChildren())
{
switch (section.Key)
{
case "Encoder":
{
throw new InvalidOperationException("Cannot create instance of type 'System.Text.Encodings.Web.JavaScriptEncoder' because it is missing a public instance constructor.");
}
case "Indented":
{
if (configuration["Indented"] is string stringValue12)
{
obj.Indented = ParseBool(stringValue12, () => section.Path)!;
}
}
break;
case "MaxDepth":
{
if (configuration["MaxDepth"] is string stringValue13)
{
obj.MaxDepth = ParseInt(stringValue13, () => section.Path)!;
}
}
break;
case "SkipValidation":
{
if (configuration["SkipValidation"] is string stringValue14)
{
obj.SkipValidation = ParseBool(stringValue14, () => section.Path)!;
}
}
break;
default:
{
if (binderOptions?.ErrorOnUnknownConfiguration == true)
{
(temp ??= new List<string>()).Add($"'{section.Key}'");
}
}
break;
}
}
if (temp is not null)
{
throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {typeof(JsonWriterOptions)}: {string.Join(", ", temp)}");
}
}
public static void BindCore(IConfiguration configuration, ref JsonConsoleFormatterOptions obj, BinderOptions? binderOptions)
{
if (obj is null)
{
throw new ArgumentNullException(nameof(obj));
}
List<string>? temp = null;
foreach (IConfigurationSection section in configuration.GetChildren())
{
switch (section.Key)
{
case "JsonWriterOptions":
{
if (HasChildren(section))
{
JsonWriterOptions temp15 = obj.JsonWriterOptions;
BindCore(section, ref temp15, binderOptions);
obj.JsonWriterOptions = temp15;
}
}
break;
case "IncludeScopes":
{
if (configuration["IncludeScopes"] is string stringValue16)
{
obj.IncludeScopes = ParseBool(stringValue16, () => section.Path)!;
}
}
break;
case "TimestampFormat":
{
obj.TimestampFormat = configuration["TimestampFormat"]!;
}
break;
case "UseUtcTimestamp":
{
if (configuration["UseUtcTimestamp"] is string stringValue18)
{
obj.UseUtcTimestamp = ParseBool(stringValue18, () => section.Path)!;
}
}
break;
default:
{
if (binderOptions?.ErrorOnUnknownConfiguration == true)
{
(temp ??= new List<string>()).Add($"'{section.Key}'");
}
}
break;
}
}
if (temp is not null)
{
throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {typeof(JsonConsoleFormatterOptions)}: {string.Join(", ", temp)}");
}
}
public static void BindCore(IConfiguration configuration, ref SimpleConsoleFormatterOptions obj, BinderOptions? binderOptions)
{
if (obj is null)
{
throw new ArgumentNullException(nameof(obj));
}
List<string>? temp = null;
foreach (IConfigurationSection section in configuration.GetChildren())
{
switch (section.Key)
{
case "ColorBehavior":
{
if (configuration["ColorBehavior"] is string stringValue19)
{
obj.ColorBehavior = ParseLoggerColorBehavior(stringValue19, () => section.Path)!;
}
}
break;
case "SingleLine":
{
if (configuration["SingleLine"] is string stringValue20)
{
obj.SingleLine = ParseBool(stringValue20, () => section.Path)!;
}
}
break;
case "IncludeScopes":
{
if (configuration["IncludeScopes"] is string stringValue21)
{
obj.IncludeScopes = ParseBool(stringValue21, () => section.Path)!;
}
}
break;
case "TimestampFormat":
{
obj.TimestampFormat = configuration["TimestampFormat"]!;
}
break;
case "UseUtcTimestamp":
{
if (configuration["UseUtcTimestamp"] is string stringValue23)
{
obj.UseUtcTimestamp = ParseBool(stringValue23, () => section.Path)!;
}
}
break;
default:
{
if (binderOptions?.ErrorOnUnknownConfiguration == true)
{
(temp ??= new List<string>()).Add($"'{section.Key}'");
}
}
break;
}
}
if (temp is not null)
{
throw new InvalidOperationException($"'ErrorOnUnknownConfiguration' was set on the provided BinderOptions, but the following properties were not found on the instance of {typeof(SimpleConsoleFormatterOptions)}: {string.Join(", ", temp)}");
}
}
public static bool HasChildren(IConfiguration configuration)
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
return true;
}
return false;
}
public static bool ParseBool(string stringValue, Func<string?> getPath)
{
try
{
return bool.Parse(stringValue);
}
catch (Exception exception)
{
throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(bool)}'.", exception);
}
}
public static ConsoleLoggerFormat ParseConsoleLoggerFormat(string stringValue, Func<string?> getPath)
{
try
{
return (ConsoleLoggerFormat)Enum.Parse(typeof(ConsoleLoggerFormat), stringValue, ignoreCase: true);
}
catch (Exception exception)
{
throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(ConsoleLoggerFormat)}'.", exception);
}
}
public static LogLevel ParseLogLevel(string stringValue, Func<string?> getPath)
{
try
{
return (LogLevel)Enum.Parse(typeof(LogLevel), stringValue, ignoreCase: true);
}
catch (Exception exception)
{
throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(LogLevel)}'.", exception);
}
}
public static ConsoleLoggerQueueFullMode ParseConsoleLoggerQueueFullMode(string stringValue, Func<string?> getPath)
{
try
{
return (ConsoleLoggerQueueFullMode)Enum.Parse(typeof(ConsoleLoggerQueueFullMode), stringValue, ignoreCase: true);
}
catch (Exception exception)
{
throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(ConsoleLoggerQueueFullMode)}'.", exception);
}
}
public static int ParseInt(string stringValue, Func<string?> getPath)
{
try
{
return int.Parse(stringValue, NumberStyles.Integer, CultureInfo.InvariantCulture);
}
catch (Exception exception)
{
throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(int)}'.", exception);
}
}
public static LoggerColorBehavior ParseLoggerColorBehavior(string stringValue, Func<string?> getPath)
{
try
{
return (LoggerColorBehavior)Enum.Parse(typeof(LoggerColorBehavior), stringValue, ignoreCase: true);
}
catch (Exception exception)
{
throw new InvalidOperationException($"Failed to convert configuration value at '{getPath()}' to type '{typeof(LoggerColorBehavior)}'.", exception);
}
}
}
}Details
|
src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatterOptions.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleFormatterOptions.cs
Outdated
Show resolved
Hide resolved
3ae119c to
e472927
Compare
|
The failing test is due to #88112. |
...braries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj
Outdated
Show resolved
Hide resolved
That was totally a mistake in the test to have different cases. 😊 But at least now we know we have decent test coverage. |
|
If you wanted to unblock this you could fix that test to have matching case, the have the fix for #88112 add test case(s). |
|
Taking Eric's (St. J :) suggestion - #88067 (comment). |
...nsions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs
Outdated
Show resolved
Hide resolved
90f598e to
23c67f8
Compare
Generated code following #88338 (click to view)```C# // #nullable enable #pragma warning disable CS0612, CS0618 // Suppress warnings about [Obsolete] member usage in generated code./// Generated helper providing an AOT and linking compatible implementation for configuration binding.[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Configuration.Binder.SourceGeneration", "42.42.42.42")] internal static class GeneratedConfigurationBinder { /// Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.public static void Bind(this global::Microsoft.Extensions.Configuration.IConfiguration configuration, global::Microsoft.Extensions.Logging.Console.ConsoleFormatterOptions obj) => global::Microsoft.Extensions.Configuration.Binder.SourceGeneration.CoreBindingHelper.BindCore(configuration, ref obj, binderOptions: null); } namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration } |
eerhardt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really good, @layomia.
...braries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj
Outdated
Show resolved
Hide resolved
...braries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj
Outdated
Show resolved
Hide resolved
...braries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerExtensions.Obsolete.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerOptions.cs
Outdated
Show resolved
Hide resolved
tarekgh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modulo @eerhardt comments, LGTM.
This reverts commit fa73b4edcd4a67ad195ab30a405e7f316ae34e5e.
23c67f8 to
63d7519
Compare
...braries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj
Outdated
Show resolved
Hide resolved
...braries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj
Outdated
Show resolved
Hide resolved
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
eerhardt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @layomia!
eiriktsarpalis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see all this bespoke code being removed 👍
|
Failing checks unrelated; check with running indicator appears to be complete -https://dev.azure.com/dnceng-public/public/_build/results?buildId=339049&view=logs&jobId=f02b8cf9-dd4d-54fc-c292-2bb1d305b019. |
Generated code (click to view)
Details