Re: LDAP Debug messages & severity levels

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

Howard Chu writes:
>Hallvard B Furuseth wrote:
>>   Not sure how the loglevel and severity level should interact.
>>   The most severe messages should be given independently of
>>   loglevel, so maybe their macros do not need a loglevel.
>>
>>   With the others - log the message if both loglevel and
>>   severity level is satisfied?  That's inconsistent with severe
>>   messages that are logged just due to severity level though.
>>   Maybe it'll be easier to decide after the debug messages
>>   have been classified by severity.
>
> I think you're going the right direction.  Genuine Error and Warning
> messages should not need any further qualification. It is only
> Info/Debug messages that really need the fine-grain "loglevel"
> differentiators, so they can be selectively turned on and off.

OK.  Well, actually I'm not sure about warnings.  I certainly appreciate
fine-control warning options in some other programs.  But that can wait
in any case:  I'd prefer a loglevel argument even in macros that don't
use it at least until the code stabilizes, so a simple s/Warn/Info/ will
be enough to downgrade a message from a severity level which doesn't use
the loglevel to one which does.

> There are some parts of the documentation that state that Statslog is
> always available, even with --disable-debug. I think that doc is now
> wrong, but it reflects an intention that we should probably restore.
> That pretty much means Stats/Info, Warn, and Error are never disabled.

OK.

Should the default loglevel be 0 instead of Stats with --disable-debug
- at least in OpenLDAP 2.3?  Different defaults from different
configurations can be confusing, but OTOH that will by now be the
smallest change for current installations that use --disable-debug.

>>   include/ldap_log.h #defines twelve LDAP_LEVEL_* constants,
>>   but they only seem to be used in the unused debug2syslog().
>
> Left over from NEW_LOGGING ?

Ah, of course.  From ldap_log.h revision 1.19 "first try at logging
improvements".  Well, they could just as well be left alone for now.
Might need them soon.


A few other points:

Should we take the opportunity to get rid of "too many arguments for
format" warnings?  Introduce <Debug,Info,Warn,Err,Statslog><1,2,3,4,5>()
macros that take 1-5 arguments.  I believe they'd all be one-liners
calling the same general logging macro.  Well, maybe Statslog*() would
keep a separate macro.  The old Debug() and Statslog() APIs would also
stay, at least for now.

We could also switch to using Debug/Err/...( SOMELEVEL, ... ) instead
of Debug( LDAP_DEBUG_SOMELEVEL, ...) and have the macros prepend
LDAP_DEBUG_ to SOMELEVEL, as NEW_LOGGING did.


As a practical matter, this change could be introduced behind

#ifdef SEVERITY_LOGGING /* defined when LDAP_DEVEL */
# define Debug(), Info(), Err() etc to do just what Debug() does now
#else
# define them to obey a configurable severity level, and #define all
# except Debug() even when --disable-debug.
#endif

That way there should be is no need for a PITA like #ifdef NEW_LOGGING
everywhere in the code, nor a separate CVS branch (which I imagine would
have a lot of conflicts by the time we were ready to merge it into
HEAD).

-- 
Hallvard