All of lore.kernel.org
 help / color / mirror / Atom feed
From: William Kennington <wak@google.com>
To: Johnathan Mantey <johnathanx.mantey@intel.com>
Cc: "Vijay Khemka" <vijaykhemka@fb.com>,
	"Vernon Mauery" <vernon.mauery@linux.intel.com>,
	"Patrick Williams" <patrick@stwcx.xyz>,
	openbmc <openbmc@lists.ozlabs.org>,
	"Brad Bishop" <bradleyb@fuzziesquirrel.com>,
	郁雷 <yulei.sh@bytedance.com>
Subject: Re: OpenBMC Logging and Error Handling Dos and Don'ts
Date: Wed, 3 Jun 2020 12:25:34 -0700	[thread overview]
Message-ID: <CAPnigKnAytWQP0LjujX0MvG2JZCxYjth3RSYScYYGjshxCYBgg@mail.gmail.com> (raw)
In-Reply-To: <41f9a41f-1681-54be-2cb5-dc71e86829f7@intel.com>

[-- Attachment #1: Type: text/plain, Size: 5156 bytes --]

The local process always emits all logs generated by phosphor-logging, it
is up to the journald configuration what levels are used for different
things.
https://www.freedesktop.org/software/systemd/man/journald.conf.html

You can change the priority level output by journalctl on the command line
too, to filter out lower priority messages.
`journal -p err` or some other level

On Wed, Jun 3, 2020 at 12:18 PM Johnathan Mantey <
johnathanx.mantey@intel.com> wrote:

> I would like it to be easier to determine how to change the error message
> level.
> I don't bother with the log code because I don't know where it's
> controlled.
>
> On 6/3/20 12:02 PM, Vijay Khemka wrote:
>
> On 6/2/20, 5:08 PM, "openbmc on behalf of Vernon Mauery" <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vernon.mauery@linux.intel.com> <openbmc-bounces+vijaykhemka=fb.com@lists.ozlabs.orgonbehalfofvernon.mauery@linux.intel.com> wrote:
>
>     On 02-Jun-2020 06:06 PM, Patrick Williams wrote:
>     >On Mon, Jun 01, 2020 at 11:07:46PM -0700, William Kennington wrote:
>     >> If you use the fmt library it would only require one extra temporary string
>     >> to be constructed and it works trivially with something like phosphor
>     >> logging.
>     >>
>     >> log<level::INFO>(fmt::format("My error: {}", filename));
>     >>
>     >> It also has the advantage of understanding basic c++ types like
>     >> std::strings and std::string_views.
>     >>
>     >> On Mon, Jun 1, 2020 at 8:33 PM 郁雷 <yulei.sh@bytedance.com> <yulei.sh@bytedance.com> wrote:
>     >>
>     >> > I have a bit concern about this. The existing phosphor-logging API
>     >> > does not support this well.
>     >> > Specifically, it does not support the "printf" way to generate a
>     >> > string with the variables to log.
>     >> > So previously we typically put the variables in entries.
>     >> > I do agree that logging the variables in the MESSAGE field is much better.
>     >> > But to encourage such logging, the logging API should be enhanced to
>     >> > support the "printf" way.
>     >> > E.g.
>     >> >     log<level::INFO>("Something is wrong: %s:%d", xx, xxx);
>     >> > is much better than:
>     >> >     std::string to_log = xxx; // generate the message manually
>     >> >     log<level::INFO>(to_log);
>     >> >
>     >> > Otherwise, it's really not convenient to write such logging code.
>     >> >
>     >
>     >Separate from this proposal from Brad, I'd like to work on a
>     >next-generation of phosphor-logging.  When we originally implemented
>     >phosphor-logging we were all pretty new to C++14.  I think there can be
>     >some great improvement in the syntax of phosphor-logging now that we
>     >have C++17 (and more experience).
>     >
>     >The biggest change I'd like to do is in the way the structured entries
>     >are defined, but I think we can improve the verbosity of syntax in
>     >making a log in general.
>
>     One more change I would like to see is a way to actually have the real
>     filename and line number show up in the systemd log. Right now the
>     filename and line number are always the logger code:
>         CODE_LINE=76
>         CODE_FUNC=helper_log
>         CODE_FILE=.../usr/include/phosphor-logging/log.hpp
>
> It will be good if we can have debug as a level and somehow we can see
> messages from debug by enabling or disabling.
>
>     It looks like C++20 has a shiny new helper for this: source_location
>     https://urldefense.proofpoint.com/v2/url?u=https-3A__en.cppreference.com_w_cpp_experimental_source-5Flocation&d=DwIFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=v9MU0Ki9pWnTXCWwjHPVgpnCR80vXkkcrIaqU7USl5g&m=qFbu7FdFQSHVMy4ZvMtHDH5RzYQnqMUQ_T0h4qL0Xd0&s=QRsbQvb4LKAEoa8AITFyPFhcoCSVHtFLMOU66UodPnM&e=
>
>     --Vernon
>
>     >Some example syntax:
>     >
>     >// Current syntax (and I'm being especially verbose on the namespaces).
>     >phosphor::logging::log<phosphor::logging::level::ERR>("Some message",
>     >    phosphor::logging::entry("EXAMPLE=%s"), example_str);
>     >
>     >// Potential syntax
>     >lg::error("Some message", "EXAMPLE"_s, example_str);
>     >
>     >In a code-review I was on, we had some discussions about how difficult
>     >it is to create data that isn't in the "-o verbose" and I think William
>     >and Lei are both referring to that as well.  I am concerned that a
>     >generic format leads us more to unstructured logging, which personally I
>     >don't like.  My suggestion is that we have some easy syntax to indicate
>     >"also put this structured data into the raw message" so that my example
>     >above the message becomes "Some message. EXAMPLE='the string'".
>     >
>     >I'd be interested to hear what "next generation logging" others have in
>     >mind.
>     >
>     >--
>     >Patrick Williams
>
>
>
>
>
> --
> Johnathan Mantey
> Senior Software Engineer
> *azad te**chnology partners*
> Contributing to Technology Innovation since 1992
> Phone: (503) 712-6764
> Email: johnathanx.mantey@intel.com
>
>

[-- Attachment #2: Type: text/html, Size: 7098 bytes --]

      reply	other threads:[~2020-06-03 19:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-01 20:35 OpenBMC Logging and Error Handling Dos and Don'ts Brad Bishop
2020-06-02  3:32 ` 郁雷
2020-06-02  6:07   ` William Kennington
2020-06-02 23:06     ` Patrick Williams
2020-06-02 23:09       ` William Kennington
2020-06-03  0:07       ` Vernon Mauery
2020-06-03  2:51         ` 郁雷
2020-06-03 19:02         ` Vijay Khemka
2020-06-03 19:17           ` Johnathan Mantey
2020-06-03 19:25             ` William Kennington [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPnigKnAytWQP0LjujX0MvG2JZCxYjth3RSYScYYGjshxCYBgg@mail.gmail.com \
    --to=wak@google.com \
    --cc=bradleyb@fuzziesquirrel.com \
    --cc=johnathanx.mantey@intel.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=patrick@stwcx.xyz \
    --cc=vernon.mauery@linux.intel.com \
    --cc=vijaykhemka@fb.com \
    --cc=yulei.sh@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.