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?
>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.