Use SLF4J instead of Log4J directly
Opened this issue · 15 comments
Having this depend specifically on Log4J can produce a weird situation when used in projects that use other logging frameworks (which is the problem SLF4J, or depending only on API jars, tackle).
As far as I could see, there isn't a lot of complex things going on here that would require direct access to some Log4J-specific features, so I would like to know if you'd be willing to migrate this to SLF4J (or Log4J2-API).
I can port it myself, but I wanted to know your positioning on the matter before forking and creating a PR
Yes, makes sense. Use of log4j is due package predating slf4j.
But do note that log4j is "provided" dependency, and not used by default.
So what might make more sense is to add another backend implementation for slf4j to go with existing log4j and java.util.logging varieties?
In my opinion, given what SLF4J is, the best, I think, would just to depend on slf4j-api for compilation, use only SLF4J, and let the user use some SLF4J implementation/bridge/whatever to use the logging framework of their choice.
The whole point of SLF4J is to not depend directly on logger implementation and let the user choose
Even though the dependency is in provided scope, IIRC, it still has to be in the classpath for compiling projects that depend on it.
I either have to add log4j or log4j-over-slf4j at compile time for to my projects, for example.
I think fully moving to SLF4J is the way to go (but then release the change as a major, potentially breaking change, I believe).
Looking into some other usages of this project here on GitHub, I see many people are doing log4j -> slf4j -> their logging framework of choice (same thing I'm doing right now)
I deleted my other GitHub account. Contact this account if you need.
Looks like there is a java modules problem with the log4j dependency.
The module-info.java file has 'requires log4j;'
That gives the following error when you try to add com.fasterxml.uuid as a java module.
"java.lang.module.FindException: Module log4j not found, required by com.fasterxml.uuid"
@hanson76 I'll test with jigsaw tomorrow. It makes sense to not require that anymore, also.
@hanson76 Could you test it with the latest version from my fork? See #33
@cowtowncoder I would like to know if the OSGi thing is important and if I did it correctly (I never did anything with OSGi before)
Sorry for late answer but been busy.
I've now tested your fork and it works where 3.2.0 does not.
@hanson76 does that include log messages from this library?
Yes
@cowtowncoder Is there a plan to merge the fix in #33 and release a new version anytime soon?
There one more good reason for the migration to SLF4J.
java-uuid-generator currently uses the log4j 1.2.17 dependency, which is vulnerable to https://nvd.nist.gov/vuln/detail/CVE-2019-17571, so migrating to another package which is able to use a newer version of log4j (i.e. 2.x) might be a good idea.
It seems there is a legitimate reason to migrate then, @cowtowncoder
That CVE may be a practical concern: from quick look it does not seem like actual security issue in itself or via JUG, but security tools whine incessantly about such dependency so as to make it a practical problem nonetheless (no, I am not big fan of current handling of CVEs as a maintainer).
Or put another way: dependency only becomes problem if a serialization framework enables deserialization of log4j types; in that case lack of log4j class can prevent that particular security hole. But there are literally dozens of others, a small number even in JDK itself.
Still, due to simplistic security evaluation tools this will no doubt be brought up again and again :-(
Having said that... My only concern so far has been that of trying to avoid adding strong dependency (i.e. being able to leave dependency as provided) -- possibly by actually simply dropping any implementation aside from JavaUtilLogger that relies on JDK's logger.
So perhaps the simplest and best course is to just move to using slf4j api; and since this is fundamental dependency change, increase major version to 4. I'll go over the PR and see if I some suggestions or questions.
Implemented, released as new major version, 4.0, available via Maven Central.