Fix default log color behavior when console is redirected

src/libraries/Common/src/System/Console/ConsoleUtils.cs

@@ -0,0 +1,53 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+
+namespace System
+{
+    internal static partial class ConsoleUtils
+    {
+        /// <summary>Whether to output ansi color strings.</summary>
+        private static volatile int s_emitAnsiColorCodes = -1;
+
+        /// <summary>Get whether to emit ANSI color codes.</summary>
+        public static bool EmitAnsiColorCodes
+        {
+            get
+            {
+                // The flag starts at -1.  If it's no longer -1, it's 0 or 1 to represent false or true.
+                int emitAnsiColorCodes = s_emitAnsiColorCodes;
+                if (emitAnsiColorCodes != -1)
+                {
+                    return Convert.ToBoolean(emitAnsiColorCodes);
+                }
+
+                // We've not yet computed whether to emit codes or not.  Do so now.  We may race with
+                // other threads, and that's ok; this is idempotent unless someone is currently changing
+                // the value of the relevant environment variables, in which case behavior here is undefined.
+
+                // By default, we emit ANSI color codes if output isn't redirected, and suppress them if output is redirected.
+                bool enabled = !Console.IsOutputRedirected;
+
+                if (enabled)
+                {
+                    // We subscribe to the informal standard from https://no-color.org/.  If we'd otherwise emit
+                    // ANSI color codes but the NO_COLOR environment variable is set, disable emitting them.
+                    enabled = Environment.GetEnvironmentVariable("NO_COLOR") is null;
+                }
+                else
+                {
+                    // We also support overriding in the other direction.  If we'd otherwise avoid emitting color
+                    // codes but the DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION environment variable is
+                    // set to 1 or true, enable color.
+                    string? envVar = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION");
+                    enabled = envVar is not null && (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase));
+                }
+
+                // Store and return the computed answer.
+                s_emitAnsiColorCodes = Convert.ToInt32(enabled);
+                return enabled;
+            }
+        }
+    }
+}

src/libraries/Microsoft.Extensions.Logging.Console/src/ConsoleLoggerProvider.cs

@@ -59,6 +59,7 @@ public ConsoleLoggerProvider(IOptionsMonitor<ConsoleLoggerOptions> options, IEnu
                 console = new AnsiParsingLogConsole();
                 errorConsole = new AnsiParsingLogConsole(stdErr: true);
             }
+
             _messageQueue = new ConsoleLoggerProcessor(console, errorConsole);
         }
 
@@ -162,7 +163,7 @@ private static void UpdateFormatterOptions(ConsoleFormatter formatter, ConsoleLo
             {
                 defaultFormatter.FormatterOptions = new SimpleConsoleFormatterOptions()
                 {
-                    ColorBehavior = deprecatedFromOptions.DisableColors ? LoggerColorBehavior.Disabled : LoggerColorBehavior.Enabled,
+                    ColorBehavior = deprecatedFromOptions.DisableColors ? LoggerColorBehavior.Disabled : LoggerColorBehavior.Default,
                     IncludeScopes = deprecatedFromOptions.IncludeScopes,
                     TimestampFormat = deprecatedFromOptions.TimestampFormat,
                     UseUtcTimestamp = deprecatedFromOptions.UseUtcTimestamp,

src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj

@@ -28,6 +28,8 @@
              Link="Common\Interop\Windows\Interop.GetConsoleMode.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetStdHandle.cs"
              Link="Common\Interop\Windows\Interop.GetStdHandle.cs" />
+    <Compile Include="$(CommonPath)System\Console\ConsoleUtils.cs"
+             Link="Common\System\Console\ConsoleUtils.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">

src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatter.cs

@@ -158,7 +158,7 @@ private static string GetLogLevelString(LogLevel logLevel)
         private ConsoleColors GetLogLevelConsoleColors(LogLevel logLevel)
         {
             bool disableColors = (FormatterOptions.ColorBehavior == LoggerColorBehavior.Disabled) ||
-                (FormatterOptions.ColorBehavior == LoggerColorBehavior.Default && System.Console.IsOutputRedirected);
+                (FormatterOptions.ColorBehavior == LoggerColorBehavior.Default && !System.ConsoleUtils.EmitAnsiColorCodes);
             if (disableColors)
             {
                 return new ConsoleColors(null, null);

src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs

@@ -4,8 +4,10 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Text.RegularExpressions;
+using Microsoft.DotNet.RemoteExecutor;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging.Test.Console;
@@ -413,6 +415,36 @@ public void WriteInformation_LogsCorrectColors()
             Assert.Equal(TestConsole.DefaultForegroundColor, write.ForegroundColor);
         }
 
+        [ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))]
+        public void AddConsole_IsOutputRedirected_ColorDisabled()
+        {
+            RemoteExecutor.Invoke(() =>
+            {
+                // Arrange
+                var sink = new ConsoleSink();
+                var console = new TestConsole(sink);
+                var loggerOptions = new ConsoleLoggerOptions();
+                var consoleLoggerProcessor = new TestLoggerProcessor(console, null!);
+
+                var loggerProvider = new ServiceCollection()
+                    .AddLogging(builder => builder
+                        .AddConsole()
+                    )
+                    .BuildServiceProvider()
+                    .GetRequiredService<ILoggerProvider>();
+
+                var consoleLoggerProvider = Assert.IsType<ConsoleLoggerProvider>(loggerProvider);
+                var logger = (ConsoleLogger)consoleLoggerProvider.CreateLogger("Category");
+
+                // Assert
+                Assert.True(System.Console.IsOutputRedirected);
+                Assert.Null(logger.Options.FormatterName);
+                var formatter = Assert.IsType<SimpleConsoleFormatter>(logger.Formatter);
+                Assert.Equal(LoggerColorBehavior.Default, formatter.FormatterOptions.ColorBehavior);
+
+            }, new RemoteInvokeOptions { StartInfo = new ProcessStartInfo() { RedirectStandardOutput = true } }).Dispose();
+        }
+
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
         public void WriteDebug_LogsCorrectColors()
         {
@@ -1365,6 +1397,9 @@ private string CreateHeader(ConsoleLoggerFormat format, int eventId = 0)
                     throw new ArgumentOutOfRangeException(nameof(format));
             }
         }
+
+        public static bool IsThreadingAndRemoteExecutorSupported =>
+            PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported;
     }
 
     internal class TestLoggerProcessor : ConsoleLoggerProcessor

