Log Colors

[egelja]

I just want to preface this by saying, awesome library! This is really cool.

Would it be possible to add log colors? So based on the level (info, trace, etc.) a different ANSI escape color code would be inserted at the start of the log line, and then a "reset" code would be printed at the end.

[erenon]

Hi, unfortunately, it is difficult to come up with a universally pleasing color scheme, and I don't want to complicate matters with that. However, you can easily add colors, by piping the output through sed, e.g:

$ bread log.blog | sed "s/^CRIT/<reset escape seq><red escape seq>CRIT/" ...etc for every severity

[egelja]

Ok, thank you, that makes sense.

However, just want to make a suggestion. The color scheme could be configurable, like this:

inline std::string_view severityToColor(Severity severity)
{
  switch (severity)
  {
    case Severity::trace:    return BINLOG_COLOR_TRACE;
    case Severity::debug:    return BINLOG_COLOR_DEBUG;
    case Severity::info:     return BINLOG_COLOR_INFO;
    case Severity::warning:  return BINLOG_COLOR_WARN;
    case Severity::error:    return BINLOG_COLOR_ERROR;
    case Severity::critical: return BINLOG_COLOR_CRITICAL;
    case Severity::no_logs:  return BINLOG_COLOR_NOLOG;
    default:                 return BINLOG_COLOR_UNKNOWN;
  }
}

And could be enabled like this:

// PrettyPrinter.cpp
void PrettyPrinter::printEvent(
  std::ostream& ostr,
  const Event& event,
  const WriterProp& writerProp,
  const ClockSync& clockSync
)
{
  detail::OstreamBuffer out(ostr);
  _clockSync = &clockSync;

#ifdef BINLOG_ENABLE_COLORS
  out << severityToColor(event.source->severity);
#endif

  for (std::size_t i = 0; i < _eventFormat.size(); ++i)
  {
    const char c = _eventFormat[i];
    if (c == '%' && ++i != _eventFormat.size())
    {
      const char spec = _eventFormat[i];
      printEventField(out, spec, event, writerProp);
    }
    else
    {
      out.put(c);
    }
  }

#ifdef BINLOG_ENABLE_COLORS
  // Reset colors
  out << "\e[0m";
#endif
}

And then configured like this

option(BINLOG_ENABLE_COLORS OFF)

# These could be given default values
option(BINLOG_COLOR_TRACE "")
option(BINLOG_COLOR_DEBUG "")
option(BINLOG_COLOR_INFO "")
option(BINLOG_COLOR_WARN "")
option(BINLOG_COLOR_ERROR "")
option(BINLOG_COLOR_CRITICAL "")
option(BINLOG_COLOR_NOLOG "")
option(BINLOG_COLOR_UNKNOWN "")

if(BINLOG_ENABLE_COLORS)
  target_compile_definitions(binlog PRIVATE
    BINLOG_ENABLE_COLORS
    BINLOG_COLOR_TRACE=${BINLOG_COLOR_TRACE}
    # etc.
  )
endif()

This is not a large amount of code, and its completely configurable, so it shouldn't be a burden on the library. It would even be off by defult.

[erenon]

What if the user sets a format string (-f) that doesn't start with the severity (%S)? What if the user uses different terminal color schemes based on the available natural light, therefore different colors are preferred at different times? What if different users use different terminal color schemes, but the same installed bread binary? (Think of a shared drive with installed programs) What if the output should not contain escape codes, if the output is not a tty?

I think a preferred place to add this functionality would be around here: https://github.com/morganstanley/binlog/blob/main/bin/printers.cpp#L24

  while (const binlog::Event* event = eventStream.nextEvent(entryStream))
  {
    // here
    pp.printEvent(output, *event, eventStream.writerProp(), eventStream.clockSync());
  }

And it should take effect only if -c (color) is specified on the command line. However, I'm not convinced that the complexity does worth it. The point of using a fast structured log is to be able to process huge amounts of logs programmatically, without eyeballing.

Otherwise I do understand that for smaller cases, the output would look nicer - so thanks for the suggestion!

[egelja]

Ok, I understand your concern. Thank you for considering this feedback.

Would you like me to close the issue?

[erenon]

Yes, please close it.