From eabaeba435abf546cce256a13978250539fa7f98 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 24 Sep 2024 20:32:30 -0400 Subject: [PATCH 1/5] start converting internal tests --- .../src/test/groovy/RegressionTest.groovy | 19 ------ .../internal/classloader/RegressionTest.java | 19 ++++++ .../classloader/ResourceInjectionTest.java | 64 +++++++++++++++++++ 3 files changed, 83 insertions(+), 19 deletions(-) delete mode 100644 instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy create mode 100644 instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java create mode 100644 instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy deleted file mode 100644 index 8a4601927225..000000000000 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/RegressionTest.groovy +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification - -class RegressionTest extends AgentInstrumentationSpecification { - - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/5155 - // loading a class that is extended/implemented by a helper class causes - // java.lang.LinkageError: loader 'app' (instance of jdk.internal.loader.ClassLoaders$AppClassLoader) attempted duplicate interface definition for org.apache.commons.lang3.function.FailableCallable - // this test verifies that the duplicate class definition LinkageError is not thrown into - // application code - def "test no duplicate class definition"() { - expect: - Class.forName("org.apache.commons.lang3.function.FailableCallable") != null - } -} diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java new file mode 100644 index 000000000000..fd8022cb78f5 --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java @@ -0,0 +1,19 @@ +package io.opentelemetry.javaagent.instrumentation.internal.classloader; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +class RegressionTest { + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/5155 + // loading a class that is extended/implemented by a helper class causes + // java.lang.LinkageError: loader 'app' (instance of jdk.internal.loader.ClassLoaders$AppClassLoader) + // attempted duplicate interface definition for org.apache.commons.lang3.function.FailableCallable + // this test verifies that the duplicate class definition LinkageError is not thrown into + // application code + @Test + void noDuplicateClassDefinition() throws ClassNotFoundException { + assertThat(Class.forName("org.apache.commons.lang3.function.FailableCallable")).isNotNull(); + } + +} diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java new file mode 100644 index 000000000000..4f62f46fcb02 --- /dev/null +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java @@ -0,0 +1,64 @@ +package io.opentelemetry.javaagent.instrumentation.internal.classloader; + +import org.junit.jupiter.api.Test; +import java.io.IOException; +import java.lang.ref.WeakReference; +import java.net.URL; +import java.net.URLClassLoader; +import java.time.Duration; +import java.util.Enumeration; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; + +import static io.opentelemetry.instrumentation.test.utils.GcUtils.awaitGc; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + + +class ResourceInjectionTest { + + @Test + @SuppressWarnings("UnnecessaryAsync") + void resourcesInjectedToNonDelegatingClassLoader() + throws IOException, ClassNotFoundException, InterruptedException, TimeoutException { + String resourceName = "test-resources/test-resource.txt"; + URL[] urls = {ResourceInjectionTest.class.getProtectionDomain().getCodeSource().getLocation()}; + AtomicReference emptyLoader = new AtomicReference<>(new URLClassLoader(urls, null)); + + Enumeration resourceUrls = emptyLoader.get().getResources(resourceName); + assertThat(resourceUrls.hasMoreElements()).isFalse(); + + URLClassLoader notInjectedLoader = new URLClassLoader(urls, null); + + // this triggers resource injection + emptyLoader.get().loadClass(ResourceInjectionTest.class.getName()); + + for (int i = 0; i < 2; i++) { + + URL test = ( resourceUrls.asIterator(); + if (i == 0) { + assertThat(test).isEqualTo("Hello world!"); + } else { + assertThat(test).isEqualTo("Hello there"); + } + } + assertThat(resourceUrls.hasMoreElements()).isFalse(); + + +// resourceUrls = (Enumeration) Collections.list(emptyLoader.get().getResources(resourceName)); +// assertThat(resourceUrls.).isEqualTo(2); +// assertThat(list.get(0).openStream().toString().trim()).isEqualTo("Hello world!"); +// assertThat(list.get(1).openStream().toString().trim()).isEqualTo("Hello there"); + + assertThat(notInjectedLoader.getResources(resourceName).hasMoreElements()).isFalse(); + + // references to emptyloader are gone + emptyLoader.get().close(); + WeakReference ref = new WeakReference<>(emptyLoader.get()); + emptyLoader.set(null); + + awaitGc(ref, Duration.ofSeconds(10)); + + assertThat(ref.get()).isNull(); + } + +} From b81f295daba66afdacb2239522fbd0bb9b2df906 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 31 Dec 2024 08:05:32 -0500 Subject: [PATCH 2/5] convert, still debugging --- .../test/groovy/ResourceInjectionTest.groovy | 53 ---------------- .../internal/classloader/RegressionTest.java | 13 ++-- .../classloader/ResourceInjectionTest.java | 62 ++++++++++--------- 3 files changed, 42 insertions(+), 86 deletions(-) delete mode 100644 instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/ResourceInjectionTest.groovy diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/ResourceInjectionTest.groovy b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/ResourceInjectionTest.groovy deleted file mode 100644 index b5b3d799c496..000000000000 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/groovy/ResourceInjectionTest.groovy +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification -import org.apache.commons.lang3.SystemUtils - -import java.lang.ref.WeakReference -import java.time.Duration -import java.util.concurrent.atomic.AtomicReference - -import static io.opentelemetry.instrumentation.test.utils.GcUtils.awaitGc - -class ResourceInjectionTest extends AgentInstrumentationSpecification { - - def "resources injected to non-delegating class loader"() { - setup: - String resourceName = 'test-resources/test-resource.txt' - URL[] urls = [SystemUtils.getProtectionDomain().getCodeSource().getLocation()] - AtomicReference emptyLoader = new AtomicReference<>(new URLClassLoader(urls, (ClassLoader) null)) - - when: - def resourceUrls = emptyLoader.get().getResources(resourceName) - then: - !resourceUrls.hasMoreElements() - - when: - URLClassLoader notInjectedLoader = new URLClassLoader(urls, (ClassLoader) null) - - // this triggers resource injection - emptyLoader.get().loadClass(SystemUtils.getName()) - - resourceUrls = Collections.list(emptyLoader.get().getResources(resourceName)) - - then: - resourceUrls.size() == 2 - resourceUrls.get(0).openStream().text.trim() == 'Hello world!' - resourceUrls.get(1).openStream().text.trim() == 'Hello there' - - !notInjectedLoader.getResources(resourceName).hasMoreElements() - - when: "references to emptyLoader are gone" - emptyLoader.get().close() // cleanup - def ref = new WeakReference(emptyLoader.get()) - emptyLoader.set(null) - - awaitGc(ref, Duration.ofSeconds(10)) - - then: "HelperInjector doesn't prevent it from being collected" - null == ref.get() - } -} diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java index fd8022cb78f5..a5b5cdd8e2a3 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/RegressionTest.java @@ -1,13 +1,19 @@ -package io.opentelemetry.javaagent.instrumentation.internal.classloader; +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ -import org.junit.jupiter.api.Test; +package io.opentelemetry.javaagent.instrumentation.internal.classloader; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import org.junit.jupiter.api.Test; + class RegressionTest { // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/5155 // loading a class that is extended/implemented by a helper class causes - // java.lang.LinkageError: loader 'app' (instance of jdk.internal.loader.ClassLoaders$AppClassLoader) + // java.lang.LinkageError: loader 'app' (instance of + // jdk.internal.loader.ClassLoaders$AppClassLoader) // attempted duplicate interface definition for org.apache.commons.lang3.function.FailableCallable // this test verifies that the duplicate class definition LinkageError is not thrown into // application code @@ -15,5 +21,4 @@ class RegressionTest { void noDuplicateClassDefinition() throws ClassNotFoundException { assertThat(Class.forName("org.apache.commons.lang3.function.FailableCallable")).isNotNull(); } - } diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java index 4f62f46fcb02..5618aee7b3e3 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java @@ -1,28 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.javaagent.instrumentation.internal.classloader; -import org.junit.jupiter.api.Test; -import java.io.IOException; +import static io.opentelemetry.instrumentation.test.utils.GcUtils.awaitGc; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +import java.io.BufferedReader; +import java.io.InputStreamReader; import java.lang.ref.WeakReference; import java.net.URL; import java.net.URLClassLoader; +import java.nio.charset.Charset; import java.time.Duration; +import java.util.Collections; import java.util.Enumeration; -import java.util.concurrent.TimeoutException; +import java.util.List; import java.util.concurrent.atomic.AtomicReference; - -import static io.opentelemetry.instrumentation.test.utils.GcUtils.awaitGc; -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; - +import org.apache.commons.lang3.SystemUtils; +import org.junit.jupiter.api.Test; class ResourceInjectionTest { + private static String trimStream(URL url) throws Exception { + try (BufferedReader reader = + new BufferedReader(new InputStreamReader(url.openStream(), Charset.defaultCharset()))) { + return reader.readLine().trim(); + } + } + @Test @SuppressWarnings("UnnecessaryAsync") - void resourcesInjectedToNonDelegatingClassLoader() - throws IOException, ClassNotFoundException, InterruptedException, TimeoutException { + void resourcesInjectedToNonDelegatingClassLoader() throws Exception { + String resourceName = "test-resources/test-resource.txt"; - URL[] urls = {ResourceInjectionTest.class.getProtectionDomain().getCodeSource().getLocation()}; - AtomicReference emptyLoader = new AtomicReference<>(new URLClassLoader(urls, null)); + URL[] urls = {SystemUtils.class.getProtectionDomain().getCodeSource().getLocation()}; + AtomicReference emptyLoader = + new AtomicReference<>(new URLClassLoader(urls, null)); Enumeration resourceUrls = emptyLoader.get().getResources(resourceName); assertThat(resourceUrls.hasMoreElements()).isFalse(); @@ -30,29 +46,18 @@ void resourcesInjectedToNonDelegatingClassLoader() URLClassLoader notInjectedLoader = new URLClassLoader(urls, null); // this triggers resource injection - emptyLoader.get().loadClass(ResourceInjectionTest.class.getName()); + emptyLoader.get().loadClass(SystemUtils.class.getName()); - for (int i = 0; i < 2; i++) { - - URL test = ( resourceUrls.asIterator(); - if (i == 0) { - assertThat(test).isEqualTo("Hello world!"); - } else { - assertThat(test).isEqualTo("Hello there"); - } - } - assertThat(resourceUrls.hasMoreElements()).isFalse(); + List resourceList = Collections.list(emptyLoader.get().getResources(resourceName)); - -// resourceUrls = (Enumeration) Collections.list(emptyLoader.get().getResources(resourceName)); -// assertThat(resourceUrls.).isEqualTo(2); -// assertThat(list.get(0).openStream().toString().trim()).isEqualTo("Hello world!"); -// assertThat(list.get(1).openStream().toString().trim()).isEqualTo("Hello there"); + assertThat(resourceList.size()).isEqualTo(2); + assertThat(trimStream(resourceList.get(0))).isEqualTo("Hello world!"); + assertThat(trimStream(resourceList.get(1))).isEqualTo("Hello there"); assertThat(notInjectedLoader.getResources(resourceName).hasMoreElements()).isFalse(); // references to emptyloader are gone - emptyLoader.get().close(); + emptyLoader.get().close(); // cleanup WeakReference ref = new WeakReference<>(emptyLoader.get()); emptyLoader.set(null); @@ -60,5 +65,4 @@ void resourcesInjectedToNonDelegatingClassLoader() assertThat(ref.get()).isNull(); } - } From ba1e6290ebbfe428544b87420bacddc7cd2e6a5b Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 31 Dec 2024 08:34:15 -0500 Subject: [PATCH 3/5] fix gc issue --- .../internal/classloader/ResourceInjectionTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java index 5618aee7b3e3..8f9dc411c9e5 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java @@ -42,6 +42,7 @@ void resourcesInjectedToNonDelegatingClassLoader() throws Exception { Enumeration resourceUrls = emptyLoader.get().getResources(resourceName); assertThat(resourceUrls.hasMoreElements()).isFalse(); + resourceUrls = null; URLClassLoader notInjectedLoader = new URLClassLoader(urls, null); From 2e03f7fa1a1c046ef33f858c2967f2f9a3e547e1 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Thu, 2 Jan 2025 05:27:58 -0500 Subject: [PATCH 4/5] rename method --- .../internal/classloader/ResourceInjectionTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java index 8f9dc411c9e5..28f96113b136 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java @@ -24,7 +24,7 @@ class ResourceInjectionTest { - private static String trimStream(URL url) throws Exception { + private static String readLine(URL url) throws Exception { try (BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream(), Charset.defaultCharset()))) { return reader.readLine().trim(); @@ -52,8 +52,8 @@ void resourcesInjectedToNonDelegatingClassLoader() throws Exception { List resourceList = Collections.list(emptyLoader.get().getResources(resourceName)); assertThat(resourceList.size()).isEqualTo(2); - assertThat(trimStream(resourceList.get(0))).isEqualTo("Hello world!"); - assertThat(trimStream(resourceList.get(1))).isEqualTo("Hello there"); + assertThat(readLine(resourceList.get(0))).isEqualTo("Hello world!"); + assertThat(readLine(resourceList.get(1))).isEqualTo("Hello there"); assertThat(notInjectedLoader.getResources(resourceName).hasMoreElements()).isFalse(); From 683cce41ac8a68e575ac0e4c9bc7c8614347cb4b Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 3 Jan 2025 09:31:42 +0200 Subject: [PATCH 5/5] don't rely on default charset --- .../internal/classloader/ResourceInjectionTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java index 28f96113b136..342d0b94567f 100644 --- a/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java +++ b/instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionTest.java @@ -13,7 +13,7 @@ import java.lang.ref.WeakReference; import java.net.URL; import java.net.URLClassLoader; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Collections; import java.util.Enumeration; @@ -26,7 +26,7 @@ class ResourceInjectionTest { private static String readLine(URL url) throws Exception { try (BufferedReader reader = - new BufferedReader(new InputStreamReader(url.openStream(), Charset.defaultCharset()))) { + new BufferedReader(new InputStreamReader(url.openStream(), StandardCharsets.UTF_8))) { return reader.readLine().trim(); } } @@ -34,7 +34,6 @@ private static String readLine(URL url) throws Exception { @Test @SuppressWarnings("UnnecessaryAsync") void resourcesInjectedToNonDelegatingClassLoader() throws Exception { - String resourceName = "test-resources/test-resource.txt"; URL[] urls = {SystemUtils.class.getProtectionDomain().getCodeSource().getLocation()}; AtomicReference emptyLoader =