[Spice-devel] [PATCH spice-common 3/4] RFC: Add spice log categories

[Victor Toso]

Hi,

On Tue, Jun 13, 2017 at 10:55:23AM -0400, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
>
> > > If you set G_LOG_USE_STRUCTURED, glib will use structured log API. But
> > > there is a single structured log handler (contrary to traditional glog
> > > that can have per-domain).
> >
> > Hm, I have yet to play with the API to say for sure but I thought we
> > could use G_LOG_USE_STRUCTURED and set a g_log_set_writer_func() on our
> > spice_log_init() and if it doesn't not belong to our domain, we could
> > have the _writer_func_handler() to return G_LOG_WRITER_UNHANDLED.
> >
> > But now I see that g_log_set_writer_func() manual says "Libraries **must
> > not** call this function — only programs are allowed to install a writer
> > function, as there must be a single, central point where log messages
> > are formatted and outputted." so you are probably right that
> > G_LOG_USE_STRUCTURED is not useful to us.
>
> What do you mean by "not useful to us"

Sorry if I wasn't clear. By using G_LOG_USE_STRUCTURED I thought we
could use g_debug and friends in spice-gtk, spice-server and
spice-common instead of wrapping it with spice_debug and friends. As we
can't use G_LOG_USE_STRUCTURED in a library, this is not possible and
your solution seems to be the right on.

> you can use the structured logging to do more advanced queries with
> the various fields, and I hope that tools like GNOME Logs will provide
> even more friendly ways to display & dig in the logs.

Yes, I'm happy about it.

>
> > > How would you improve "override future deprecation warning" ? add the
> > > version it was introduced?
> > 
> > Something like your reply to Jonathon:
> > 
> > "override future deprecation warning from symbols introduced after
> > GLIB_VERSION_MAX_ALLOWED"
>
> ok, I thought we were used to that IGNORE_DEPRECATIONS, since I see it
> used many times in spice-gtk without such comments.

Your comment is fine. The confusion is probably around introducing new
function wrapped with ignore-deprecations. AFAIK, all the other places
the patches were related to either changing functions or removing the
deprecation warnings.

>
> > > > Why not spice_log(Name, log_level, Msg, ...) and on SPICE_LOG_ENABLED we
> > > > consider the log_level as well?
> > >
> > > As I said in other comments, we should consider this. But we would
> > > first need to transition the code to the new API. That is easier if we
> > > switch all spice_{debug,warning,...} to glog (since it's 1-1), then
> > > reintroduce new spice_{debug,warning..} with categories.
> >
> > Right.
> >
> > > Does that make sense? Do you see another easy path forward?
> >
> > Easy? No :) I think it is straight forward as is but it'll depend if we
> > want to keep some of those environments for longer time and what
> > considerations we have on spice logging functions as API between
> > spice-gtk and spice-server (Frediano had a comment about it IIRC)
> >
> > IMHO removing the deprecated environment variables should be fine and
> > finishing up your series with spice_{debug,warning,...} using glog and
> > categories in the end would be great as, for what I see, we should move
> > all g_{debug,warning,...} in spice-gtk, spice-server and spice-common to
> > use spice_{debug,warning,...} (correct me if I'm mistaken here, plz)
>
> Right, everything we want to log with a category, we can use spice log
> (spice_debug and friends) instead of glog.

Right

> Imho, if there is no clear category (I guess there should always be
> one, being named after the file name for ex),

Yes, makes sense to me.

> or if you want to use more structured fields, direct glog/structured
> usage is fine too.
>
> I sometime question the need for several categories in the same file,
> but since our code is not that well structured, I think it's necessary
> (VIR_DEBUG doesn't allow multiple categories in the same file for ex)

Flexibility with extra categories per file is good IMHO. Not sure how
much we will end up using it but maybe it can be used to provide more
verbose (development related) debug such as what c3d was interested in
having.

Cheers,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170614/75a93df8/attachment.sig>