Fix log4j2 correlation with shaded log4j

CHANGELOG.asciidoc

@@ -31,6 +31,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
 
 === Unreleased
 
+[float]
+===== Bug fixes
+* Fix log4j2 log correlation with shaded application jar - {pull}3764[#3764]
+
 [[release-notes-1.x]]
 === Java Agent version 1.x
 

apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchers.java

@@ -19,9 +19,9 @@
 package co.elastic.apm.agent.sdk.bytebuddy;
 
 import co.elastic.apm.agent.sdk.internal.InternalUtil;
+import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils;
 import co.elastic.apm.agent.sdk.logging.Logger;
 import co.elastic.apm.agent.sdk.logging.LoggerFactory;
-import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils;
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
 import net.bytebuddy.description.NamedElement;
@@ -31,20 +31,21 @@
 import javax.annotation.Nullable;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.JarURLConnection;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.security.CodeSource;
 import java.security.ProtectionDomain;
 import java.util.Collection;
+import java.util.Properties;
 import java.util.jar.Attributes;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
+import java.util.zip.ZipEntry;
 
-import static net.bytebuddy.matcher.ElementMatchers.nameContains;
-import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
-import static net.bytebuddy.matcher.ElementMatchers.none;
+import static net.bytebuddy.matcher.ElementMatchers.*;
 
 public class CustomElementMatchers {
 
@@ -183,15 +184,23 @@ public static <T extends NamedElement> ElementMatcher.Junction<T> isProxy() {
      * @param version the version to check against
      * @return an LTE SemVer matcher
      */
-    public static ElementMatcher.Junction<ProtectionDomain> implementationVersionLte(final String version) {
-        return implementationVersion(version, Matcher.LTE);
+    public static ElementMatcher.Junction<ProtectionDomain> implementationVersionLte(String version) {
+        return implementationVersion(version, Matcher.LTE, null, null);
     }
 
-    public static ElementMatcher.Junction<ProtectionDomain> implementationVersionGte(final String version) {
-        return implementationVersion(version, Matcher.GTE);
+    public static ElementMatcher.Junction<ProtectionDomain> implementationVersionGte(String version) {
+        return implementationVersion(version, Matcher.GTE, null, null);
     }
 
-    private static ElementMatcher.Junction<ProtectionDomain> implementationVersion(final String version, final Matcher matcher) {
+    public static ElementMatcher.Junction<ProtectionDomain> implementationVersionLte(String version, String groupId, String artifactId) {
+        return implementationVersion(version, Matcher.LTE, groupId, artifactId);
+    }
+
+    public static ElementMatcher.Junction<ProtectionDomain> implementationVersionGte(String version, String groupId, String artifactId) {
+        return implementationVersion(version, Matcher.GTE, groupId, artifactId);
+    }
+
+    private static ElementMatcher.Junction<ProtectionDomain> implementationVersion(final String version, final Matcher matcher, @Nullable final String mavenGroupId, @Nullable final String mavenArtifactId) {
         return new ElementMatcher.Junction.AbstractBase<ProtectionDomain>() {
             /**
              * Returns true if the implementation version read from the manifest file referenced by the given
@@ -205,7 +214,7 @@ private static ElementMatcher.Junction<ProtectionDomain> implementationVersion(f
             @Override
             public boolean matches(@Nullable ProtectionDomain protectionDomain) {
                 try {
-                    Version pdVersion = readImplementationVersionFromManifest(protectionDomain);
+                    Version pdVersion = readImplementationVersion(protectionDomain, mavenGroupId, mavenArtifactId);
                     if (pdVersion != null) {
                         Version limitVersion = Version.of(version);
                         return matcher.match(pdVersion, limitVersion);
@@ -221,22 +230,51 @@ public boolean matches(@Nullable ProtectionDomain protectionDomain) {
     }
 
     @Nullable
-    private static Version readImplementationVersionFromManifest(@Nullable ProtectionDomain protectionDomain) throws IOException, URISyntaxException {
+    private static Version readImplementationVersion(@Nullable ProtectionDomain protectionDomain, @Nullable String mavenGroupId, @Nullable String mavenArtifactId) throws IOException, URISyntaxException {
         Version version = null;
         JarFile jarFile = null;
+
+        if (protectionDomain == null) {
+            logger.info("Cannot read implementation version - got null ProtectionDomain");
+            return null;
+        }
+
         try {
-            if (protectionDomain != null) {
-                CodeSource codeSource = protectionDomain.getCodeSource();
-                if (codeSource != null) {
-                    URL jarUrl = codeSource.getLocation();
-                    if (jarUrl != null) {
-                        // does not yet establish an actual connection
-                        URLConnection urlConnection = jarUrl.openConnection();
-                        if (urlConnection instanceof JarURLConnection) {
-                            jarFile = ((JarURLConnection) urlConnection).getJarFile();
-                        } else {
-                            jarFile = new JarFile(new File(jarUrl.toURI()));
+            CodeSource codeSource = protectionDomain.getCodeSource();
+            if (codeSource != null) {
+                URL jarUrl = codeSource.getLocation();
+                if (jarUrl != null) {
+                    // does not yet establish an actual connection
+                    URLConnection urlConnection = jarUrl.openConnection();
+                    if (urlConnection instanceof JarURLConnection) {
+                        jarFile = ((JarURLConnection) urlConnection).getJarFile();
+                    } else {
+                        jarFile = new JarFile(new File(jarUrl.toURI()));
+                    }
+
+                    // read maven properties first as they have higher priority over manifest
+                    // this is mostly for shaded libraries in "fat-jar" applications
+                    if (mavenGroupId != null && mavenArtifactId != null) {
+                        ZipEntry zipEntry = jarFile.getEntry(String.format("META-INF/maven/%s/%s/pom.properties", mavenGroupId, mavenArtifactId));
+                        if (zipEntry != null) {
+                            Properties properties = new Properties();
+                            try (InputStream input = jarFile.getInputStream(zipEntry)) {
+                                properties.load(input);
+                            }
+                            if (mavenGroupId.equals(properties.getProperty("groupId")) && mavenArtifactId.equals(properties.getProperty("artifactId"))) {
+                                String stringVersion = properties.getProperty("version");
+                                if (stringVersion != null) {
+                                    version = Version.of(stringVersion);
+                                }
+                            }
                         }
+                    }
+
+                    // reading manifest if library packaged as a jar
+                    //
+                    // doing this after maven properties is important as it might report executable jar version
+                    // when application is packaged as a "fat jar"
+                    if (version == null) {
                         Manifest manifest = jarFile.getManifest();
                         if (manifest != null) {
                             Attributes attributes = manifest.getMainAttributes();
@@ -248,12 +286,9 @@ private static Version readImplementationVersionFromManifest(@Nullable Protectio
                             if (manifestVersion != null) {
                                 version = Version.of(manifestVersion);
                             }
-
                         }
                     }
                 }
-            } else {
-                logger.info("Cannot read implementation version - got null ProtectionDomain");
             }
         } finally {
             if (jarFile != null) {

apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/CustomElementMatchersTest.java

@@ -18,20 +18,29 @@
  */
 package co.elastic.apm.agent.sdk.bytebuddy;
 
-import org.apache.http.client.HttpClient;
-
 import net.bytebuddy.description.type.TypeDescription;
+import org.apache.http.client.HttpClient;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 import java.io.File;
+import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.security.CodeSigner;
 import java.security.CodeSource;
 import java.security.ProtectionDomain;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static co.elastic.apm.agent.sdk.bytebuddy.CustomElementMatchers.*;
 import static net.bytebuddy.matcher.ElementMatchers.none;
@@ -63,6 +72,38 @@ void testSemVerLteWithEncodedFileUrl() throws MalformedURLException, URISyntaxEx
         assertThat(implementationVersionLte("2.1.9").matches(protectionDomain)).isTrue();
     }
 
+    @Test
+    void testSemVerFallbackOnMavenProperties(@TempDir Path tempDir) throws URISyntaxException, IOException {
+        // Relying on Apache httpclient-4.5.6.jar
+        // creates a copy of the jar without the manifest so we should parse maven properties
+        URL originalUrl = HttpClient.class.getProtectionDomain().getCodeSource().getLocation();
+        Path modifiedJar = tempDir.resolve("test.jar");
+        try {
+            Files.copy(Paths.get(originalUrl.toURI()), modifiedJar);
+
+            Map<String, String> fsProperties = new HashMap<>();
+            fsProperties.put("create", "false");
+
+            URI fsUri = URI.create("jar:file://" + modifiedJar.toAbsolutePath());
+            try (FileSystem zipFs = FileSystems.newFileSystem(fsUri, fsProperties)) {
+                Files.delete(zipFs.getPath("META-INF", "MANIFEST.MF"));
+            }
+
+            ProtectionDomain protectionDomain = new ProtectionDomain(new CodeSource(modifiedJar.toUri().toURL(), new CodeSigner[0]), null);
+
+            assertThat(implementationVersionLte("4.5.5", "org.apache.httpcomponents", "httpclient").matches(protectionDomain)).isFalse();
+            assertThat(implementationVersionLte("4.5.6", "org.apache.httpcomponents", "httpclient").matches(protectionDomain)).isTrue();
+            assertThat(implementationVersionGte("4.5.7", "org.apache.httpcomponents", "httpclient").matches(protectionDomain)).isFalse();
+
+
+        } finally {
+            if (Files.exists(modifiedJar)) {
+                Files.delete(modifiedJar);
+            }
+        }
+
+    }
+
     private void testSemVerLteMatcher(ProtectionDomain protectionDomain) {
         assertThat(implementationVersionLte("3").matches(protectionDomain)).isFalse();
         assertThat(implementationVersionLte("3.2").matches(protectionDomain)).isFalse();

apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2LogCorrelationInstrumentation.java

@@ -71,9 +71,11 @@ public ElementMatcher<? super MethodDescription> getMethodMatcher() {
     }
 
     public static class Log4J2_6LogCorrelationInstrumentation extends Log4j2LogCorrelationInstrumentation {
+
         @Override
         public ElementMatcher.Junction<ProtectionDomain> getProtectionDomainPostFilter() {
-            return implementationVersionGte("2.6").and(not(implementationVersionGte("2.7")));
+            return implementationVersionGte("2.6", "org.apache.logging.log4j", "log4j-core")
+                .and(not(implementationVersionGte("2.7", "org.apache.logging.log4j", "log4j-core")));
         }
 
         public static class AdviceClass {
@@ -94,7 +96,7 @@ public static void removeFromThreadContext(@Advice.Enter boolean addedToMdc) {
     public static class Log4J2_7PlusLogCorrelationInstrumentation extends Log4j2LogCorrelationInstrumentation {
         @Override
         public ElementMatcher.Junction<ProtectionDomain> getProtectionDomainPostFilter() {
-            return implementationVersionGte("2.7");
+            return implementationVersionGte("2.7", "org.apache.logging.log4j", "log4j-core");
         }
 
         public static class AdviceClass {