-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
ConsoleLogger output is garbled with color codes when redirected #37421
Comments
Tagging subscribers to this area: @eiriktsarpalis |
(FYI, there's a semi-related discussion at #33980, which I still need to follow-up on.) |
I've changed the are from |
/cc: @shirhatti |
This would not occur on a windows OS machine currently but with the new changes coming in for #34742 we would basically embed ANSI codes even on windows machines for the Default consoles which uses DisableColors. |
@maryamariyan can you look into this issue as part of the |
For sure. The PR would also fix this. UPDATE: #38616 (comment) |
Is this fixed now that ConsoleLogger changes are in? |
@eerhardt recommended earlier in my linked PR that we could choose to strip out the color codes (by using AnsiParsingLogConsole) when the output is redirected and could be solution for fixing the current issue. But I have postponed this fix for now because of the following two talking points:
cc: @tarekgh |
It's very much possible for the software "reading" the redirected output to parse and display colours.
Please consider this:
To me, it seems like the safer option is to remove colours for now. Producing colours is a nice to have, but producing garbage makes us look bad. When we have a better mechanism to detect and enable colours, we should do that. |
Would a reasonable fix here be:
We can make an API change here because we just introduced this API in 5.0, so we still have time to change it. Thoughts? |
Nullable bool named with a negative is a little confusing for an API. Maybe an enum would be clearer (or at least another option to consider). |
As long as we can set enums through configuration that would be fine with me.
How about just |
I think I remember in an API review there was a guideline to avoid using Default for enums. First bulletpoint in #34305 (comment), or maybe I misunderstood. I think that was another reason we switched from using Default formatter to simple too.
enums should also be possible to be set in configuration. ConsoleLoggerFormat enum (now deprecated) could be set in configuration before. |
namespace Microsoft.Extensions.Logging.Console
{
+ /// <summary>
+ /// Flags to help determine whether or not logs should contain color.
+ /// </summary>
+ public enum LoggerColorBehavior
+ {
+ /// <summary>
+ /// Disabled when output is redirected
+ /// </summary>
+ Default = 0,
+
+ /// <summary>
+ /// Enable color for logging
+ /// </summary>
+ Enabled = 1,
+
+ /// <summary>
+ /// Disable color for logging.
+ /// </summary>
+ Disabled = 2,
+ }
/// <summary>
/// Options for the built-in default console log formatter.
/// </summary>
public class SimpleConsoleFormatterOptions : ConsoleFormatterOptions
{
public SimpleConsoleFormatterOptions() { }
- /// <summary>
- /// Disables colors when <see langword="true" />.
+ /// Determines whether or not to provide color for console logs />.
- /// </summary>
- public bool DisableColors { get; set; }
+ public LoggerColorBehavior ColorBehavior { get; set; }
/// <summary>
/// When <see langword="false" />, the entire message gets logged in a single line.
/// </summary>
public bool SingleLine { get; set; }
}
} Motivation:As showcased in the description of this issue, the existing mechanism we have in place does not allow to properly decide what to do for embedding colors when output is getting redirected elsewhere. In cases where the redirected output does not support ANSI color codes, the output gets polluted with unwanted string produced by the logger. Usage:With:
Then in the console formatter checks for Instead, |
|
@eerhardt updated. Changed |
cc: @davidfowl |
namespace Microsoft.Extensions.Logging.Console
{
+ public enum LoggerColorBehavior
+ {
+ Default = 0,
+ Enabled = 1,
+ Disabled = 2,
+ }
public class SimpleConsoleFormatterOptions : ConsoleFormatterOptions
{
public SimpleConsoleFormatterOptions();
- public bool DisableColors { get; set; }
+ public LoggerColorBehavior ColorBehavior { get; set; }
public bool SingleLine { get; set; }
}
} |
On Kubernetes, logs from ASP.NET Core apps are garbled with color codes:
.NET
System.Console
will not output colors when it is not connected to a terminal. This is the case on Kubernetes, whereConsole.IsOutputRedirected
returnstrue
.ConsoleLogger
doesn't considerIsOutputRedirected
and outputs colors regardless.Behaving like
Console
would produce cleaner logs when running ASP.NET Core applications on Kubernetes (and other redirected environments).I read through dotnet/extensions#673. There is a
ConsoleLogger.DisableColors
. I'm not sure why it does not default totrue
whenConsole.IsOutputRedirected
istrue
?cc @rynowak @glennc
The text was updated successfully, but these errors were encountered: