sofastack/sofa-registry

The usage of Log4j2 Strings Util class in the code makes users force dependency on log4j2.

Opened this issue · 4 comments

In what area(s)?

/area test-and-release

Describe the feature

Please use the commons-lang3 library or maintain self StringUtils class in the project.

I agree with you, we can use commons-lang3 StringUtils to replaces Strings in log4j-api, and upgrade commons-lang to commons-lang3 in the project.
Can you help to commit a pr to do this?

I agree with you, we can use commons-lang3 StringUtils to replaces Strings in log4j-api, and upgrade commons-lang to commons-lang3 in the project. Can you help to commit a pr to do this?

Done. PTAL

By the way, Is there a plan to publish new version to the Maven repository recently? The dubbo-spi-extension repository have some modules about sofa-registry and we has been doing version upgrading and writing unit tests. The is some problem encountered this issue when writing unit tests.

I agree with you, we can use commons-lang3 StringUtils to replaces Strings in log4j-api, and upgrade commons-lang to commons-lang3 in the project. Can you help to commit a pr to do this?

Done. PTAL

By the way, Is there a plan to publish new version to the Maven repository recently? The dubbo-spi-extension repository have some modules about sofa-registry and we has been doing version upgrading and writing unit tests. The is some problem encountered this issue when writing unit tests.

why use common-lang will cause problem when writing unit tests, what are the problems?

why use common-lang will cause problem when writing unit tests, what are the problems?

The main issue is the Strings reference to log4j. In the code left by previous developers, log4j was excluded and logback was introduced as an additional dependency.

https://github.com/apache/dubbo-spi-extensions/blob/9e99495f897493900f9b6ee2d961cf264484fd29/dubbo-registry-extensions/dubbo-registry-sofa/pom.xml#L101

It will cause java.lang.NoClassDefFoundError: org/apache/logging/log4j/spi/AbstractLoggerAdapter when used the TestRegistryMain.startRegistry() by the log4j-slf4j-impl, here is the stacktrace.

Failed to instantiate SLF4J LoggerFactory
Reported exception:
java.lang.NoClassDefFoundError: org/apache/logging/log4j/spi/AbstractLoggerAdapter
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
	at java.net.URLClassLoader.defineClass(URLClassLoader.java:473)
	at java.net.URLClassLoader.access$100(URLClassLoader.java:74)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:369)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:363)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.net.URLClassLoader.findClass(URLClassLoader.java:362)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:355)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
	at org.slf4j.impl.StaticLoggerBinder.<clinit>(StaticLoggerBinder.java:36)
	at org.slf4j.LoggerFactory.bind(LoggerFactory.java:150)
	at org.slf4j.LoggerFactory.performInitialization(LoggerFactory.java:124)
	at org.slf4j.LoggerFactory.getILoggerFactory(LoggerFactory.java:412)
	at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:357)
	at com.alipay.sofa.registry.log.SLF4JLogger.getLoggerBySpace(SLF4JLogger.java:105)
	at com.alipay.sofa.registry.log.SLF4JLogger.<init>(SLF4JLogger.java:74)
	at com.alipay.sofa.registry.log.LoggerFactory.getLogger(LoggerFactory.java:42)
	at com.alipay.sofa.registry.net.NetUtil.<clinit>(NetUtil.java:44)
	at com.alipay.sofa.registry.server.test.TestRegistryMain.<clinit>(TestRegistryMain.java:35)
	at org.apache.dubbo.registry.sofa.HelloServiceImpl.main(HelloServiceImpl.java:37)

While excluded the log4j-slf4j-impl it will cause another NoClassDefFoundError, here is the stacktrace.

java.lang.NoClassDefFoundError: org/apache/logging/log4j/util/Strings
	at com.alipay.sofa.registry.server.integration.RegistryApplication.executeSqlScript(RegistryApplication.java:287) ~[registry-server-integration-6.3.0.jar:na]
	at com.alipay.sofa.registry.server.integration.RegistryApplication.createTables(RegistryApplication.java:217) ~[registry-server-integration-6.3.0.jar:na]
	at com.alipay.sofa.registry.server.integration.RegistryApplication.main(RegistryApplication.java:89) ~[registry-server-integration-6.3.0.jar:na]
	at com.alipay.sofa.registry.server.test.TestRegistryMain.startRegistry(TestRegistryMain.java:51) ~[registry-test-6.3.0.jar:na]
	at org.apache.dubbo.registry.sofa.HelloServiceImpl.main(HelloServiceImpl.java:39) ~[test-classes/:na]

I can certainly remove the exclusion statements, but I would recommend not force users dependency on log4j2. I just upgraded commons-lang to commons-lang3 based on this suggestion.

and upgrade commons-lang to commons-lang3 in the project.

However, some modules in the project still has the commons-lang library such as registry-server-data, which is transitive dependency by jraft-core.

If upgrade commons-lang to commons-lang3 is not necessary, I will close this PR and recreate another pr.