Compatibility test JDK 17 (#2184)

* enable jdk 17 + disable 15,16 * Adding utility to mock environment variables * update war plugin * update maven to 3.8.3 * update javadoc plugin version * log4j1 broken on Java 17, just disable tests * add log4j1 instrumentation for test * fix log4j1 to keep testing it * fix typo * fix log correlaction plugin * fix logging activation listener test * fix the url connection plugin test Co-authored-by: eyalkoren <[email protected]>
elastic · Dec 6, 2021 · 50104e9 · 50104e9
1 parent 15e6803
commit 50104e9
Show file tree

Hide file tree

Showing 15 changed files with 217 additions and 120 deletions.
diff --git a/.mvn/wrapper/MavenWrapperDownloader.java b/.mvn/wrapper/MavenWrapperDownloader.java
@@ -1,34 +1,31 @@
 /*
-Licensed to the Apache Software Foundation (ASF) under one
-or more contributor license agreements.  See the NOTICE file
-distributed with this work for additional information
-regarding copyright ownership.  The ASF licenses this file
-to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
-with the License.  You may obtain a copy of the License at
-
-  http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing,
-software distributed under the License is distributed on an
-"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-KIND, either express or implied.  See the License for the
-specific language governing permissions and limitations
-under the License.
-*/
-
+ * Copyright 2007-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 import java.net.*;
 import java.io.*;
 import java.nio.channels.*;
 import java.util.Properties;
 
 public class MavenWrapperDownloader {
 
+    private static final String WRAPPER_VERSION = "0.5.6";
     /**
      * Default URL to download the maven-wrapper.jar from, if no 'downloadUrl' is provided.
      */
-    private static final String DEFAULT_DOWNLOAD_URL =
-            "https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.4.0/maven-wrapper-0.4.0.jar";
+    private static final String DEFAULT_DOWNLOAD_URL = "https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/"
+        + WRAPPER_VERSION + "/maven-wrapper-" + WRAPPER_VERSION + ".jar";
 
     /**
      * Path to the maven-wrapper.properties file, which might contain a downloadUrl property to
@@ -76,13 +73,13 @@ public static void main(String args[]) {
                 }
             }
         }
-        System.out.println("- Downloading from: : " + url);
+        System.out.println("- Downloading from: " + url);
 
         File outputFile = new File(baseDirectory.getAbsolutePath(), MAVEN_WRAPPER_JAR_PATH);
         if(!outputFile.getParentFile().exists()) {
             if(!outputFile.getParentFile().mkdirs()) {
                 System.out.println(
-                        "- ERROR creating output direcrory '" + outputFile.getParentFile().getAbsolutePath() + "'");
+                        "- ERROR creating output directory '" + outputFile.getParentFile().getAbsolutePath() + "'");
             }
         }
         System.out.println("- Downloading to: " + outputFile.getAbsolutePath());
@@ -98,6 +95,16 @@ public static void main(String args[]) {
     }
 
     private static void downloadFileFromURL(String urlString, File destination) throws Exception {
+        if (System.getenv("MVNW_USERNAME") != null && System.getenv("MVNW_PASSWORD") != null) {
+            String username = System.getenv("MVNW_USERNAME");
+            char[] password = System.getenv("MVNW_PASSWORD").toCharArray();
+            Authenticator.setDefault(new Authenticator() {
+                @Override
+                protected PasswordAuthentication getPasswordAuthentication() {
+                    return new PasswordAuthentication(username, password);
+                }
+            });
+        }
         URL website = new URL(urlString);
         ReadableByteChannel rbc;
         rbc = Channels.newChannel(website.openStream());

diff --git a/.mvn/wrapper/maven-wrapper.jar b/.mvn/wrapper/maven-wrapper.jar
diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties
@@ -1 +1,2 @@
-distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.6.3/apache-maven-3.6.3-bin.zip
+distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.3/apache-maven-3.8.3-bin.zip
+wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -333,7 +333,8 @@ pipeline {
           axis {
             // the list of support java versions can be found in the infra repo (ansible/roles/java/defaults/main.yml)
             name 'JAVA_VERSION'
-            values 'openjdk12', 'openjdk13', 'openjdk14', 'openjdk15', 'openjdk16'
+            values 'openjdk12', 'openjdk13', 'openjdk14', 'openjdk17'
+
           }
         }
         stages {

diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/ErrorLoggingListener.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/ErrorLoggingListener.java
@@ -30,7 +30,7 @@ public class ErrorLoggingListener extends AgentBuilder.Listener.Adapter {
     @Override
     public void onError(String typeName, ClassLoader classLoader, JavaModule module, boolean loaded, Throwable throwable) {
         if (throwable instanceof MinimumClassFileVersionValidator.UnsupportedClassFileVersionException) {
-            logger.warn("{} uses an unsupported class file version (pre Java {}}) and can't be instrumented. " +
+            logger.warn("{} uses an unsupported class file version (pre Java {})) and can't be instrumented. " +
                 "Consider updating to a newer version of that library.",
                 typeName,
                 ((MinimumClassFileVersionValidator.UnsupportedClassFileVersionException)throwable).getMinVersion());

diff --git a/...ent-core/src/test/java/co/elastic/apm/agent/testinstr/OptionConverterInstrumentation.java b/...ent-core/src/test/java/co/elastic/apm/agent/testinstr/OptionConverterInstrumentation.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.testinstr;
+
+import co.elastic.apm.agent.sdk.ElasticApmInstrumentation;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+import javax.annotation.Nullable;
+import java.util.Collection;
+import java.util.Collections;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+
+/**
+ * Instruments {@link org.apache.log4j.helpers.OptionConverter} to make {@link org.apache.log4j.helpers.Loader} work as
+ * expected on Java 17+. As log4j1 is now EOL http://logging.apache.org/log4j/1.2/ it's the best way to keep our tests
+ * active and relevant on this feature.
+ */
+public class OptionConverterInstrumentation extends ElasticApmInstrumentation {
+
+    @Override
+    public ElementMatcher<? super TypeDescription> getTypeMatcher() {
+        return named("org.apache.log4j.helpers.OptionConverter");
+    }
+
+    @Override
+    public ElementMatcher<? super MethodDescription> getMethodMatcher() {
+        return named("getSystemProperty");
+    }
+
+    @Override
+    public Collection<String> getInstrumentationGroupNames() {
+        return Collections.emptyList();
+    }
+
+    public static class AdviceClass {
+
+        @Advice.AssignReturned.ToReturned
+        @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
+        public static String onExit(@Advice.Argument(0) String key,
+                                    @Advice.Return @Nullable String returnValue) {
+
+            if (returnValue == null || !"java.version".equals(key)) {
+                return returnValue;
+            }
+
+            if (returnValue.indexOf('.') < 0) {
+                // just convert '17' to '17.0' to make Log4j simple version parsing work and not assume Java 1.x
+                return returnValue + ".0";
+            } else {
+                return returnValue;
+            }
+        }
+    }
+}
diff --git a/...og-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerIT.java b/...og-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerIT.java
@@ -18,28 +18,15 @@
  */
 package co.elastic.apm.agent.mdc;
 
-import co.elastic.apm.agent.MockReporter;
-import co.elastic.apm.agent.bci.ElasticApmAgent;
-import co.elastic.apm.agent.configuration.SpyConfiguration;
-import co.elastic.apm.agent.errorlogging.Log4j2LoggerErrorCapturingInstrumentation;
-import co.elastic.apm.agent.errorlogging.Slf4jLoggerErrorCapturingInstrumentation;
-import co.elastic.apm.agent.impl.ElasticApmTracerBuilder;
-import co.elastic.apm.agent.impl.ElasticApmTracer;
+import co.elastic.apm.agent.AbstractInstrumentationTest;
 import co.elastic.apm.agent.impl.transaction.Transaction;
 import co.elastic.apm.agent.logging.LoggingConfiguration;
-import net.bytebuddy.agent.ByteBuddyAgent;
 import org.apache.logging.log4j.ThreadContext;
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.Assumptions;
-import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.stubbing.Answer;
 import org.slf4j.Logger;
 import org.slf4j.MDC;
-import org.stagemonitor.configuration.ConfigurationRegistry;
-
-import java.util.Arrays;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
@@ -48,31 +35,12 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-class MdcActivationListenerIT {
+class MdcActivationListenerIT extends AbstractInstrumentationTest {
 
-    protected static ElasticApmTracer tracer;
-    protected static MockReporter reporter;
-    protected static ConfigurationRegistry config;
     private LoggingConfiguration loggingConfiguration;
 
-    @BeforeAll
-    static void beforeAll() {
-        reporter = new MockReporter();
-        config = SpyConfiguration.createSpyConfig();
-        tracer = new ElasticApmTracerBuilder()
-            .configurationRegistry(config)
-            .reporter(reporter)
-            .buildAndStart();
-        ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install(), Arrays.asList(new Slf4jLoggerErrorCapturingInstrumentation(), new Log4j2LoggerErrorCapturingInstrumentation()));
-    }
-
-    @AfterAll
-    static void afterAll() {
-        ElasticApmAgent.reset();
-    }
-
     @BeforeEach
-    void setUp() throws Exception {
+    void setUp() {
         MDC.clear();
         org.apache.log4j.MDC.clear();
         ThreadContext.clearAll();
@@ -114,10 +82,6 @@ void testVerifyThatWithEnabledCorrelationAndLoggedErrorMdcErrorIdIsNotBlankWithS
 
     @Test
     void testVerifyThatWithEnabledCorrelationAndLoggedErrorMdcErrorIdIsNotBlankWithLog4j() {
-        Assumptions.assumeTrue(() -> {
-            org.apache.log4j.MDC.put("test", true);
-            return org.apache.log4j.MDC.get("test") == Boolean.TRUE;
-        }, "Log4j MDC is not working, this happens with some versions of Java 10 where log4j thinks it's Java 1");
         when(loggingConfiguration.isLogCorrelationEnabled()).thenReturn(true);
         org.apache.logging.log4j.Logger logger = mock(org.apache.logging.log4j.Logger.class);
         doAnswer(invocation -> assertMdcErrorIdIsNotEmpty()).when(logger).error(anyString(), any(Exception.class));
@@ -135,10 +99,6 @@ void testVerifyThatWithEnabledCorrelationAndLoggedErrorMdcErrorIdIsNotBlankWithL
 
     @Test
     void testVerifyThatWithEnabledCorrelationAndLoggedErrorMdcErrorIdIsNotBlankWithLog4jNotInTransaction() {
-        Assumptions.assumeTrue(() -> {
-            org.apache.log4j.MDC.put("test", true);
-            return org.apache.log4j.MDC.get("test") == Boolean.TRUE;
-        }, "Log4j MDC is not working, this happens with some versions of Java 10 where log4j thinks it's Java 1");
         when(loggingConfiguration.isLogCorrelationEnabled()).thenReturn(true);
         org.apache.logging.log4j.Logger logger = mock(org.apache.logging.log4j.Logger.class);
         doAnswer(invocation -> assertMdcErrorIdIsNotEmpty()).when(logger).error(anyString(), any(Exception.class));

diff --git a/...-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java b/...-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java
@@ -54,6 +54,9 @@ class MdcActivationListenerTest extends AbstractInstrumentationTest {
     void setUp() {
         org.apache.log4j.MDC.put("test", true);
         log4jMdcWorking = (Boolean) org.apache.log4j.MDC.get("test");
+        assertThat(log4jMdcWorking)
+            .describedAs("MDC not working as expected")
+            .isNotNull();
 
         forAllMdc(MdcImpl::clear);
         loggingConfiguration = config.getConfig(LoggingConfiguration.class);

diff --git a/...n/src/test/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/...n/src/test/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation
@@ -0,0 +1,2 @@
+# Allows to make Log4j 1.x work on Java 17 even if not supported
+co.elastic.apm.agent.testinstr.OptionConverterInstrumentation
diff --git a/...n-common/src/test/java/co/elastic/apm/agent/log/shader/LogShadingInstrumentationTest.java b/...n-common/src/test/java/co/elastic/apm/agent/log/shader/LogShadingInstrumentationTest.java
@@ -344,10 +344,12 @@ private void verifyEcsFormat(String[] splitRawLogLine, JsonNode ecsLogLineTree,
         assertThat(ecsLogLineTree.get("event.dataset").textValue()).isEqualTo(serviceName + ".FILE");
         assertThat(ecsLogLineTree.get("service.version").textValue()).isEqualTo("v42");
         assertThat(ecsLogLineTree.get("some.field").textValue()).isEqualTo("some-value");
+        JsonNode jsonTraceId = ecsLogLineTree.get("trace.id");
         if (traceId != null) {
-            assertThat(ecsLogLineTree.get("trace.id").textValue()).isEqualTo(traceId);
+            assertThat(jsonTraceId).isNotNull();
+            assertThat(jsonTraceId.asText()).isEqualTo(traceId);
         } else {
-            assertThat(ecsLogLineTree.get("trace.id")).isNull();
+            assertThat(jsonTraceId).isNull();
         }
     }
 

diff --git a/...n/src/test/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/...n/src/test/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation
@@ -0,0 +1,2 @@
+# Allows to make Log4j 1.x work on Java 17 even if not supported
+co.elastic.apm.agent.testinstr.OptionConverterInstrumentation
diff --git a/...lugin/src/test/java/co/elastic/apm/agent/urlconnection/SSLContextInstrumentationTest.java b/...lugin/src/test/java/co/elastic/apm/agent/urlconnection/SSLContextInstrumentationTest.java
@@ -20,59 +20,51 @@
 
 import co.elastic.apm.agent.AbstractInstrumentationTest;
 import co.elastic.apm.agent.util.ExecutorUtils;
-import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import javax.net.ssl.HttpsURLConnection;
-import java.io.IOException;
-import java.lang.reflect.Field;
-import java.net.URL;
-import java.net.URLConnection;
+import javax.net.SocketFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocketFactory;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadPoolExecutor;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
 class SSLContextInstrumentationTest extends AbstractInstrumentationTest {
 
-    private static Field defaultSSLSocketFactoryField;
-    private static ThreadPoolExecutor elasticApmThreadPool = ExecutorUtils.createSingleThreadSchedulingDaemonPool("HttpsUrlConnection-Test");
+    // We can't really assert that the agent does not initialize the SSL context, but we can test how it's implemented
+    // thus here we just check that trying to get default SSL context/factories is not possible from agent threads.
 
-    @BeforeAll
-    static void setup() throws Exception {
-        defaultSSLSocketFactoryField = HttpsURLConnection.class.getDeclaredField("defaultSSLSocketFactory");
-        defaultSSLSocketFactoryField.setAccessible(true);
-        defaultSSLSocketFactoryField.set(null, null);
-    }
-
-    @BeforeEach
-    void resetState() throws Exception {
-        defaultSSLSocketFactoryField.set(null, null);
-    }
+    private static final ThreadPoolExecutor elasticApmThreadPool = ExecutorUtils.createSingleThreadSchedulingDaemonPool("HttpsUrlConnection-Test");
 
     @Test
-    void testNonSkipped() throws Exception {
-        createConnection();
-        Object defaultSslFactory = defaultSSLSocketFactoryField.get(null);
-        assertThat(defaultSslFactory).isNotNull();
-        assertThat(defaultSslFactory).isEqualTo(HttpsURLConnection.getDefaultSSLSocketFactory());
+    void testNonSkipped() {
+        createConnection(false);
     }
 
     @Test
     void testSkipped() throws Exception {
-        Future<?> connectionCreationFuture = elasticApmThreadPool.submit(this::createConnection);
+        Future<?> connectionCreationFuture = elasticApmThreadPool.submit(()-> createConnection(true));
         connectionCreationFuture.get();
-        assertThat(defaultSSLSocketFactoryField.get(null)).isNull();
-        createConnection();
-        assertThat(defaultSSLSocketFactoryField.get(null)).isNotNull();
     }
 
-    private void createConnection() {
+    private void createConnection(boolean expectNull) {
         try {
-            URLConnection urlConnection = new URL("https://localhost:11111").openConnection();
-            assertThat(urlConnection).isInstanceOf(HttpsURLConnection.class);
-        } catch (IOException e) {
+            SocketFactory defaultSslSocketFactory = SSLSocketFactory.getDefault();
+            SSLContext defaultSslContext = SSLContext.getDefault();
+            SocketFactory defaultSocketFactory = SocketFactory.getDefault();
+
+            if (expectNull) {
+                assertThat(defaultSslSocketFactory).isNull();
+                assertThat(defaultSslContext).isNull();
+                assertThat(defaultSocketFactory).isNull();
+            } else {
+                assertThat(defaultSslSocketFactory).isNotNull();
+                assertThat(defaultSslContext).isNotNull();
+                assertThat(defaultSocketFactory).isNotNull();
+            }
+
+        } catch (Exception e) {
             throw new RuntimeException(e);
         }
     }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		# Allows to make Log4j 1.x work on Java 17 even if not supported
		co.elastic.apm.agent.testinstr.OptionConverterInstrumentation