hab-sup --no-color doesn't strip color from all output

[bookshelfdave]

--no-color cli arg parsing was fixed in #996, but not all text is stripped of color.

[kbknapp]

Is this color from the argument parser, or elsewhere? Because if it's from the argument parser, there's a slightly hacky workaround that you can use. Keep in mind that I haven't dug through the sup code too much, so what I'm proposing may, or may not work for this use case.

What you can do is have your code that builds the CLI (but doesn't call get_matches()) abstracted away, then use clap's AppSettings::ColorNever if the --no-color flag is detected.

There are two ways to do this, option A is to check for the --no-color flag prior to clap doing the parsing, and adding appropriate settings based on those findings. Option B is checking for the --no-color flag after clap does the parsing and having the parsing done twice. Option B is easier to implement in place, but since the parsing is done twice its slower (although I severely doubt it's humanly noticeable).

Option A (preferred)

1. Check for the existence of the --no-color flag, prior to building the CLI
2. Build CLI like normal if that flag isn't present, add the call to setting(AppSettings::ColorNever) if its
3. Call get_matches() and continue like normal.

A very simplified version looks like this:

fn cli() -> App<'static, 'static> {
    let mut app = App::new("test")
        .arg(Arg::with_name("nocolor")
            .short("n")
            .long("no-color")
            .help("Disables color"));
    app = if env::args().any(|arg| arg == "--no-color") {
        app.setting(AppSettings::ColorNever)
    } else {
        app
    };
    app
}

fn main() {
    let matches = cli().get_matches();

    // All else like normal...
}

Option B

1. Build the CLI like normal
2. Check for the existence of the --no-color flag, after building the CLI
3. If that flag isn't present, add the call to setting(AppSettings::ColorNever)
4. Re-parse the args by callingl get_matches() and continue like normal.

fn cli() -> App<'static, 'static> {
    App::new("test")
        .arg(Arg::with_name("nocolor")
            .short("n")
            .long("no-color")
            .help("Disables color"))
}

fn main() {
    let res = cli().get_matches_safe();
    let matches = if env::args().any(|arg| arg == "--no-color") {
        cli().setting(AppSettings::ColorNever).get_matches()
    } else {
        res.unwrap_or_else(|e| e.exit())
    };

    // All else like normal...
}

Notice we actually leave the --no-color flag in place, for other areas that may want to know about that flag, as well as having it show in the help messages. But we don't actually use it. Instead we call env::args() to do a quick scan of the raw arguments before clap is able to parse them.

In Option B, we have to change the call for clap to parse them from get_matches() to get_matches_safe() which returns a Result<ArgMatches, clap::Error> instead of just blindly printing a (potentially colored) error message to stdout. But we also can't just call res.unwrap() because if clap detected an error in parsing, we actually do want the error message...just without color. So we call unwrap_or_else(|e| e.exit()) the e.exit() piece is defined by clap to do exactly that, print the nice error message and exit.

Hope this helps some!

[eeyun]

We believe we might have a workaround for this issue by @fnichol's pr here: #1682. However we'll try to verify and repro.

@fnichol and @metadave if either of you are aware of whether the above PR corrects the issue or gets us a step closer feel free to update the issue here.

[eeyun]

Another update: this is certainly not a workaround as it appears the above PR actually just implements the --no-color option as an env var

[smith]

It looks like this does not work at all in the supervisor:

You get the same result with HAB_NOCOLORING=1 set.

I'd say the correct behavior should be that the output from the supervisor (hab-sup(MR)), etc. and the output from the program being supervised (USA) should both be non-colored if either of these options is set.

[rhass]

While not a fix, I have identified a workaround for syslog messages so that the message output is preserved and control characters are not mutated. My workaround however is probably platform specific, and assumes the system syslog daemon is rsyslog.

/etc/rsyslog.d/10-habitat:

# EscapeControlCharactersOnReceive when enabled strips color control characters
# and replaces the control code with a `#`. Since there is a bug in habitat
# which prevents us from disabling color output from the supervisor, we want
# to preserve the control characters and keep the color output.
$EscapeControlCharactersOnReceive off
# This matches the SyslogIdentifier key name which is emitted from the systemd
# journal.
:programname, isequal, "hab" /var/log/hab.log
# The funny line below tells rsyslog not to push the matched log messages
# to any other log file such as the syslog.
& ~

The rsyslog config assumes your init method prepends the log lines with the application identifier of hab. If you are using systemd, this is configurable:
/etc/systemd/system/hab.service

Description=Habitat Supervisor

[Service]
# SyslogIdentifier is used to match the log output in rsyslog so
# we can push the logs to a specific file rather than syslog and
# preserve the colorize control characters.
SyslogIdentifier=hab
ExecStart=/bin/hab sup run

[Install]
WantedBy=default.target

You will probably want to make a logrotate file if you use this:
/etc/logrotate.d/hab:

/var/log/hab.log {
  rotate 7
  daily
  copytruncate
  compress
  notifempty
  missingok
}

[jamessewell]

OK I've been doing a bit hacking round with this for an internal usecase we have.

I've got the colour turned off for the most part - but the problem comes when people paint colours outside of the core/output.rs. This means that the control codes get pushed into the strings and it's a tough time.

Three options I can see:

• Write something which strips colour from strings and use that in output.rs. This would strip colour which apps put into their logs though - which could be a bad idea?
• Start passing ANSIString round, which means that they can be de-referenced into strings if needed. Potentially the same issue as above.
• Just do all colouring in core/output.rs based on the information in log keys

[jamessewell]

At the risk of discussing this with myself, I think option 1 is best.

I also implemented JSON output and even if the service creates coloured logs you don't really want that in JSON.

I'll write a regex function which strips the controls and then all the paint option can be left as is im the code.

Unless someone else has some thoughts?

[bixu]

This also seems to be an issue for a systemd-managed Supervisor when we set HAB_NOCOLOR=true as an environment variable in our unit file. That, in turn, is a massive PITA when shipping logs to our ELK service (logz.io).

It really seems to me that colorless output should be the default behavior, with colored output being available via some environment variable that a user can set in their local dev environment.

cc: @mspringfeldt