serilog-expressions
serilog-expressions copied to clipboard
Add support for NO_COLOR env var
Hey!
There is a slight inconsistency between different serilog console logging methods when it comes to support for NO_COLOR env variable.
See https://no-color.org/ for the reasoning.
Also there has been a PR last year to add the support for console sink here: https://github.com/serilog/serilog-sinks-console/pull/154/files
However it doesn't work when using ExpressionTemplate because it has its own theming implementation.
I think it makes sense to add this support here as well.
I am willing to make the changes and prepare a PR but wanted to discuss it first to make sure it won't get rejected for some reason.
Thanks for checking in first, deeply appreciated 👍 👍
I think if the extent of the change was:
- Modify
SelectThemeto consider theNO_COLORvariable, - Leave
applyThemeWhenOutputIsRedirectedas an override (ignoringNO_COLORif theapplyThemeWhen..is `true), and - Update the XDOC for
applyThemeWhen..to mention also opting-out ofNO_COLOR(theapplyThemeWhen..variable can't be renamed without breaking existing configs, so we'd have to live with the minor naming discrepancy)
this would be a welcome change. (The applyThemeWhen.. variable can't be renamed without breaking existing configs, so we'd have to live with the minor naming discrepancy.) Let me know if you had anything different in mind. Cheers!
Hey, thanks for your reply. I had something slightly different in mind to be honest.
We're using Serilog console sink in development (local env) settings which needs applyThemeWhenOutputIsRedirected to be true, because it's managed by tilt which uses stdout redirection in order to have colors there.
Also running any app locally should output colors. Since we have multiple apps and services we have a common NuGet for logging.
This common NuGet always sets applyThemeWhenOutputIsRedirected = true because otherwise we don't get to see the colors in tilt.
However in CI (GH Actions) these colors don't really look good so wanted to turn them off. NO_COLOR override was the easiest idea that came to my mind.
Now I'm having second thoughts though. Looks like I was wrong and NO_COLOR should not be the final override but rather a hint which can be overridden by app configuration. Given that it doesn't solve my problem at all ;)
I think there is no other way rather than exposing applyThemeWhenOutputIsRedirected outside our NuGet one way or another.