You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by bu...@apache.org on 2008/01/28 15:17:56 UTC

DO NOT REPLY [Bug 44308] New: - [Patch] JMX component for managing Logger configurations

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=44308>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=44308

           Summary: [Patch] JMX component for managing Logger configurations
           Product: Log4j
           Version: 1.2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: PatchAvailable
          Severity: enhancement
          Priority: P2
         Component: Other
        AssignedTo: log4j-dev@logging.apache.org
        ReportedBy: stefan.fleiter@web.de


Log4j comes with HierarchyDynamicMBean for managing Log4j by means of JMX.

This class has the following restrictions:
- It can not be registered more than once at the same MBean server, this
  is a critical restriction in J2EE Environments.
- Only loggers specified in the Log4j configuration file can be configured.
- The code is overly complex.

Since these restrictions are by design I have implemented a more straight 
forward solution to this problem.

LoggerManager:             Class with Methods for managing loggers
LoggerManagerModelMBean:   Model MBean which makes LoggerManager methods
                           available per JMX
RollbackableLoggerManager: Extension to LoggerManager which records intial state
                           for all changed loggers and offers a method to
                           roll them back

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


DO NOT REPLY [Bug 44308] - [Patch] JMX component for managing Logger configurations

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=44308>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=44308





------- Additional Comments From psmith@apache.org  2008-02-07 18:35 -------
Ok, I _finally_ had time to sit down and review this.  It does look quite
substantial and generally well thought out.

Here are my comments:

RollbackableLoggerManager
	Ordering of rollback, should this be stack based instead?
	Be nice to indicate the size of the rollback state (number of undos) via jmx
	Operation to rollback just the most recent?
	
LoggerManager
	getLevelByName - stylistic, but should that method declare the RuntimeException?
	since this class is designed for extension, should some thought be made towards
making some of the methods final? (Effective Java)
	setRootLogLevel - declaring throwing RuntimeException?
	getConfiguredLoggers - is there an off-by-1 error there in th efor loop with
the ++i instead of i++  ?
	
LoggerManagerModelMBean
	+        try {
	+            getDelegate().getClass().getMethod("rollbackLogConfiguration", null);
	+            modelMBeanOperations.add(rollbackLogConfiguration);
	+        } catch (NoSuchMethodException expected) {
	+            // nothing to do
	+        }
	This seems a bit of a hack! :)   Given RollbackableLoggerManager extends from
LoggerManager, perhaps the same thought could be done to having a second
RollbackableLoggerManagerModelMBean that has a doPostRegisterOtherSchtuff()
style methods.
	
	
	

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


DO NOT REPLY [Bug 44308] - [Patch] JMX component for managing Logger configurations

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=44308>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=44308





------- Additional Comments From stefan.fleiter@web.de  2008-01-28 06:22 -------
I forgot to mention that I would like to contribute this patch under the
Apache License, Version 2.0.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


DO NOT REPLY [Bug 44308] - [Patch] JMX component for managing Logger configurations

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=44308>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=44308





------- Additional Comments From stefan.fleiter@web.de  2008-02-08 01:55 -------
(In reply to comment #4)

> RollbackableLoggerManager
> 	Ordering of rollback, should this be stack based instead?

The intention of Rollback was only to get Log4j in the state it was initially
configured. Since Log4j itself does not offer such an operation I implemented
this myself.
The order of the modifications of the levels of the logger instances is
not important for this purpose since Log4j determines the effective Level
with every logging action.
Since we do not set the effective level (which is not even possible) the order
of the undo operations does not matter.

> 	Be nice to indicate the size of the rollback state (number of undos)
>       via jmx Operation to rollback just the most recent?

I can add a method to show the size of the rollback information, no problem.
The undo information can be smaller than the modifications made
since only for the first change of a logger its' initial level is saved.
I do not know if there is an advantage of doing partial undo.
The purpose of rollback was to have the possibility to change the log levels
of production machines for a debugging session and to have a single operation
which can undo all changes to get it in the same state as the other nodes.

Partial undo would require a complete changed implementation of the 
RollbackableLoggerManager since I do only save the initial states.
One could do this with a MultiMap but I do not see a usecase for partial undo.
What usecase did you think of?
 	
> LoggerManager
> 	getLevelByName - stylistic, but should that method declare the
>       RuntimeException?

It's only for documentation purposes, if you prefer not to declare 
RuntimeExceptions I have no problems removing those declarations.

> 	since this class is designed for extension, should some thought be made 
>       towards making some of the methods final? (Effective Java)

What methods have you thought of?
When I start to think about this I could rename doSetLogLevel
to setLogLevelInternal and call a doSetLogLevelInternal template method.
Then one could make setLogLevelInternal final.
But even that is difficult because the extending class could wish to veto
the modification.
I am not a big fan of the final keyword because there always is somebody
who wants to do something with your class you have never thought of.

But if you feel strongly for making some method(s) final I will do this.

> 	setRootLogLevel - declaring throwing RuntimeException?

See above, no problem removing the declaration.

> 	getConfiguredLoggers - is there an off-by-1 error there in the for loop 
>       with the ++i instead of i++  ?

I hope I understand you correctly, but Java language specification 3 states
explicitly under 14.14.1.2, that the update part of the for statement (third
part in parentheses) is executed after the body/statement.
Therefor ++i and i++ have the same effect.
I got used to prefer pre-increment to post-increment if there is no difference
in their effects when programming C++.
So this can be changed but I do not see the mentioned off by one bug.


> LoggerManagerModelMBean
> 	+        try {
> 	+            getDelegate().getClass().getMethod("rollbackLogConfiguration",
null);
> 	+            modelMBeanOperations.add(rollbackLogConfiguration);
> 	+        } catch (NoSuchMethodException expected) {
> 	+            // nothing to do
> 	+        }
> 	This seems a bit of a hack! :)   Given RollbackableLoggerManager extends 
>       from LoggerManager, perhaps the same thought could be done to having a
>       second RollbackableLoggerManagerModelMBean that has a 
>       doPostRegisterOtherSchtuff() style methods.

You got me there. :-)
That really is a hack.
RollbackableLoggerManagerModelMBean could easily override
createModelMBeanOperationInfos of LoggerManagerModelMBean which would
not register that method.

Thanks a lot for your kind review.


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


DO NOT REPLY [Bug 44308] - [Patch] JMX component for managing Logger configurations

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=44308>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=44308


stefan.fleiter@web.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #21436|0                           |1
        is obsolete|                            |




------- Additional Comments From stefan.fleiter@web.de  2008-01-29 02:31 -------
Created an attachment (id=21442)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=21442&action=view)
Refactored patch to provide an AbstractModelMBean base class

I have refactored the previous patch to provide a AbstractModelMBean base
class.
The code is more clean and concerns are separated.
The primary goal of the refactoring was to make LoggerManagerModelMBean easily
extendable if anybody has further needs in this direction.


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


DO NOT REPLY [Bug 44308] - [Patch] JMX component for managing Logger configurations

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=44308>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=44308





------- Additional Comments From stefan.fleiter@web.de  2008-01-28 06:19 -------
Created an attachment (id=21436)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=21436&action=view)
Path to manage Loggers per JMX


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org