ldap: add new option ldap_library_debug_level

[sumit-bose]

With the new option ldap_library_debug_level the debug level for
OpenLDAP's internal debugging can be set. If set the OpenLDAP debug
messages will be written to the logs independent of the general
debug_level.

[alexey-tikhonov]

Why str and not int?

[sumit-bose]

ah, sorry, typo, fixed.

[alexey-tikhonov]

(JFTR: I guess this relates to https://bugzilla.redhat.com/show_bug.cgi?id=1839972)

An approach to debug messages from different libs, e.g. libldap vs libldb, feels a little bit inconsistent...

I wonder if it would be better (simpler to use) to just rename SSSDBG_TRACE_LDB to something like SSSDBG_TRACE_FOREIGN_LIBS and use this level for all those occasions...

[alexey-tikhonov]

I also think it is not very useful to have an ability to enable libldap logs totally independent of general sssd log level. Some combinations doesn't really have a lot of value, imo.

[sumit-bose]

Hi,

I made the debug output independent of the general debug_level mainly based on the experience with libldb.

Typically the debug output of libldb is not needed and make logs harder to read. So if it would be only about libldb a dedicated debug_level would work. But I think something like SSSDBG_TRACE_FOREIGN_LIBS does not help much because if in the given case we want to check what libldap is doing internally why would I need libldb output as well? That why I think to solution which dedicated options scales better when we want to handle the debug output of multiple libraries.

But I agree there are some drawbacks as well. E.g. new options are needed. Or it is not easy to just switch on all debugging, but here I think it is typically not needed, especially libldap debugging.

bye,
Sumit

[alexey-tikhonov]

I think something like SSSDBG_TRACE_FOREIGN_LIBS does not help much because if in the given case we want to check what libldap is doing internally why would I need libldb output as well?

Right, agree.

But having this output with SSSDBG_TRACE_ALL level regardless debug_level still feels weird and most probably useless (if debug_level< SSSDBG_TRACE_FUNC most probably it will be impossible to interpret output in most cases)

Wouldn't it make sense to:

1. either use DEBUG instead of sss_debug_fn directly, so that ldap_library_debug_level will have effect only if debug_level is high enough
2. or have something like ldap_library_debug_level = log_level=N,log_mask=M to specify log level to use in sss_ldap_debug

I was thinking about generalization of this option for all "foreign libs" in the form libs_debug = lob_level=N, libldap=mask, libldb = mask ... but perhaps this is too complicated.

In general I worry how to do this change more user-friendly / useful. So far this feels like "yet another one knob only few developers will be aware of".

Having said this, patch itself is functional and should work fine, of course.

[pbrezina]

I just used this patch to debug something and it works as expected.

The SSSD debug level is a bitmask and the idea behind it is that you can enable or disable specific messages. So we can certainly add SSSDDBG_EXTERNAL_LDAP or something and enable -1 ldap level if this is set. But I'm fine with the option as well, especially if you think that something else then -1 (enable all) is helpful.

[sumit-bose]

Hi,

as I said I'd prefer to use a separate option for this because in more or less all cases this debug output is not needed and -1 is very verbose. So I think "yet another one knob only few developers will be aware of" is completely find here because it should be only used if there are strong indications that something is wrong on the libldap level.

bye,
Sumit

[alexey-tikhonov]

as I said I'd prefer to use a separate option for this because in more or less all cases this debug output is not needed and -1 is very verbose. So I think "yet another one knob only few developers will be aware of" is completely find here because it should be only used if there are strong indications that something is wrong on the libldap level.

Ok.

[pbrezina]

Ack.

[pbrezina]

Pushed PR: #5178

• master
  • 5fb2263 - ldap: add new option ldap_library_debug_level