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]>

Author: SylvainJuge

.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());

.mvn/wrapper/maven-wrapper.jar

@@ -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

.mvn/wrapper/maven-wrapper.properties

@@ -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 {

Jenkinsfile

@@ -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());

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/ErrorLoggingListener.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;
+            }
+        }
+    }
+}

apm-agent-core/src/test/java/co/elastic/apm/agent/testinstr/OptionConverterInstrumentation.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));

apm-agent-plugins/apm-log-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerIT.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);

apm-agent-plugins/apm-log-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerTest.java

@@ -0,0 +1,2 @@
+# Allows to make Log4j 1.x work on Java 17 even if not supported
+co.elastic.apm.agent.testinstr.OptionConverterInstrumentation

apm-agent-plugins/apm-log-correlation-plugin/src/test/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation

@@ -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();
         }
     }