[jbossts-issues] [JBoss JIRA] Commented: (JBTM-612) check use of isDebugEnabled/debugAllowed

[Jonathan Halliday (JIRA)]

    [ https://jira.jboss.org/jira/browse/JBTM-612?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12483325#action_12483325 ] 

Jonathan Halliday commented on JBTM-612:
----------------------------------------

There appear to be two common (mis-)usages that cause problems.

if (moduleLogger.theLogger.debugAllowed())
{
    moduleLogger.theLogger.debug(DebugLevel.FOO, VisibilityLevel.VIS_FOO, FacilityCode.FAC_FOO, "string");
}

which uses the zero args version of debugAllowed rather than passing it the three levels that will be used in the message. That causes it to default to checking against full logging for each level, which normally fails, so the call returns false. Not too bad - we correctly skip the debug call in most configurations. It's more expensive in configurations where the DebugLevel on the logger is full logging (0xffffffff) and we are relying on the underling logger (e.g. log4j) to stop the log statements. This is not as uncommon as it sounds - remembering to change both the JBossTS logging config and the log4j config to enable/disable logging is a pain, so some users leave JBossTS on all the time and just toggle the log4j threshold. To deal with such cases we've added a isDebugEnabled call within the debugAllowed impl (r29049), although this is still somewhat expensive compared to our internal checks, see below.

if (moduleLogger.theLogger.isDebugEnabled())
{
    moduleLogger.theLogger.debug(DebugLevel.FOO, VisibilityLevel.VIS_FOO, FacilityCode.FAC_FOO, "string");
}

This is expensive as the implementation of isDebugEnabled ignores our internal DebugLevel and passes the query direct to the underlying logger, a much more expensive check. Ideally the user should call debugAllowed instead. Alternatively we can change isDebugEnabled impl to have the same behaviour as debugAllowed i.e. perform threshold checks against both out own internal level and (if needed) the underlying logger.


> check use of isDebugEnabled/debugAllowed
> ----------------------------------------
>
>                 Key: JBTM-612
>                 URL: https://jira.jboss.org/jira/browse/JBTM-612
>             Project: JBoss Transaction Manager
>          Issue Type: Task
>      Security Level: Public(Everyone can see) 
>          Components: Common
>    Affects Versions: 4.7.0
>            Reporter: Jonathan Halliday
>            Assignee: Jonathan Halliday
>             Fix For: 4.9
>
>
> The CLF exposes an rather complex API, in which isDebugEnabled and debugAllowed have different semantics. As pretty much everything is logged, the cost of these methods, and especially the cost of incorrect usage that leads to creating debug messages that just get throw away, is a non-trivial overhead. Both the implementation of these methods and their usage is suspect and needs a review.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira