On Fri, 18 Oct 2024 19:07:18 +0200 David Marchand <david.march...@redhat.com> wrote:
> Hello Stephen, > > On Wed, Oct 16, 2024 at 10:24 PM Stephen Hemminger > <step...@networkplumber.org> wrote: > > > > Improvements and unification of logging library. > > This version works on all platforms: Linux, Windows and FreeBSD. > > > > This is update to rework patch set. It adds several new features > > to the console log output. > > > > * Putting a timestamp on console output which is useful for > > analyzing performance of startup codes. Timestamp is optional > > and must be enabled on command line. > > > > * Displaying console output with colors. > > It uses the standard conventions used by many other Linux commands > > for colorized display. The default is to enable color if the > > console output is going to a terminal. But it can be always > > on or disabled by command line flag. This default was chosen > > based on what dmesg(1) command does. > > > > Color is used by many tools (vi, iproute2, git) because it is helpful; > > DPDK drivers and libraries print lots of not very useful messages. > > And having error messages highlighted in bold face helps. > > This might also get users to pay more attention to error messages. > > Many bug reports have earlier messages that are lost because > > there are so many info messages. > > > > * Add support for automatic detection of systemd journal > > protocol. If running as systemd service will get enhanced > > logging. > > > > * Use of syslog is optional and the meaning of the > > --syslog flag has changed. The default is *not* to use > > syslog if output is going to a terminal. > > > > Add myself as maintainer for log because by now have added > > more than previous authors. > > Thanks for the series. > > Overall, it looks good, but I am too short on time for merging in rc1 > and I have some comments. > I'll consider merging it in rc2. > > > - The main point is the "automatic" aspect but we want to provide some > way to force where the logs end up. > With this series, the user has --syslog (whose meaning is changed) and > --log-journal options to affect where the logs go. > Can we get a single option? > Like maybe --log-destination=console|syslog|journal|auto ? That could work. The goal here is to do the right thing without options. And reduce the duplication. Right now, applications end up logging to both console and syslog. > > - I don't really understand why changing the --syslog is better. > We lose the ability to select the syslog facility. > Either this feature was useless, and I would rather deprecate or > remove it explicitly (and the --syslog with it). > Or we should keep it as is for compat reason. Syslog facility really was pretty useless. The number of facilities is too small and there never was a good reason to have it. Maybe keep old option syntax as fallback? > > - The color and timestamping options only affect the console output, > which is not clear with the --log-color / --log-timestamp names. > Maybe we can enhance with some other name? Timestamp is unneeded with syslog or journal since they do it already. It is possible to force timestamp or color when going to a file (by setting log stream). Did consider that options could be: --color and --timestamp but seemed better to keep all log related options together. > > - Did you test redirecting all logs to an external logging function? > I suppose it still works, but it is important not to break such feature for > OVS. Did not have test for that, should be put in functional tests. I flip/flopped on default for color. I like the auto mode and it might make users see errors more. But concerned that things like CI which are capturing via pseudo-terminals might get color and not know what to do with it. Final version defaults to off.
