Refactoring logging not to indirect via JUL

[timfox]

As discussed in http://www.jboss.org/index.html?module=bb&op=viewtopic&t=161992 currently all logging delegates to JUL logging.

We want to change this to make the class org.hornetq.core.logging.Logger do the indirection instead.

I'm thinking perhaps we should use a system property to configure the logger fatcory class since logging might occur before the Configuration class is read.

We could specify the class name of a logger factory class in the system property and based on that in the constructor of the class org.hornetq.core.logging.Logger we can use an instance of that factory class to create the underlying logger instance.

Out of the box we should provide three implementations of the new logger factory class - one for JUL, one for log4j and one for JBoss logging.

[timfox]

https://jira.jboss.org/jira/browse/HORNETQ-167

[skaffman]

System properties bring with them some of the same difficulties as with configuration of java.util.logging.

I've been thinking of using the way that JBossAS does logging; it starts off using a default simple log4k config, and then switches to the user-defined config once it's bootstrapped its kernel.

My suggestion would be for HornetQServerImpl to inject the logging implementation into org.hornetq.core.logging.Logger right at the start of its start() procedure. If logging happens before then, it doesn't really matter, as long as the new config takes over and replaces any loggers so far created.

Logger would remain a class with the existing method signature, including static getLogger() (which would delegate to the provided factory), and the warn/info/debug etc methods which would delegate to whatever the factory created.

I've already written some code to do all of this, including implementations for JUL, log4j and Commons Logging. I can post it for you to review - should I post to the JIRA issue, or to here?

[timfox]

"skaffman" wrote:
System properties bring with them some of the same difficulties as with configuration of java.util.logging.

I've been thinking of using the way that JBossAS does logging; it starts off using a default simple log4k config, and then switches to the user-defined config once it's bootstrapped its kernel.

That sounds reasonable, although I would default to System.out/System.err since not everyone will have log4j

My suggestion would be for HornetQServerImpl to inject the logging implementation into org.hornetq.core.logging.Logger right at the start of its start() procedure. If logging happens before then, it doesn't really matter, as long as the new config takes over and replaces any loggers so far created.

ok

Logger would remain a class with the existing method signature, including static getLogger() (which would delegate to the provided factory), and the warn/info/debug etc methods which would delegate to whatever the factory created.

ok

I've already written some code to do all of this, including implementations for JUL, log4j and Commons Logging. I can post it for you to review - should I post to the JIRA issue, or to here?

That was quick :)

Can you attach it to the JIRA?

[skaffman]

OK, I've attached a patch to the JIRA issue.

The main refactoring was resolving the delegate for each call to one of the Logger methods. This was necessary because the Logger instances are generally instantiated as static finals, so resolving the delegate at that point is impractical. This solution should be quick enough, though.

I didn't touch any of the tests - I'm not sure what would be appropriate in this case.

[timfox]

Thanks Kenny-

I've integrated your patch into TRUNK, and made a few changes along the way, including:

1) A few changes to the main Logger including not looking up the Delegate each time
2) Allow it to be configured with a system property as well as calling setDelegateFactory() - on the client side setDelegateFactory() would not be called and we need some way for the user to choose the type of logging.
3) Removed apache commons log delegate - I don't think we need this.
4) Add config property in the main HornetQ config for choosing the log delegate - updated configs in various places to include this.

[ataylor]

have the docs been updated?

I also think that logging configuration would be a helpful wiki page!

[timfox]

"ataylor" wrote:
have the docs been updated?

Not yet