search

orocos-toolchain / ocl

Orocos Component Library
Other
16 stars 33 forks source link

OCL:Logging: Add logging priority filter to appenders. #70

Open disRecord opened 6 years ago

disRecord commented 6 years ago

I do not known if someone uses OCL::Logging. But without analog of log4cpp::Appender::setTreshold its interface looks incomplete.

meyerj commented 6 years ago

Looks good to me. @snrkiwi ?

snrkiwi commented 6 years ago

Supporting setting the threshold per appender is fine, but in reality what I think you really want to do is set the priority on the category(-ies) being logged instead. If this appender has multiple connected categories being logged at a variety of levels, what is the desired result on then setting the Appender's priority level also?

Also, that's a non-backwards compatible fix to force setting in startHook(). Are you relying on the "NOTSET" value to not actually filter anything? Also, why set in startHook() and not configureHook()?

disRecord commented 6 years ago

Actually?, only log levels cannot address all use cases. Consider following configuration:

motion = WARN  --> OstreamAppender (to display ERRORS and WARNING in console)
motion.herkulex = DEBUG --> FileAppender (for debug purposes)

Without filter property we get the flood of debug messages in console. And this problem does not have good solution. If additivity is disabled ERROR messages from herkulex sybsystem are not displayed. Another way is move herkulex category outside from motion but this breaks logical structure of categories.

Also, that's a non-backwards compatible fix to force setting in startHook(). Are you relying on the "NOTSET" value to not actually filter anything?

NOTSET disables threshold checking according to log4cpp documentation. http://log4cpp.sourceforge.net/api/classlog4cpp_1_1Appender.html#a7

Also, why set in startHook() and not configureHook()?

I agree, it seems better to move configureThreshold() from startHook() to configureLayout().