gazebosim / gazebosim/sdformat
Explicitly delimit EOM for logging
- Dominant language
- C++
- Stars
- 216
- Forks
- 125
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 14
Description
## Desired behavior
The Console (i.e., logging) abstraction in libsdformat currently follows the semantics of something like `std::cerr` -- a simple text stream where the call sites conclude each call with a newline (`sdferr << ... << "\n";`). Even if we switch to use the ignition-common console per #351, it still currently embodies the same abstraction.
However, in a lot of production code, log message telemetry is not just a dedicated console read by human eyes -- it is recorded in a structured form, sent over the network, and/or otherwise treated on a _message_ basis rather a _stream_ basis.
My request here is to change the Console to use the same semantics -- where there is a clear "end of message" demarcation.
Several C++ logging frameworks (e.g., log4cpp) do this by using the _logger lifetime_ to conclude the message. My patch file in https://github.com/RobotLocomotion/drake/pull/16348 shows an example of this. The `Console::ColorMsg` function returns a short-lived stream by-value, instead of a long-lived stream by-reference. When the statement ends, the temporary is destroyed, and in its destructor the message is posted.
Another technique is what `spdlog` does -- the user makes a single function call to log, passing both the format string and all of the arguments all at once. This would be a more significant rewrite of the calling-code than the stream lifetime change, but would be a better choice in case the `Console` needed to be used from languages beyond just C++.
## Alternatives considered
A different design would be to keep the streaming API unchanged, but to have the _implementation_ monitor the character buffer insertions for newlines, and replace all newlines with EOM / log posting. I am not a fan of that design, because it removes the ability for a single log message to have embedded newlines, which I've often found to be useful.
## Additional context
For sdformat in particular, as I've explained in https://github.com/ignitionrobotics/sdformat/issues/334#issuecomment-683985626 the _library_ should never be printing to any console anyway. The calls to parse should have a _structured_ way to report errors, and should be silent on `cerr`. The current design and use of `sdf::Error` gets us most of the way there, but there are still a ton of places in the library code that stream to `sdferr` (or `sdfwarn`, etc.) instead of appending an `sdf::Error`. If we fixed _that_ problem instead, then we could delete the sdformat-specific `Console` class and this whole discussion here is moot (though would still apply to the ignition-common logging API design).
Contributor guide
Assessment
This issue has not been assessed yet.