client_java icon indicating copy to clipboard operation
client_java copied to clipboard

Add log4j2 annotation processor so packages is not required

Open rquinio1A opened this issue 3 years ago • 6 comments
trafficstars

Currently it's necessary to pass the packages attributes via <Configuration packages="io.prometheus.client.log4j2"> in log4j configuration to discover the appender. That means relying on step 5 of https://logging.apache.org/log4j/2.x/manual/plugins.html for appender discovery (analyzing packages and annotations at startup).

It would seem less error prone and more efficient to add the log4j annotation processor to the Maven compiler plugin, so it builds the metadata META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat at build time (step1) ?

rquinio1A avatar Dec 29 '21 14:12 rquinio1A

Would that be something that users of simpleclient_log4j2 need to do, or is that something that we need to do when building simpleclient_log4j2?

fstab avatar Dec 30 '21 12:12 fstab

It would need to be added in the build of simpleclient_log4j2, so that simpleclient_log4j2.jar in Maven central contains the meatadata file META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat

rquinio1A avatar Dec 30 '21 12:12 rquinio1A

Thanks for the response. I didn't work with Log4j2Plugins.dat before. I'm wondering

  1. What happens if a user wants to create a uber-JAR with the Maven shade plugin, but multiple dependencies have the Log4j2Plugins.dat file? My guess is that this will fail, because the final JAR will only have one META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat and the Maven shade plugin cannot know which one to choose. That would mean that we break existing builds by adding Log4j2Plugins.dat to simpleclient_log4j2 if the existing build already uses another dependency that has a Log4j2Plugins.dat file.
  2. Even if a user does not create a uber-JAR but puts all dependencies in the classpath: Would log4j be able to load all Log4j2Plugins.dat from all dependencies? I guess if they load Log4j2Plugins.dat as a classpath resource they will just load the one from the JAR file that happens to be first in the classpath. If that's the case, it will be pretty random whether the Log4j2Plugins.dat from simpleclient_log4j2 gets picked up or not.

It would be great if you could elaborate a bit to help me understand how this will work.

fstab avatar Jan 03 '22 09:01 fstab

Good points, here's what I've found:

  1. For uber-jar we're using this transformer to merge the Log4j2Plugins.dat fields: https://github.com/edwgiz/maven-shaded-log4j-transformer. It's calling log4j-core APIs to do the merge and dump the resulting file.
  2. log4j2 itself is iterating on all the classloader resources and merging plugins in memory: https://github.com/apache/logging-log4j2/blob/f72100df0decc9bda96b4d769822c4e48b2848fc/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginRegistry.java#L162

rquinio1A avatar Jan 03 '22 10:01 rquinio1A

Thanks for looking into this. So it looks like (2) is not a problem.

However, (1) sounds like this would break existing builds if users create uber-JARs but didn't configure the log4j-transformer plugin. I imagine fixing the build will be non-trivial in some cases. Many frameworks like Spring inherit the Maven shade plugin from a parent POM, and it is not straightforward where to put the log4j-transformer plugin configuration.

fstab avatar Jan 03 '22 11:01 fstab

Using the packages attribute is now deprecated in log4j 2. The attribute will be removed in log4j 3. This should be fixed

lily-es avatar Jun 07 '23 14:06 lily-es