On (23/11/15 09:16), Jakub Hrozek wrote:
On Mon, Nov 23, 2015 at 09:02:05AM +0100, Lukas Slebodnik wrote:
> On (23/11/15 08:56), Petr Cech wrote:
> >On 11/23/2015 08:37 AM, Lukas Slebodnik wrote:
> >>On (23/11/15 08:32), Petr Cech wrote:
> >>>On 11/23/2015 08:13 AM, Lukas Slebodnik wrote:
> >>>>On (23/11/15 08:03), Petr Cech wrote:
> >>>>>On 11/20/2015 12:13 PM, Petr Cech wrote:
> >>>>>>Hello all,
> >>>>>>
> >>>>>>I would like to work on:
> >>>>>>https://fedorahosted.org/sssd/ticket/2808
> >>>>>>However, there are two different opinions.
> >>>>>>
> >>>>>>Ticket description says:
> >>>>>>........................
> >>>>>>Currently a log message of SSSD like e.g.
> >>>>>>
> >>>>>># (Mon Jun 29 22:10:59 2015) [sssd[be[lab.runlevelone.lan]]]
> >>>>>>[be_get_account_info] (0x0200): Got request for
[0x1001][1][name=dlavu]
> >>>>>>
> >>>>>>has the debug level given by '(0x0200)'. For the
inexperienced reader it
> >>>>>>is hard to see if the message indicates an error or is only
some
> >>>>>>informational messages.
> >>>>>>
> >>>>>>It would be good to replace them which short text tags which
better
> >>>>>>indicates the character and severity of the message. E.g.
something like
> >>>>>>ERR_CRIT, INFO_DATA, TRACE_FUNC ...
> >>>>>>-----
> >>>>>>
> >>>>>>Jakub said:
> >>>>>>One issue that I realized is that we might want to add the
string
> >>>>>>representation in addition to hexa to avoid breaking existing
scripts.
> >>>>>>But on the other hand, I'm not sure if we should care too
much about
> >>>>>>keeping compatibility of debug logs...
> >>>>>>
> >>>>>>And Lukas said:
> >>>>>>I use very often hexadecimal representation for filtering
huge log file.
> >>>>>>So it should not be replaced. It's much simpler (and
shorter) to write
> >>>>>>regex for hexadecimal numbers.
> >>>>>>-----
> >>>>>>
> >>>>>>In my opinion, we should write numerical values if and only
if they
> >>>>>>represent values (uid for example), not enum constant.
> >>>>>>
> >>>>>>
> >>>>>>In this situation I feel that filtering is important.
> >>>>>>I suggest use:
> >>>>>># (Mon Jun 29 22:10:59 2015) [sssd[be[lab.runlevelone.lan]]]
> >>>>>>[be_get_account_info] (0x0200 | INFO_FUNC_DATA): Got request
for
> >>>>>>[0x1001][1][name=dlavu]
> >>>>>>
> >>>>>>What is your opinion, please?
> >>>>>>
> >>>>>>Regards
> >>>>>>
> >>>>>>Petr
> >>>>>>_______________________________________________
> >>>>>>sssd-devel mailing list
> >>>>>>sssd-devel(a)lists.fedorahosted.org
>
>>>>>>https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
> >>>>>>
> >>>>>
> >>>>>Hi,
> >>>>>
> >>>>>I have one opinion.
> >>>>>
> >>>>>I just try to do such text labels:
> >>>>>SSSDBG_FATAL_FAILURE --> "ERR_FATAL_FAILURE"
> >>>>>SSSDBG_CRIT_FAILURE --> "ERR_CRIT_FAILURE"
> >>>>>SSSDBG_OP_FAILURE --> "ERR_OP_FAILURE"
> >>>>>SSSDBG_MINOR_FAILURE --> "ERR_MINOR_FAILURE"
> >>>>>SSSDBG_CONF_SETTINGS --> "INFO_CONF_SETTINGS"
> >>>>>SSSDBG_FUNC_DATA --> "INFO_FUNC_DATA"
> >>>>>SSSDBG_TRACE_FUNC --> "TRACE_FUNC"
> >>>>>SSSDBG_TRACE_LIBS --> "TRACE_LIBS"
> >>>>>SSSDBG_TRACE_INTERNAL --> "TRACE_INTERNAL"
> >>>>>SSSDBG_TRACE_ALL --> "TRACE_ALL"
> >>>>>SSSDBG_BE_FO --> "FAIL_OVER"
> >>>>>
> >>>>>Isn't this way better?
> >>>>>SSSDBG_FATAL_FAILURE --> "F4"
> >>>>>SSSDBG_CRIT_FAILURE --> "F3"
> >>>>>SSSDBG_OP_FAILURE --> "E3"
> >>>>>SSSDBG_MINOR_FAILURE --> "W3"
> >>>>>SSSDBG_CONF_SETTINGS --> "I2"
> >>>>>SSSDBG_FUNC_DATA --> "I2"
> >>>>>SSSDBG_TRACE_FUNC --> "T3"
> >>>>>SSSDBG_TRACE_LIBS --> "T2"
> >>>>>SSSDBG_TRACE_INTERNAL --> "T1"
> >>>>>SSSDBG_TRACE_ALL --> "T1"
> >>>>>SSSDBG_BE_FO --> "E4"
> >>>> ^^
> >>>> SSSDBG_BE_FO does not mean an error.
> >>>>>
> >>>>>I suppose category Fatal, Error, Warning. Trace, Info, each at 4
levels.
> >>>>No because man sssd.conf says something different about debug
levels.
> >>>>
> >>>>@see man sss.conf -> GENERAL OPTIONS
> >>>> -> Options usable in all sections
> >>>> -> debug_level
> >>>>
> >>>>LS
> >>>
> >>>Hi Lukas,
> >>>
> >>>I am afraid that I have to disagree, Lukas. The idea of ticket #2808 is
make
> >>>logs more readable for admins. I think it is good idea.
> >>>
> >>Have you read manual page?
> >Yes, I have read manual page.
> >
> >>
> >>Does the descrition in man page match your proposal with
> >>Fatal, Error, Warning. Trace, Info?
> >Yes, the description in man page matches my proposal in hexadecimal code. And
> >the description says, for example, that 0x0200 is linked to Function data.
> >
> >You could read my proposal like:
> >...(0x0200) (I3)...
> > ^^^^^^ ^^
> > | |---------- It is info, if you need quick orientation.
> > |------------------- This code says: Function data, look at man.
> But 0x0080 does not say anything about warning. It's still an error.
> man sssd.conf says:
> 3, 0x0080: Minor failures. These are the errors that would
> percolate down to cause the operation failure of 2.
>
>
> >
> >I hope it is clear what I would like to say. I don't say it is the best
> >solution. It is my opinion which matches your remarks to my patch.
> >I am pointing mainly on b) and c). You wrote in reaction to patch:
> >"
> >IMHO the text representation of debuglevel does not improve anything.
> >Reasons:
> >a) approximately 50% of codes were converted from nemerical value to
> >constants.
> > So most of messages does not correspond with debug constant
> >b) String representation of debug_level decrease readability of log.
> > Currently all debug messages starts at the same column. After your change
> > they will not. If the purpose of this patch is to simplify reading of logs
> > by human then the result will be opposite
Maybe we could add padding?
The point c) will get worse with extra padding.
And it's not only about size of logs. The user will need to have wider terminal
to read log file. Or move to right in paging utilities (more, less).
But moving to righ will cause missed info about timestamp.
> >c) the size of logs will be increased.
> >"
> >
> >So... if INFO, ERROR, FATAL and so on are wrong, well. Is there any idea how
> >to improve readability of logs?
> >
> Not the simple one.
>
> Firstly we need to fix point a) ; otherwise we will just confuse users with
> any string representation of debug level.
I think it would be enough to make sure that during the most common
operations, there are no logs with failure levels. The difference
between FUNC_DATA and TRACE_FUNC is small even to us, much less to an
admin.
So it might make sense to set clear difference between FUNC_DATA and
TRACE_FUNC
(or other debug_levels) and check it as part of review.
Otherwise it does not make a sense to use such variety of debug levels.
Because if difference is small to us (developers) then we can merge them
and simplify things.
LS