src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj

@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
+    <IncludeRemoteExecutor>true</IncludeRemoteExecutor>
     <TargetFrameworks>$(NetCoreAppCurrent);$(NetFrameworkMinimum)</TargetFrameworks>
     <TestRuntime>true</TestRuntime>
     <EnableDefaultItems>true</EnableDefaultItems>

src/libraries/System.Console/src/System.Console.csproj

@@ -156,6 +156,8 @@
   <!-- Unix -->
   <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'Unix'">
     <Compile Include="System\ConsolePal.Unix.cs" />
+    <Compile Include="$(CommonPath)System\Console\ConsoleUtils.cs"
+             Link="Common\System\Console\ConsoleUtils.cs" />
     <Compile Include="System\TermInfo.cs" />
     <Compile Include="System\IO\StdInReader.cs" />
     <Compile Include="System\IO\SyncTextReader.Unix.cs" />

src/libraries/System.Console/src/System/ConsolePal.Unix.cs

@@ -39,9 +39,6 @@ internal static class ConsolePal
         private static int s_windowHeight;  // Cached WindowHeight, invalid when s_windowWidth == -1.
         private static int s_invalidateCachedSettings = 1; // Tracks whether we should invalidate the cached settings.
 
-        /// <summary>Whether to output ansi color strings.</summary>
-        private static volatile int s_emitAnsiColorCodes = -1;
-
         public static Stream OpenStandardInput()
         {
             return new UnixConsoleStream(Interop.CheckIo(Interop.Sys.Dup(Interop.Sys.FileDescriptors.STDIN_FILENO)), FileAccess.Read,
@@ -770,7 +767,7 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
             // Changing the color involves writing an ANSI character sequence out to the output stream.
             // We only want to do this if we know that sequence will be interpreted by the output.
             // rather than simply displayed visibly.
-            if (!EmitAnsiColorCodes)
+            if (!ConsoleUtils.EmitAnsiColorCodes)
             {
                 return;
             }
@@ -832,52 +829,12 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
         /// <summary>Writes out the ANSI string to reset colors.</summary>
         private static void WriteResetColorString()
         {
-            if (EmitAnsiColorCodes)
+            if (ConsoleUtils.EmitAnsiColorCodes)
             {
                 WriteStdoutAnsiString(TerminalFormatStrings.Instance.Reset);
             }
         }
 
-        /// <summary>Get whether to emit ANSI color codes.</summary>
-        private static bool EmitAnsiColorCodes
-        {
-            get
-            {
-                // The flag starts at -1.  If it's no longer -1, it's 0 or 1 to represent false or true.
-                int emitAnsiColorCodes = s_emitAnsiColorCodes;
-                if (emitAnsiColorCodes != -1)
-                {
-                    return Convert.ToBoolean(emitAnsiColorCodes);
-                }
-
-                // We've not yet computed whether to emit codes or not.  Do so now.  We may race with
-                // other threads, and that's ok; this is idempotent unless someone is currently changing
-                // the value of the relevant environment variables, in which case behavior here is undefined.
-
-                // By default, we emit ANSI color codes if output isn't redirected, and suppress them if output is redirected.
-                bool enabled = !Console.IsOutputRedirected;
-
-                if (enabled)
-                {
-                    // We subscribe to the informal standard from https://no-color.org/.  If we'd otherwise emit
-                    // ANSI color codes but the NO_COLOR environment variable is set, disable emitting them.
-                    enabled = Environment.GetEnvironmentVariable("NO_COLOR") is null;
-                }
-                else
-                {
-                    // We also support overriding in the other direction.  If we'd otherwise avoid emitting color
-                    // codes but the DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION environment variable is
-                    // set to 1 or true, enable color.
-                    string? envVar = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION");
-                    enabled = envVar is not null && (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase));
-                }
-
-                // Store and return the computed answer.
-                s_emitAnsiColorCodes = Convert.ToInt32(enabled);
-                return enabled;
-            }
-        }
-
         /// <summary>Cache of the format strings for foreground/background and ConsoleColor.</summary>
         private static readonly string[,] s_fgbgAndColorStrings = new string[2, 16]; // 2 == fg vs bg, 16 == ConsoleColor values