-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29770 Exclude commons-logging from HBase #7539
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
base: master
Are you sure you want to change the base?
Conversation
|
Since these libraries use commons-logging as their logging facade, excluding commons-logging may introduce troubles like ClassNotFoundException? And at least, we can not see the logs of these libraries? |
This comment has been minimized.
This comment has been minimized.
Not if the the jcl-over-slf4j jar is present on the classpath (which already is). The jcl-over-slf4j jar implements the commons-logging API. We may have such problems if jcl-over-slf4j is omitted from the classpath. The Hadoop-common dependency is actually via commons-configuration2, but we don't have that dependency, hence I added the exclusion a level higher.
In my experience commons-logging logs are usually lost, or end up in some unexpeced place because HBase does not include commons-logging configuration. Using jcl-over-slf4j ensures that the commons-logging messages get piped into SLF4j and from there to Log4j2. We've done the same in Phoenix some time ago, and tested that the commons-logging components' logs are logged via slf4j->log4j2 (there are very few, we had to work to trigger them) |
This comment has been minimized.
This comment has been minimized.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Jcl-over-slf4j is just a commons-logging implementation based on slf4j, so we still need the commons-logging facade? IIRC, commons-logging is mostly like slf4j, usually you will use commons-logging and log4j together in the old time. So what is the problem of commons-logging dependency here? |
|
Ah, OK, checked the code, seems jcl-over-slf4j contains the class in commons-logging... |
|
Better also ban commons-logging import in the maven enforcer plugin? |
The commons-logging classes (along with log4j1 and log4j2 ) are already restricted. |
No description provided.