On (23/11/15 09:40), Jakub Hrozek wrote:
On Mon, Nov 23, 2015 at 09:28:59AM +0100, Lukas Slebodnik wrote:
> 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:32), Petr Cech wrote:
> >> >>>>>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.
It's 2015 :-)
(And this is different than the 80-chars limit which still makes sense
for developers who often like to vertically split their editors)
It's not about 80 character limit. We already does not meet this criteria.
Small example:
(Mon Nov 23 10:12:24 2015) [sssd[be[ipa.corp.example.com]]] [server_setup] (0x0400):
CONFDB: /var/lib/sss/db/config.ldb
Even the 190 columns is not enough for sme messages.
(Mon Nov 23 10:12:24 2015) [sssd[be[ipa.corp.example.com]]] [sbus_message_handler]
(0x2000): Received SBUS method org.freedesktop.sssd.dataprovider.getDomains on path
/org/freedesktop/sssd/dataprovider
But with string representations it will be extra 20 columns.
>
> >> >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.
Maybe, but neither of this helps admins distinguish what messages to
look for in the logs.
String representation of debug_level will not help either.
@see point a)
That's the reason why we firstly need make an order between messages (content)
and debug levels.
LS