diff --git a/instrumentation/eclipse-osgi-3.6/javaagent/eclipse-osgi-3.6-javaagent.gradle b/instrumentation/eclipse-osgi-3.6/javaagent/eclipse-osgi-3.6-javaagent.gradle new file mode 100644 index 000000000000..c1b86a5e4372 --- /dev/null +++ b/instrumentation/eclipse-osgi-3.6/javaagent/eclipse-osgi-3.6-javaagent.gradle @@ -0,0 +1,22 @@ +apply from: "$rootDir/gradle/instrumentation.gradle" + +// this instrumentation applies to the class 'org.eclipse.osgi.internal.loader.BundleLoader' +// which is present in the following artifacts dating back to version 3.6 (2010): +// +// * 'org.eclipse.platform:org.eclipse.osgi' +// * 'org.eclipse.tycho:org.eclipse.osgi' +// * 'org.eclipse.osgi:org.eclipse.osgi' + +// TODO write a smoke test that does the following: +// +// docker run --mount 'type=bind,src=$AGENT_PATH,dst=/opentelemetry-javaagent-all.jar' +// -e JAVA_TOOL_OPTIONS=-javaagent:/opentelemetry-javaagent-all.jar +// wso2/wso2ei-business-process:6.5.0 +// +// without this instrumentation, the following error will appear in the docker logs: +// java.lang.ClassNotFoundException: org.wso2.carbon.humantask.ui.fileupload.HumanTaskUploadExecutor +// cannot be found by org.wso2.carbon.ui_4.4.36 +// +// ... or even better, write a standalone OSGi application that exhibits similar issue, +// so we can run against arbitrary (e.g. latest) Eclipse OSGi release, especially since +// this instrumentation patches a private method which could be renamed at any time diff --git a/instrumentation/eclipse-osgi-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/osgi/EclipseOsgiInstrumentationModule.java b/instrumentation/eclipse-osgi-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/osgi/EclipseOsgiInstrumentationModule.java new file mode 100644 index 000000000000..f61e5a558897 --- /dev/null +++ b/instrumentation/eclipse-osgi-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/osgi/EclipseOsgiInstrumentationModule.java @@ -0,0 +1,78 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.osgi; + +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.instrumentation.api.internal.InClassLoaderMatcher; +import io.opentelemetry.javaagent.tooling.InstrumentationModule; +import io.opentelemetry.javaagent.tooling.TypeInstrumentation; +import java.util.List; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * The ClassLoaderMatcher's call to ClassLoader.getResource() causes the Eclipse OSGi class loader + * to "dynamically import" a bundle for the package if such a bundle is not found, which can lead to + * application failure later on due to the bundle hierarchy no longer being "consistent". + * + *

Any side-effect of the ClassLoaderMatcher's call to ClassLoader.getResource() is generally + * undesirable, and so this instrumentation patches the behavior and suppresses the "dynamic import" + * of the missing package/bundle when the call is originating from ClassLoaderMatcher.. + */ +@AutoService(InstrumentationModule.class) +public class EclipseOsgiInstrumentationModule extends InstrumentationModule { + public EclipseOsgiInstrumentationModule() { + super("eclipse-osgi"); + } + + @Override + public List typeInstrumentations() { + return singletonList(new EclipseOsgiInstrumentation()); + } + + private static class EclipseOsgiInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("org.eclipse.osgi.internal.loader.BundleLoader"); + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod().and(named("isDynamicallyImported")).and(returns(boolean.class)), + EclipseOsgiInstrumentation.class.getName() + "$IsDynamicallyImportedAdvice"); + } + + public static class IsDynamicallyImportedAdvice { + + // "skipOn" is used to skip execution of the instrumented method when a ClassLoaderMatcher is + // currently executing, since we will be returning false regardless in onExit below + @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class, suppress = Throwable.class) + public static boolean onEnter() { + return InClassLoaderMatcher.get(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.Return(readOnly = false) boolean result, + @Advice.Enter boolean inClassLoaderMatcher) { + if (inClassLoaderMatcher) { + result = false; + } + } + } + } +} diff --git a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java new file mode 100644 index 000000000000..525b74c6b55c --- /dev/null +++ b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/internal/InClassLoaderMatcher.java @@ -0,0 +1,61 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.api.internal; + +public final class InClassLoaderMatcher { + + private static final ThreadLocal inClassLoaderMatcher = + ThreadLocal.withInitial(MutableBoolean::new); + + private InClassLoaderMatcher() {} + + /** + * Returns whether the ClassLoaderMatcher is currently executing. + * + *

