-
Notifications
You must be signed in to change notification settings - Fork 38.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider migrating from Commons-Logging to SLF4j [SPR-5327] #10000
Comments
Tim Wlcek commented As I have been spending the last two days trying to migrate my project to slf4j and to fix the transitive dependencies from maven, I would dearly love to see this issue resolved. At least consider marking clogging as optional in the spring poms. |
Carlos Zuniga commented Here is a stab at changing the logging framework from Commons Logging to SLF4J. Feedback is welcome. Carlos |
adrian commented JCL can be a pain to setup on Websphere, see https://www.ibm.com/developerworks/forums/thread.jspa?threadID=227753. Migrating to SLF4J would resolve this issue. Also SLF4J provides a migration utility which could be used all the spring code baseline (didn't tested it though) : http://www.slf4j.org/migrator.html |
Neale Upstone commented This doesn't need to be nailed down as JCL or SLF4J. The main problem here, for which I'm going to report a separate bug, is that the logging dependency is not left for the runtime to provide, but is instead pulled in as "compile" scope within the Spring 3.0M3 POMs.
Could read:
I agree that SLF4J is now a far better choice than JCL. It can work very nicely with Maven. |
Dave Syer commented The current proposal (for discussion) from the Spring team is that we simply switch to declaring jcl-slf4j as the primary dependency, requiring users to provide their own bindings from there. This makes it easy to integrate third-party libraries that already use slf4j without going to whole hog ourselves. Or we could leave things as they are, and allow people to adapt in that case by using the same bindings (log4j, JUL, etc.) to our commons-logging and the third-party slf4j. The suggestion from Neale that we simply make commons-logging a "provided" dependency is interesting, signalling that it is mandatory but must be provided by the host (who can choose jcl-slf4j if they wish). I would prefer to leave the primary logging dependency as "compile" scope, but it comes to the same thing really - when you run an application for the first time it will fail and tell you that there is a missing class. The one saving grace of commons-logging is that it doesn't do that, but of course that is also its downfall: the discovery process is flawed. Using "provided" scope doesn't make life any easier for WAS users. Only moving our primary dependency to sl4j would help, and even then it might not help everyone - presumably there are people who actually like having WAS control their logging configuration, and all they have to do is leave out commons-logging from their WAR/EAR packaging. |
Grzegorz Borkowski commented IMHO, the primary logging dependency of Spring should be switched to SLF4J. From all possible decisions about logging library choice, in year 2009 this one seems the most reasonable. |
Stevo Slavić commented See here how Jetty (maven plugin) did it. |
Dave Syer commented Stevo: I looked at what Jetty does, and we don't want to go there because it involves adding yet another logging abstraction with runtime classpath discovery (like commons-logging but smarter). Grzegorz: SLF4J is undoubtedly superior to commons-logging, but the basic JCL API that we use in Spring by and large is not so evil. And we have a strong commitment to backwards compatibility: i.e. most if not all applications should work just by upgrading the version of Spring. Admittedly there has never been a concerted effort to match that commitment with our declared dependency management artifacts (POMs etc.), but that's no reason not to consider the general principle. I actually think now that the best thing we can do is leave commons-logging as a primary compile-scoped dependency, but only in spring-core. Features of that approach are:
\ |
allnightlong commented In my opinion, major release is the best moment to switch to modern slf4j. If spring wouldn't do this, all community have to wait until the next major release (about 3 years?), for another chance. |
Grzegorz Borkowski commented Keeping backward compatibility is an important point, and this must be seriously considered.
The same code using SLF4J would look like:
I've seen some projects where "if isDebugEnabled" was the most repeatable pattern around the code, that's why I'm allergic to it :) This means that new Spring code will be able to use this approach, which is better for code readability. |
Juergen Hoeller commented Actually, we do expose a JCL Log in plenty of Spring-provided base classes... Changing the type of that field to an SFL4J logger would break binary compatibility with external subclasses using that logger, not only with user code but also with third-party libraries that are built on Spring (and can't be recompiled as easily as user code). Juergen |
adrian commented
Ok, this is a problem, but perhaps with version 3, you can allow yourself to break compatibility (and use private modifier for SLF4J logger attributes) ? [1] Some projects of those projects :
Other projects not using SLF4J
Perhaps contacting main open source projects for a 'global' switch to SLF4J would be a good idea (struts2, wicket, gwt, ...) ?
I thought this issue was more about modifying spring source dependency towards jcl. Everyone can already use Spring 2.5.6 with jcl-slf4j (I'm already using this setup in my projects).
It would enable some of us to setup spring application with parent_first classloader setting (I'm tied to parent_last since I'm using jcl-slf4j)... until WAS also switches to from JCL to SLF4J) |
Grzegorz Borkowski commented
Well, if that's the case, than situation is much more difficult. I've never used such Spring logger, but many others could have done it. Spring always focused on keeping backward compatibility, so I understand that changing all loggers to SLF4J is problematic. One choice is to break this compatibility, as it was suggested by others - major releases theoretically allow for this; besides, the change is not that big, and other libraries can be quite easily aligned. Spring already did some breaking compatibility change in 3.0, by changing naming scheme of modules (for maven/ivy), which cases problems in some cases. However, if you decide that you simply can't do it, than we have to use some of the approaches proposed above, like e.g. restrictive dependency on JCL to spring-core, ... others? |
Dave Syer commented Resolving this one as Won't Fix, but this is a good discussion. I have captured some of it in a new section of the user guide. |
Jon Fisher commented So to summarize, the only reason for not migrating is backwards compatibility, is this correct? :( |
Dave Syer commented Yes, that's it in a nutshell. Commons logging as an implementation has its problems but the API is good enough for our purposes so it doesn't make sense to break anything. The JCL-SLF4J bridge is a good option for people who want to mix and match with SLF4J/ |
Keith Donald commented Examples of projects with Maven POMs that pull down SLF4J with the commons-logging bridge instead are here: |
Neale Upstone commented I didn't look back on this at the time, but I'll make another play for htis on 3.1. At the moment, we still have 3.1 with Commons Logging in compile scope. While it won't make life any easier for those on Websphere, can we please change to provided? Maven POMs are verbose enough without being cluttered with every Spring dep with an exclusion for CL |
Dave Syer commented Sorry Neale, but I don't think it's going to happen - we don't want WAS users to be the only group for whom Spring works out of the box. You should only need an exclusion for spring-core, as that's the only POM that should depend on commons-logging. If that's not the case you can raise a separate JIRA ticket. |
Neale Upstone commented Ah. Had that lightbulb moment (ding!), which I'll share for all, and.. I suggest should be the standard approach for Spring examples and for Spring Roo. The wrong way of fighting to exclude SLF4J is as demonstrated in https://src.springframework.org/svn/spring-samples/mvc-ajax/trunk, which is to include a maven artifact, and exclude it. You can end up with something like this, which works. It makes sense that any single dependency recording an exclusion should exclude for all.
This is unclear, and for anyone who doesn't know the detailed black box magic of Maven's internals, they may duplicate these (just create a vanilla project under Spring Roo 1.1.2 and you'll see this demonstrated). The clear way of expressing what Dave has clarified above (that it only needs excluding on spring-core), is to use dependencyManagement:
Simpler still, for a multi-module project, the dependencyManagemnt entry would go in a parent POM. |
Juergen Hoeller commented FYI, and FWIW, Hibernate 4.0 migrates away from SLF4J and uses their own JBoss Logging now. So it doesn't look like common Java open source projects actually agree on SLF4J on the way forward. Juergen |
Aaron Digulla commented That isn't entirely correct. Someone seems to love the JUL API so much that they wrapped it in Java 5 (varargs support) and wrote a couple of plugins to translate it to various logging frameworks (JUL, log4j and slf4j are supported even though slf4j isn't mentioned in the docs). It's in 4.0 release the POM as a test dependency, though. The code looks fast enough but I'm not sure how much value you get from adding another log-layer on top of the existing ones. Since I can't find any pointers why they did it, my guess is currently that the JBoss logging layer allows to i18n the log messages. Great if you need it but I'm wary of the performance impact. That said, I'd still vote to replace JCL with slf4j for these reasons:
For example: I've written an appender with logback that logs nothing until an error happens. At that time, it will log the last 20 INFO messages and the last 200 DEBUG messages. The whole code is about 250 lines. I get a log file which only contains interesting information plus a fast logger. To drive this home: I'm patching my third-party dependencies to use slf4j if they depend on JUL. The last bigger project on my list was BIRT (patched 400+ files). ZK is probably next. Spring uses JCL, which is not a very big problem because I can use the slf4j bridge which causes only a small performance loss but I always prefer frameworks that don't tie my to a certain logging implementation at runtime. The problem with JUL is that the translation is very, very expensive. |
Adriano Machado commented
|
Hendy Irawan commented Can this be reconsidered? While it seems late, it's not late for next major version of Spring (i.e. 5.0) which presumably will have Java 9 support. I'd vote for replacing The deprecation can start in 4.1.x or 4.2.x and legacy support removed in Spring 5.0 or Spring 6.0. To add weight to this issue, most Spring-related projects already use SLF4J API anyway, see LDAP-273, DATAREST-54, DATAMONGO-391, etc. #11273 refers to this. |
Grzegorz Borkowski opened SPR-5327 and commented
Currently Spring Framework depends on Commons-Logging. All of us know that for long time Commons-Logging has very bad reputation and is the source of many problems, bug, memory leaks etc.
The SLF4j is the newer implementation of similar idea, and many people claim it works much better than Commons-Logging. It contain even some adapter for Commons-Logging users, allowing for gradual transition. Perhaps with Spring 3, along with migration to Java 5, is the best time to migrate also to SLF4J and remove dependency on Commons-Logging?
In fact, you can probably do it somehow automatically, writing the script that will replace in all java files the imports and logger types.
Affects: 3.0 M4
Attachments:
Issue Links:
Referenced from: commits f4763a8, 1a06b6a, ad3fa50, 604a9f0, b7e37dd, 1202f67, 0400ccc
20 votes, 13 watchers
The text was updated successfully, but these errors were encountered: