No localization in Logging/LogValuesFormatter (src/Logging/Logging.Analyzers/src/LogValuesFormatter.cs)

[Popsikill]

If you believe you have an issue that affects the security of the platform please do NOT create an issue and instead email your issue details to secure@microsoft.com. Your report may be eligible for our bug bounty but ONLY if it is reported through email.

Describe the bug

When logging (via an implemented ILogger such as Console) the date/numeric formats are not localized. It appears currently there is no automatic localization of parameters in the LogValuesFormatter class - it is set to use the Invariant Culture. As the Invariant Culture leans heavily toward American formatting - for the rest of the world this doesn't really fit. Ideally we should be able to DI to specify what culture should be use - with Invariant Culture being the backup.

To Reproduce

Steps to reproduce the behavior:

1. Microsoft.Extensions.Logging.Abstractions, Version 3.1.6.0
2. Create a basic .net core project
3. Configure to use ConsoleLogger
4. Inject and use ILogger with a simple DateTime
5. Message reads as Invariant Culture date format Action to begin at 07/29/2020 12:52:12

Expected behavior

Expected to be able to override culture by either;

• Setting CultureInfo.CurrentCulture (or one of it's variants),
• Setting Thread.CurrentThread.CurrentCulture (or one of it's variants),
• A localization package similar to ASP.NET Core's UseRequestLocalization that would be configurable for the use-cases.

Screenshots

N/A

Additional context

.NET Core SDK (reflecting any global.json): Version: 3.1.302 Commit: 41faccf259

Runtime Environment: OS Name: Windows OS Version: 10.0.19041 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.302\

Host (useful for support): Version: 3.1.6 Commit: 3acd9b0cd1

.NET Core SDKs installed: 3.1.202 [C:\Program Files\dotnet\sdk] 3.1.301 [C:\Program Files\dotnet\sdk] 3.1.302 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @maryamariyan See info in area-owners.md if you want to be subscribed.

[KalleOlaviNiemitalo]

ASP.NET Core RequestLocalizationMiddleware sets CultureInfo.CurrentCulture and CultureInfo.CurrentUICulture but I don't think the HTTP requests should affect how the log file is localized.

Possible API and implementation overview

I haven't wanted such a feature myself, so I'm not filing a formal API proposal.

 namespace Microsoft.Extensions.Logging
 {
     public partial class LoggingBuilderExtensions
     {
+        // To configure localization, the application will call this existing method.
         public static ILoggingBuilder Configure(this ILoggingBuilder builder, Action<LoggerFactoryOptions> action);
     }

     public class LoggerFactoryOptions
     {
         public LoggerFactoryOptions();
         public ActivityTrackingOptions ActivityTrackingOptions { get; set; }

+        // CultureInfo rather than IFormatProvider, so it can be
+        // used as CultureInfo.CurrentCulture during logging.
+        // Defaults to CultureInfo.InvariantCulture.
+        public CultureInfo Culture { get; set; }
     }
 }

1. The LoggerFactory constructor would copy the value of the Culture property to a new field.
2. The LoggerFactory.CreateLogger method would propagate the culture setting to a new property on Logger.
3. The Logger.Log method would set CultureInfo.CurrentCulture (but not CultureInfo.CurrentUICulture) before calling the first inner ILogger, and restore it after calling the last inner ILogger.
4. The Format methods of LogValuesFormatter would use the current culture rather than CultureInfo.InvariantCulture. (Strangely, the FormatArgument method is already using the current culture in its string.Join call.)

Risks

Having Logger.Log temporarily change CultureInfo.CurrentCulture could cost performance.

If an application calls ILoggerProvider.CreateLogger directly, then it does not get a Logger instance that would change CultureInfo.CurrentCulture. LogValuesFormatter might then start using non-invariant cultures even though the application did not opt in to that.

If a library has defined its own TState type (e.g. if it wants more parameters than LoggerMessage.Define supports, https://github.com/dotnet/runtime/issues/35060), then that type is presumably using the invariant culture now, and would not automatically support this feature if an application using the library requests it. That can't be helped.

Alternatives

Could pass the localization setting to TState (e.g. FormattedLogValues) and thence to LogValuesFormatter as a parameter, instead of temporarily changing CultureInfo.CurrentCulture. That might be faster and would allow using IFormatProvider instead of CultureInfo. However, it would require changing each ILoggerProvider implementation to save the localization setting, check whether TState supports such a parameter, and then pass it in (perhaps via IFormattable.ToString).

Out of scope

CultureInfo.CurrentUICulture, because that should involve changing LoggerMessage to support localization of format strings.

[RiaanVR]

Hi, I just ran into this and it is specific to a few logs that deal with numerical values, part of that includes making sure the logs make sense to the users in various locations. Being able to specify a culture upfront would help greatly, so what's the plan for this?

[KalleOlaviNiemitalo]

Anyone working on this should consider whether the logging source generator (https://github.com/dotnet/runtime/issues/36022, https://github.com/dotnet/runtime/pull/51064) also needs to be changed.

[davidfowl]

Being able to specify a culture upfront would help greatly, so what's the plan for this?

There's no plan currently for this. We'll evaluate in .NET 7.