Use SLF4J as underlying logging API

[ceefour]

This is related to #2 but rather than inventing Rewrite's own logging API, better reuse SLF4J API as it is and then users can use any logging backend (logback, log4j, JUL, etc.).

Currently rewrite's logging leaves much be desired: (which makes it somewhat impractical to configure logging levels)

unknown.jul.logger | Rewrite 2.0.12.Final initialized.

At minimum, the logger name should be the actual class name. I had to configure logback like this:

<logger name="unknown.jul.logger" level="DEBUG"/>

instead of the proper:

<logger name="org.ocpsoft.rewrite" level="DEBUG"/>

Also, I'm not sure why Rewrite uses the JUL logger although SLF4J is available on my configuration. The rewrite-logging-slf4j module also no longer exists.

[chkal]

You can add this dependency to your project:

<dependency>
    <groupId>org.ocpsoft.logging</groupId>
    <artifactId>logging-adapter-slf4j</artifactId>
    <version>1.0.2.Final</version>
</dependency>

With this on your classpath Rewrite will delegate all logging to SLF4J instead of using JUL.

[ceefour]

@chkal can you reconsider the decision? Although org.ocpsoft.logging is available, it's better (performance-wise) to use SLF4J API directly (see http://slf4j.org/faq.html#logging_performance).

Given that we're currently having performance issues with 2.0.12 (#213) it'd be great if there's one less bottleneck potential in current development.

[chkal]

I'm not sure if using SLF4J directly is so much better performance-wise because the org.ocpsoft.logging abstraction also supports parameterized logging. So I don't think this will be a huge difference. Or are there any indications that this is the case?

[ceefour]

Performancenya definitely improves since there's at least one less in the call stack, although not sure how much or how little.

Another question is whether org.ocpsoft.logging is actually necessary? What can't slf4j-api do, and what's the downside to using slf4j-api directly?

Large projects, including Spring which I believe has vast and strict requirements, use slf4j-api directly.

[chkal]

The idea of using our own logging abstraction back then was to make it as simple as possible for users which actually use the standard Java logging API but also to allow users to use their own logging backend. I agree that if I had to make this decision today, I may choose SLF4J as it is very popular now.

However, I'm not sure about the benefit of moving completely to SLF4J now. This would be a huge refactoring and it would mean that users would have to adjust their dependencies if the update to the latest release.

The only benefit I could think of would be a huge performance gain. But one step less in the stack trace doesn't sound like a huge improvement compared to the heavy regex processing Rewrite has to perform. Therefore I don't think that there will be a noticeable performance gain. However, I may be wrong about that.

One thing I noticed is that we should add logging level checks to the logging base class:

https://github.com/ocpsoft/logging/blob/master/api/src/main/java/org/ocpsoft/logging/Logger.java#L161

Basically these methods should use isErrorEnabled() before calling format(). I guess this can be fixed very easily. PRs welcome. ;)

[ceefour]

Since 3.0 is under development,I think it's acceptable change.

With the proposed change, what user needs to do:

• If app is already configured to use slf4j (I.e. all Spring Boot apps), remove ocpsoft.logging and that's it
• If using other I.e. log4j, remove ocpsoft.logging and add slf4j-log4j*
• If truly using JUL, add slf4j-jdk14

Given that Spring defaults to slf4j, and lots of libraries use slf4j, typical apps would have already used a slf4j back end anyway. So the practical change is not more dependency, but actually reducing a dependency.

[chkal]

I agree that this change would be acceptable, although it would mean that some users would have to adjust their dependencies. And as I wrote earlier, using SLF4J directly may have been the better choice back then.

However, I still don't think that replacing one logging facade with another is worth the effort. It would be quite some work to remove org.ocpsoft.logging from the Rewrite code and to replace it with SLF4J. And I really don't think that this step would result in a measurable performance gain (after implementing this minor change I mentioned earlier).

However, if @lincolnthree agrees that using SLF4J is fine and if somebody volunteers to help with the implementation, we could do it.

[ceefour]

Yes if accepted then I can volunteer

[chkal]

@lincolnthree That do you think about replacing our custom logging abstraction with slf4j?

[chkal]

I talked with @lincolnthree about this topic. We decided to keep ocpsoft-logging for now. We think that ocpsoft-logging supports a better "getting started experience", because it supports the default case, which is using JUL, out of the box and can nevertheless be configured to delegate to any other logging framework. Also we both had slf4j version issues in the past and therefore think that users should explicitly have to choose slf4j as the logging backend if they want to.

However, I released a new version of ocpsoft-logging a few days ago. This version fixes some potential performance issue. So the latest version should have basically the same performance characteristics as slf4j, even if you delegate to slf4j. Mainly because we do the same things as slf4j to work around unnecessary string construction.