This is used (at least) by the {@code eclipse-osgi} instrumentation in order to suppress a + * side effect in the Eclipse OSGi class loader that occurs when ClassLoaderMatcher calls + * ClassLoader.getResource(). See {@code EclipseOsgiInstrumentationModule} for more details. + */ + public static boolean get() { + return inClassLoaderMatcher.get().value; + } + + /** + * WARNING This should not be used by instrumentation. It should only be used by + * io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher. + * + *

The reason it can't be (easily) hidden is that this class needs to live in the bootstrap + * class loader to be accessible to instrumentation, while the ClassLoaderMatcher lives in the + * agent class loader. + */ + public static boolean getAndSet(boolean value) { + return inClassLoaderMatcher.get().getAndSet(value); + } + + /** + * WARNING This should not be used by instrumentation. It should only be used by + * io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher. + * + *

The reason it can't be (easily) hidden is that this class needs to live in the bootstrap + * class loader to be accessible to instrumentation, while the ClassLoaderMatcher lives in the + * agent class loader. + */ + public static void set(boolean value) { + inClassLoaderMatcher.get().value = value; + } + + // using MutableBoolean to avoid an extra thread local lookup for getAndSet() + private static class MutableBoolean { + + private boolean value; + + private boolean getAndSet(boolean value) { + boolean oldValue = this.value; + this.value = value; + return oldValue; + } + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/bytebuddy/matcher/ClassLoaderMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/bytebuddy/matcher/ClassLoaderMatcher.java index 515e293cff5a..e5c5d3cda0e7 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/bytebuddy/matcher/ClassLoaderMatcher.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/bytebuddy/matcher/ClassLoaderMatcher.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.tooling.bytebuddy.matcher; import io.opentelemetry.javaagent.bootstrap.WeakCache; +import io.opentelemetry.javaagent.instrumentation.api.internal.InClassLoaderMatcher; import io.opentelemetry.javaagent.tooling.AgentTooling; import net.bytebuddy.matcher.ElementMatcher; @@ -45,10 +46,15 @@ private ClassLoaderHasClassesNamedMatcher(String... classNames) { } private boolean hasResources(ClassLoader cl) { - for (String resource : resources) { - if (cl.getResource(resource) == null) { - return false; + boolean priorValue = InClassLoaderMatcher.getAndSet(true); + try { + for (String resource : resources) { + if (cl.getResource(resource) == null) { + return false; + } } + } finally { + InClassLoaderMatcher.set(priorValue); } return true; } diff --git a/settings.gradle b/settings.gradle index 3c49bab31ee2..281633996702 100644 --- a/settings.gradle +++ b/settings.gradle @@ -92,6 +92,7 @@ include ':instrumentation:dropwizard-testing' include ':instrumentation:apache-dubbo:apache-dubbo-2.7:library' include ':instrumentation:apache-dubbo:apache-dubbo-2.7:testing' include ':instrumentation:apache-dubbo:apache-dubbo-2.7:javaagent' +include ':instrumentation:eclipse-osgi-3.6:javaagent' include ':instrumentation:elasticsearch:elasticsearch-rest-common:javaagent' include ':instrumentation:elasticsearch:elasticsearch-rest-5.0:javaagent' include ':instrumentation:elasticsearch:elasticsearch-rest-6.4:javaagent'