Skip to content

Conversation

@disRecord
Copy link

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

@meyerj meyerj force-pushed the ocl-logging-appender-filters branch from 8a1bd81 to 4d3f6ab Compare January 8, 2018 18:13
@meyerj meyerj force-pushed the ocl-logging-appender-filters branch from 4d3f6ab to b875c51 Compare January 8, 2018 18:14
@meyerj
Copy link
Member

meyerj commented Jan 8, 2018

Looks good to me. @snrkiwi ?

@snrkiwi
Copy link
Contributor

snrkiwi commented Jan 8, 2018

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
Copy link
Author

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().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants