[PATCH 1/2] log: new --log-func-loc option to enable function location in logs

[Aleksander Morgado]

On Thu, May 18, 2017 at 1:51 PM, Thomas Haller <thaller at redhat.com> wrote:
>> The --debug output is a bit annoying right now because it includes
>> the
>> function location information by default. This information is most of
>> the times not very useful, so just make it optional under a new
>> command line argument for the daemon.
>
> Hi,
>
> For NetworkManager you cannot configure the bits that are logged. The
> reasoning is, that a user reporting a bug usually wouldn't tweak the
> logging format. Imagine you have to ask the reporter for enabling debug
> level, turn on time-stamping, file-location, etc.
>
> Also, one gets accustomed to the logging formats. If the user enables
> --log-func-loc, then the log file looks unfamiliar.
>
>
> The format is not configurable, but optimized to what is considered
> best.
>

I really have no reason to go against that approach really, I always
do --debug when running the daemon anyway, and that enables debug
logging plus some other internal features like --command in mmcli.

These are the options that we woiuld have w.r.t. logging (after the
patches I suggested):
Logging options
  --log-level=[LEVEL]                Log level: one of ERR, WARN, INFO, DEBUG
  --log-file=[PATH]                  Path to log file
  --log-timestamps                   Show timestamps in log output
  --log-relative-timestamps          Use relative timestamps (from MM start)
  --log-func-loc                     Enable function location
information on log messages

I assume that the first two we never want to remove them. I personally
wouldn't want to remove --log-file, as I've seen several companies
using it explicitly.

The timestamp info, we could add it by default when printing in stdout
or to a file, and then remove the support for relative timestamps, not
really sure how useful that is at the end...

>
> -- btw, for NM we also got rid of the file location. Instead logging
> lines shall have a topic prefix, like:
>
>   <info>  [1495107798.9054] manager: startup complete
>   ^       ^                 ^        ^
>   LEVEL   timestamp         topic    message
>
> Also, each logging message should be unique, so that you can find the
> exact location in the source. So, if you know the source, you don't
> need file:loc because you can grep for message.

I actually agree in not needing file:loc if messages are unique; but
sometimes I think the func:loc info may be useful for newcomers that
want to follow the logic, which is why I thought of making optional
instead of removing it. Again, not a hard opinion on that one, I'd
personally be fine removing it all together. If we end up adding
systemd journal support, it already contains the optional func:loc
info itself, so maybe this isn't really an issue any more and we can
just get rid of that custom option right away.

-- 
Aleksander
https://aleksander.es