Fix app failure under Eclipse OSGi (#2521)

* Fix app failure under Eclipse OSGi

* Feedback

* Alphabetical

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

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".
+ *
+ * <p>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<TypeInstrumentation> typeInstrumentations() {
+ return singletonList(new EclipseOsgiInstrumentation());
+ }
+
+ private static class EclipseOsgiInstrumentation implements TypeInstrumentation {
+
+ @Override
+ public ElementMatcher<TypeDescription> typeMatcher() {
+ return named("org.eclipse.osgi.internal.loader.BundleLoader");
+ }
+
+ @Override
+ public Map<? extends ElementMatcher<? super MethodDescription>, 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;
+ }
+ }
+ }
+ }
+}

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<MutableBoolean> inClassLoaderMatcher =
+ ThreadLocal.withInitial(MutableBoolean::new);
+
+ private InClassLoaderMatcher() {}
+
+ /**
+ * Returns whether the ClassLoaderMatcher is currently executing.
+ *
+ * <p>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.
+ *
+ * <p>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.
+ *
+ * <p>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;
+ }
+ }
+}

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;
  }

settings.gradle

@@ -95,6 +95,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'