Rethinking debug levels
by Pavel Březina
We recently focus on providing lots of tracing messages in new code that
helps us follow the code, however this makes the debug logs quite big
and we usually focus only on a specific area or task while debugging. A
year ago we discussed what we can do to improve our debugging
capabilities but it never got into action since we hit the long standing
requirement to revisit all debug messages and their levels.
1) Originally, we had only numeric debug levels. The problem here was
that those numbers were assigned relatively randomly per developer decision.
2) My first task when I joined the team was to change the levels from
numeric to a bit mask. This seemed good at first as it allows to select
specific messages that we want to see but it was never polished into a
usable feature for two main reasons -- it required us to revisit all
existing messages and it turned out it actually doesn't make much sense
to disable any lower level since if you want to see tracing message you
always want to see error etc. So in the end the bit mask is used in the
same way as numeric representation (if used at all) it is just harder to
read.
My needs when debugging some issue are very simple -- be able to see as
small log as possible but having all errors and tracing messages. The
problem usually lies in a small area of code and I don't need to see
other parts of code that hinders these messages.
As a developer I would like the change into new format be seamless so it
does not require manual changes into current code. And I would also like
to have smaller number of choices.
What I propose is this: have a numeric debug level and a bit mask
representing an sssd component.
*Debug level*
0 = Fatal failures, important messages. Always enabled. For cases that
prevents SSSD from starting and for very important messages.
1 = Errors, for any error that we can recover from.
2 = Warnings, for unexpected situations were we can actually continue
with the operation. (E.g. when processing bunch of objects in a for
cycle and continue on error).
3 = Data, for user input, configuration.
4 = Trace, for tracing code flow.
5 = Verbose, for things that are very rarely required and would just put
lots of things into logs (e.g. verbose information about sudo rules).
*Component*
An SSSD module as separated from the rest as possible. For example: all,
failover, sudo, id, subdomains, ldb, ... lots of possibilities.
SSSD needs to be started or configured with two options
--debug-leve=number, --component=bitmask (default all). Messages with
level <= numeric and component & mask are printed.
Ideally components can be separated also on makefile level into static
libraries that we will link with. This would help with unit testing
those parts since we wouldn't have to maintain source file list in unit
test definitions but link with a library instead. It would also speed up
compilation. This library could have -DSSS_COMPONENT_NAME=$name and we
wouldn't even have to put it in DEBUG macro.
Thoughts?
7 years, 9 months
[PATCH] dyndns: Add checks for NULL
by Michal Židek
Hi!
The attached simple patch just makes the code more
defensive. We do not know the real
cause for the segfault and we do not
have a reproducer.
Michal
7 years, 10 months
PROVIDERS: Setting right {u,g}id if unpriveleged
by Petr Cech
Hi list,
attached patch resolve blocker ticket [1].
Idea of this bug is simple.
It was used uninitialized (respectively talloc_zero() initilaized) uid
and gid for dp_init() function. This was right for sssd running as root
but not for non-root user.
PS: Locally run CI tests failed massively on valgrind tests. I hope it
is not connected.
[1] https://fedorahosted.org/sssd/ticket/3077
Regards
--
Petr^4 Čech
7 years, 10 months
Re: [PATCH] sssctl: manual page
by Michal Židek
On 07/07/2016 07:12 PM, Michal Židek wrote:
> On 07/07/2016 06:45 PM, Michal Židek wrote:
>> The man page looks good to me with exception
>> for one detail (see inline)
>>
>> On 07/04/2016 12:45 PM, Pavel Březina wrote:
>>> + <refsect1 id='options'>
>>> + <title>COMMON OPTIONS</title>
>>> + <para>
>>> + Those options are available with all commands.
>>> + </para>
>>> + <variablelist remap='IP'>
>>> + <varlistentry>
>>> + <term>
>>> + <option>--debug</option>
>>> + <replaceable>LEVEL</replaceable>
>>> + </term>
>>> + <xi:include
>>> xmlns:xi="http://www.w3.org/2001/XInclude"
>>> href="include/debug_levels.xml" />
>>
>> This include contains some irrelevant information for sssctl
>> that could be confusing for users. But I do not think we
>> should block the patch, I will create a ticket to track this
>> as a man page bug.
>>
>>> + </varlistentry>
>>> + </variablelist>
>>> + </refsect1>
>>> +
>>> + <xi:include xmlns:xi="http://www.w3.org/2001/XInclude"
>>> href="include/seealso.xml" />
>>> +
>>
>> The attached patch is the same as Pavel's, with some
>> whitespaces removed (git complaint about them).
>>
>> Otherwise LGTM ( == ACK, because we do not have much
>> time and I think it is better than nothing).
>>
>> Michal
>
> The mailman is very slow today for some mails.
>
> Sending again with the hope it will arrive sooner.
>
> Michal
Third time...
7 years, 10 months
[PATCH] sssctl: Fix warning maybe-uninitialized
by Lukas Slebodnik
ehlo,
I can see warning maybe-uninitialized in mock build.
I had a plan to initialize it to NULL.
But this version fixed if as well.
It looks like some special gcc optimalisation and special case
may cause to have unitialized output argument _dom when return
code is EOK
LS
7 years, 10 months
