Fix getLogger() incompatibility in log4j-1.2-api
#3030
Conversation
|
@taichi, can you help me to understand the "working legacy code" first, please? I created a new Maven project such that
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.example</groupId>
<artifactId>logging-log4j1</artifactId>
<version>1.0-SNAPSHOT</version>
<properties>
<maven.compiler.source>8</maven.compiler.source>
<maven.compiler.target>8</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
<dependencies>
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<version>1.2.17</version>
</dependency>
</dependencies>
</project>
package foo;
import org.apache.log4j.Logger;
public class MyLogger extends Logger {
protected MyLogger(String name) {
super(name);
}
public static MyLogger getLogger(Class clazz) {
return (MyLogger) getLogger(clazz.getName());
}
}And tried to use it in another Maven project such that
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>org.example</groupId>
<artifactId>logging-log4j1-usage</artifactId>
<version>1.0-SNAPSHOT</version>
<properties>
<maven.compiler.source>8</maven.compiler.source>
<maven.compiler.target>8</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
<dependencies>
<dependency>
<groupId>org.example</groupId>
<artifactId>logging-log4j1</artifactId>
<version>1.0-SNAPSHOT</version>
</dependency>
</dependencies>
</project>
package foo;
public class MyObject {
public static final MyLogger LOGGER = MyLogger.getLogger(MyObject.class);
public static void main(String[] args) {
LOGGER.info("foo");
}
}Though running (Using Azul Zulu 8 version Hence, I am not able to reproduce what you claimed it used to work when |
vy
added
api
|
Thank you for your reply. My sample code was intended to focus solely on the compilation errors. import org.apache.log4j.Logger;
import org.apache.log4j.spi.LoggerFactory;
public class MyLogger extends Logger {
static final LoggerFactory factory = new LoggerFactory() {
@Override
public Logger makeNewLoggerInstance(String name) {
return new MyLogger(name);
}
};
protected MyLogger(String name) {
super(name);
}
public static MyLogger getLogger(@SuppressWarnings("rawtypes") final Class clazz) {
return (MyLogger) Logger.getLogger(clazz.getName(), factory);
}
} |
Generated by gradle/develocity-actions |
|
@taichi, got it. Is the change set complete?I will accept this fix. Before merging your PR, could you verify that this is the only incompatibility you need to be fixed? Further fixesThere are several such incompatibilities between ./mvnw verify \
-pl :log4j-1.2-api \
-DskipTests -Dcyclonedx.skip -Dspotbugs.skip -Dspotless.skip -Denforce.skip -Dbnd.skip \
-Dbnd.baseline.base.coordinates=log4j:log4j:1.2.17 \
-Dbnd.baseline.full.reportand seeing the report dumped to the 1 See the |
|
Thank you for your thorough review and the detailed instructions for detecting additional changes. I can confirm that all necessary changes have been completed in this pull request. After reviewing the output, I can assure you that there are no other major incompatibilities that require addressing at this time. If you have any further questions or concerns, please don't hesitate to let me know. I'm ready to make any additional adjustments if needed. |
|
The breaking changes between I can not find any reference to the change in I am +1 on merging this PR, although I am surprised someone still uses Log4j 1 directly in their source code in 2024. @taichi, what features does |
|
@ppkarwasz Your observation is indeed valid and well-reasoned. Firstly, I would like to clarify that caller information is not being recorded in our logs at all, so any potential inaccuracies in this regard do not pose an issue. I must emphasize that my primary objective is to successfully migrate this legacy codebase, originally written circa 2007, to a modern environment while maintaining its correct functionality. It is anticipated that over the next few years, the code utilizing this Custom Logger will be gradually phased out. However, at this juncture, our capacity for modification is limited to partial replacement using the log4j-1.2-api. I appreciate your insights and would be happy to discuss any further questions or concerns you may have regarding this approach. |
If all it does it is add custom context data, you might consider implementing a custom If the context data is specific to a particular logger, please take a look at the discussion in #2214 and apache/logging-log4j-kotlin#71. We plan to introduce some new features in the Log4j API in the near future. |
vy
changed the title
getLogger() incompatibility in log4j-1.2-api
|
Thank you for guiding me to these discussions. As we have only recently begun using log4j2, this information is particularly valuable and insightful. Based on my current understanding, I believe it may be possible to effectively integrate a ContextDataProvider with either JAX-RS ContainerRequestFilters or CDI Interceptors to facilitate the population of ContextData. This approach could potentially offer a more elegant solution to our current implementation. |
This Pull Request addresses a type resolution issue that occurs when compiling legacy code using log4j-1.2-api with OpenJDK.
The problem arises from a subtle difference in method signatures between the getLogger methods defined in org.apache.log4j.Logger provided by log4j-1.2-api and the original log4j library.
Root Cause
The discrepancy lies in the method signatures of getLogger:
This difference becomes problematic when dealing with legacy code that includes custom Logger classes extending the original org.apache.log4j.Logger.
Example of Affected Legacy Code
Consider a custom MyLogger class:
Used in code like this:
When compiling with Java 21, the following error occurs:
This error occurs because the getLogger method in MyLogger lacks a type parameter, causing the compiler to select the getLogger method from org.apache.log4j.Logger instead.
Verification
To confirm this, modifying the MyLogger class as follows resolves the compilation issue:
This Pull Request implements a solution to address this type resolution problem, ensuring compatibility between legacy code using custom Logger extensions and the log4j-1.2-api library when compiling with OpenJDK.
Note
While we can rebuild the source code in our project, we cannot modify the existing legacy code itself.