Rewrite @Advice.Enter for indy advice (#9887)

instrumentation/jedis/jedis-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisInstrumentationModule.java

@@ -32,14 +32,4 @@ public class JedisInstrumentationModule extends InstrumentationModule {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new JedisConnectionInstrumentation(), new JedisInstrumentation());
   }
-
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    //      io/opentelemetry/javaagent/instrumentation/jedis/JedisRequestContext
-    //  at redis.clients.jedis.Jedis.flushAll(Jedis.java:367)
-    //  at io.opentelemetry.javaagent.instrumentation.jedis.v1_4
-    //          .JedisClientTest.setup(JedisClientTest.java:49)
-    return false;
-  }
 }

instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisInstrumentationModule.java

@@ -34,14 +34,4 @@ public class JedisInstrumentationModule extends InstrumentationModule {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new JedisConnectionInstrumentation(), new JedisInstrumentation());
   }
-
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    //      io/opentelemetry/javaagent/instrumentation/jedis/JedisRequestContext
-    //  at redis.clients.jedis.BinaryJedis.flushAll(BinaryJedis.java:595)
-    //  at io.opentelemetry.javaagent.instrumentation.jedis.v3_0
-    //       .Jedis30ClientTest.setup(Jedis30ClientTest.java:50)
-    return false;
-  }
 }

instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisInstrumentationModule.java

@@ -30,14 +30,4 @@ public class JedisInstrumentationModule extends InstrumentationModule {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new JedisConnectionInstrumentation(), new JedisInstrumentation());
   }
-
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    //      io/opentelemetry/javaagent/instrumentation/jedis/JedisRequestContext
-    // at redis.clients.jedis.Jedis.set(Jedis.java:4613)
-    // at io.opentelemetry.javaagent.instrumentation.jedis
-    //     .v4_0.Jedis40ClientTest.getCommand(Jedis40ClientTest.java:78)
-    return false;
-  }
 }

instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/KafkaClientsInstrumentationModule.java

@@ -18,12 +18,6 @@ public class KafkaClientsInstrumentationModule extends InstrumentationModule {
     super("kafka-clients", "kafka-clients-0.11", "kafka");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/metrics/KafkaMetricsInstrumentationModule.java

@@ -10,10 +10,14 @@ import static java.util.Arrays.asList;
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class KafkaMetricsInstrumentationModule extends InstrumentationModule {
+public class KafkaMetricsInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
   public KafkaMetricsInstrumentationModule() {
     super(
         "kafka-clients-metrics",
@@ -24,9 +28,11 @@ public class KafkaMetricsInstrumentationModule extends InstrumentationModule {
   }
 
   @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
+  public void injectClasses(ClassInjector injector) {
+    injector
+        .proxyBuilder(
+            "io.opentelemetry.instrumentation.kafka.internal.OpenTelemetryMetricsReporter")
+        .inject(InjectionMode.CLASS_ONLY);
   }
 
   @Override

instrumentation/kafka/kafka-streams-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkastreams/KafkaStreamsInstrumentationModule.java

@@ -18,13 +18,6 @@ public class KafkaStreamsInstrumentationModule extends InstrumentationModule {
     super("kafka-streams", "kafka-streams-0.11", "kafka");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    // io/opentelemetry/javaagent/instrumentation/kafkastreams/StateHolder
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/quarkus-resteasy-reactive/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quarkus/resteasy/reactive/QuarkusResteasyReactiveInstrumentationModule.java

@@ -19,12 +19,6 @@ public class QuarkusResteasyReactiveInstrumentationModule extends Instrumentatio
     super("quarkus", "jaxrs", "quarkus-resteasy-reactive", "quarkus-resteasy-reactive-3.0");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // RunAdvice returns OtelRequestContext that is not available to the application class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/ratpack/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java

@@ -20,8 +20,11 @@ public class RatpackInstrumentationModule extends InstrumentationModule {
 
   @Override
   public boolean isIndyModule() {
-    // StartAdvice uses both @Advice.Argument(readOnly = false) and return value from an
-    // @OnMethodEnter advice
+    // java.lang.ClassCastException: class
+    // io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.AutoValue_ServerContext
+    // cannot be cast to class
+    // io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.ServerContext
+    // (io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.AutoValue_ServerContext is in unnamed module of loader 'app'; io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.ServerContext is in unnamed module of loader io.opentelemetry.javaagent.tooling.instrumentation.indy.InstrumentationModuleClassLoader @7f088b5c)
     return false;
   }
 

instrumentation/reactor/reactor-kafka-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactor/kafka/v1_0/ReactorKafkaInstrumentationModule.java

@@ -19,12 +19,6 @@ public class ReactorKafkaInstrumentationModule extends InstrumentationModule {
     super("reactor-kafka", "reactor-kafka-1.0");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyInstrumentationModule.java

@@ -7,10 +7,12 @@ package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0;
 
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.List;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
@@ -24,18 +26,13 @@ import reactor.netty.http.client.HttpClient;
  * HttpClient#doOnRequest(BiConsumer)} to pass context from the caller to Reactor to Netty.
  */
 @AutoService(InstrumentationModule.class)
-public class ReactorNettyInstrumentationModule extends InstrumentationModule {
+public class ReactorNettyInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public ReactorNettyInstrumentationModule() {
     super("reactor-netty", "reactor-netty-1.0");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // ResponseMonoAdvice uses both @Advice.Local and return value from an @OnMethodEnter advice
-    return false;
-  }
-
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // Introduced in 1.0.0
@@ -47,6 +44,11 @@ public class ReactorNettyInstrumentationModule extends InstrumentationModule {
     return className.startsWith("reactor.netty.http.client.HttpClientConfigBuddy");
   }
 
+  @Override
+  public List<String> injectedClassNames() {
+    return singletonList("reactor.netty.http.client.HttpClientConfigBuddy");
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/spring/spring-kafka-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/kafka/v2_7/SpringKafkaInstrumentationModule.java

@@ -18,12 +18,6 @@ public class SpringKafkaInstrumentationModule extends InstrumentationModule {
     super("spring-kafka", "spring-kafka-2.7");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/spymemcached-2.12/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedInstrumentationModule.java

@@ -19,12 +19,6 @@ public class SpymemcachedInstrumentationModule extends InstrumentationModule {
     super("spymemcached", "spymemcached-2.12");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // SyncOperationAdvice uses both @Advice.Local and return value from an @OnMethodEnter advice
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new MemcachedClientInstrumentation());

instrumentation/spymemcached-2.12/javaagent/src/test/groovy/SpymemcachedTest.groovy

@@ -6,7 +6,6 @@
 import com.google.common.util.concurrent.MoreExecutors
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
-import io.opentelemetry.javaagent.instrumentation.spymemcached.CompletionListener
 import io.opentelemetry.semconv.SemanticAttributes
 import net.spy.memcached.CASResponse
 import net.spy.memcached.ConnectionFactory
@@ -627,15 +626,15 @@ class SpymemcachedTest extends AgentInstrumentationSpecification {
         "$SemanticAttributes.DB_OPERATION" operation
 
         if (error == "canceled") {
-          "${CompletionListener.DB_COMMAND_CANCELLED}" true
+          "spymemcached.command.cancelled" true
         }
 
         if (result == "hit") {
-          "${CompletionListener.MEMCACHED_RESULT}" CompletionListener.HIT
+          "spymemcached.result" "hit"
         }
 
         if (result == "miss") {
-          "${CompletionListener.MEMCACHED_RESULT}" CompletionListener.MISS
+          "spymemcached.result" "miss"
         }
       }
     }

instrumentation/vertx/vertx-kafka-client-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/kafka/v3_6/VertxKafkaInstrumentationModule.java

@@ -19,12 +19,6 @@ public class VertxKafkaInstrumentationModule extends InstrumentationModule {
     super("vertx-kafka-client", "vertx-kafka-client-3.6", "vertx");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/AdviceTransformer.java

@@ -7,6 +7,7 @@ package io.opentelemetry.javaagent.tooling.instrumentation.indy;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -24,6 +25,7 @@ import org.objectweb.asm.Type;
 import org.objectweb.asm.commons.GeneratorAdapter;
 import org.objectweb.asm.commons.Method;
 import org.objectweb.asm.tree.AnnotationNode;
+import org.objectweb.asm.tree.ClassNode;
 import org.objectweb.asm.tree.MethodNode;
 
 /**
@@ -37,6 +39,29 @@ class AdviceTransformer {
   static byte[] transform(byte[] bytes) {
     ClassReader cr = new ClassReader(bytes);
     ClassWriter cw = new ClassWriter(cr, ClassWriter.COMPUTE_MAXS);
+    ClassNode classNode = new ClassNode();
+    cr.accept(classNode, ClassReader.EXPAND_FRAMES);
+
+    // skip the class if there aren't any methods with advice annotations or these annotations have
+    // set inline = false
+    if (!hasInlineAdvice(classNode)) {
+      classNode.accept(cw);
+      return cw.toByteArray();
+    }
+
+    // sort enter advice method before exit advice
+    classNode.methods.sort(
+        Comparator.comparingInt(
+            (methodNode) -> {
+              if (isEnterAdvice(methodNode)) {
+                return 1;
+              } else if (isExitAdvice(methodNode)) {
+                return 2;
+              }
+              return 0;
+            }));
+
+    TransformationContext context = new TransformationContext();
     ClassVisitor cv =
         new ClassVisitor(Opcodes.ASM9, cw) {
 
@@ -49,20 +74,45 @@ class AdviceTransformer {
               public void visitEnd() {
                 super.visitEnd();
 
-                instrument(this, classVisitor);
+                instrument(context, this, classVisitor);
               }
             };
           }
         };
-    cr.accept(cv, ClassReader.EXPAND_FRAMES);
+    classNode.accept(cv);
     return cw.toByteArray();
   }
 
+  private static boolean hasInlineAdvice(ClassNode classNode) {
+    for (MethodNode mn : classNode.methods) {
+      if (hasInlineAdvice(mn)) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  private static boolean hasInlineAdvice(MethodNode methodNode) {
+    return hasInlineAdvice(methodNode, ADVICE_ON_METHOD_ENTER)
+        || hasInlineAdvice(methodNode, ADVICE_ON_METHOD_EXIT);
+  }
+
+  private static boolean hasInlineAdvice(MethodNode methodNode, Type type) {
+    AnnotationNode annotationNode = getAnnotationNode(methodNode, type);
+    if (annotationNode != null) {
+      // delegating advice has attribute "inline" = false
+      // all other advice is inline
+      return !Boolean.FALSE.equals(getAttributeValue(annotationNode, "inline"));
+    }
+    return false;
+  }
+
   // method argument annotated with Advice.Argument or Advice.Return
   private static class OutputArgument {
     // index of the method argument with the annotation
     final int adviceIndex;
-    // value of the annotation or -1 if Advice.Return
+    // value of the annotation or -1 if Advice.Return or Advice.Enter
     final int methodIndex;
 
     OutputArgument(int adviceIndex, int methodIndex) {
@@ -128,6 +178,28 @@ class AdviceTransformer {
     return null;
   }
 
+  private static final Type ADVICE_ENTER = Type.getType(Advice.Enter.class);
+
+  /** Argument annotated with {@code @Advice.Enter} or {@code null}. */
+  private static OutputArgument getEnterArgument(MethodNode source) {
+    Type[] argumentTypes = Type.getArgumentTypes(source.desc);
+    if (source.visibleParameterAnnotations != null) {
+      int i = 0;
+      for (List<AnnotationNode> list : source.visibleParameterAnnotations) {
+        for (AnnotationNode annotationNode : list) {
+          Type annotationType = Type.getType(annotationNode.desc);
+          if (ADVICE_ENTER.equals(annotationType)
+              && argumentTypes[i].getDescriptor().length() > 1) {
+            return new OutputArgument(i, -1);
+          }
+        }
+        i++;
+      }
+    }
+
+    return null;
+  }
+
   private static final Type ADVICE_LOCAL = Type.getType(Advice.Local.class);
 
   /** List of arguments annotated with {@code @Advice.Local}. */
@@ -152,29 +224,33 @@ class AdviceTransformer {
     return result;
   }
 
-  private static final Type ADVICE_ENTER = Type.getType(Advice.OnMethodEnter.class);
+  private static final Type ADVICE_ON_METHOD_ENTER = Type.getType(Advice.OnMethodEnter.class);
 
   private static boolean isEnterAdvice(MethodNode source) {
-    return hasAnnotation(source, ADVICE_ENTER);
+    return hasAnnotation(source, ADVICE_ON_METHOD_ENTER);
   }
 
-  private static final Type ADVICE_EXIT = Type.getType(Advice.OnMethodExit.class);
+  private static final Type ADVICE_ON_METHOD_EXIT = Type.getType(Advice.OnMethodExit.class);
 
   private static boolean isExitAdvice(MethodNode source) {
-    return hasAnnotation(source, ADVICE_EXIT);
+    return hasAnnotation(source, ADVICE_ON_METHOD_EXIT);
   }
 
-  private static boolean hasAnnotation(MethodNode source, Type type) {
+  private static AnnotationNode getAnnotationNode(MethodNode source, Type type) {
     if (source.visibleAnnotations != null) {
       for (AnnotationNode annotationNode : source.visibleAnnotations) {
         Type annotationType = Type.getType(annotationNode.desc);
         if (type.equals(annotationType)) {
-          return true;
+          return annotationNode;
         }
       }
     }
 
-    return false;
+    return null;
+  }
+
+  private static boolean hasAnnotation(MethodNode source, Type type) {
+    return getAnnotationNode(source, type) != null;
   }
 
   /**
@@ -304,10 +380,10 @@ class AdviceTransformer {
   }
 
   /**
-   * Transform arguments annotated with {@code @Advice.Local}.
+   * Transform arguments annotated with {@code @Advice.Local} and {@code @Advice.Enter}.
    *
    * <pre>{@code
-   * void foo(@Advice.Local("foo") T1 foo, @Advice.Local("bar") T1 bar)
+   * void foo(@Advice.Local("foo") T1 foo, @Advice.Local("bar") T2 bar)
    * }</pre>
    *
    * <p>for enter advice is transformed to
@@ -339,6 +415,7 @@ class AdviceTransformer {
       MethodNode source,
       String originalDesc,
       List<AdviceLocal> adviceLocals,
+      OutputArgument enterArgument,
       int returnIndex) {
     AtomicReference<GeneratorAdapter> generatorRef = new AtomicReference<>();
     AtomicInteger dataIndex = new AtomicInteger();
@@ -352,6 +429,12 @@ class AdviceTransformer {
             if (Type.getDescriptor(Advice.Local.class).equals(descriptor)) {
               descriptor = Type.getDescriptor(Advice.Unused.class);
             }
+            // replace @Advice.Enter with @Advice.Unused
+            if (enterArgument != null
+                && enterArgument.adviceIndex == parameter
+                && Type.getDescriptor(Advice.Enter.class).equals(descriptor)) {
+              descriptor = Type.getDescriptor(Advice.Unused.class);
+            }
             return super.visitParameterAnnotation(parameter, descriptor, visible);
           }
 
@@ -386,7 +469,7 @@ class AdviceTransformer {
                 ga.invokeVirtual(
                     hashMapType,
                     Method.getMethod("java.lang.Object put(java.lang.Object, java.lang.Object)"));
-                // por return value of Map.put
+                // pop return value of Map.put
                 ga.pop();
               }
               // stack: array, array, array index for map, map
@@ -404,7 +487,7 @@ class AdviceTransformer {
             Type[] argumentTypes = ga.getArgumentTypes();
 
             if (isEnterAdvice) {
-              // we have change the type fo method arguments annotated with @Advice.Local to Object
+              // we have changed the type fo method arguments annotated with @Advice.Local to Object
               // here we'll load the argument, cast it to its actual type, and store it back
               for (AdviceLocal adviceLocal : adviceLocals) {
                 ga.loadArg(adviceLocal.adviceIndex);
@@ -414,18 +497,29 @@ class AdviceTransformer {
               return;
             }
 
-            // the index of last argument where Map is inserted (argumentTypes array does not
-            // contain the Map)
+            // the index of last argument where object array returned from enter advice is inserted
+            // (argumentTypes array does not contain the object array)
             int lastArgumentIndex = argumentTypes.length;
             AnnotationVisitor av =
                 mv.visitParameterAnnotation(
                     lastArgumentIndex, Type.getDescriptor(Advice.Enter.class), true);
             av.visitEnd();
 
-            Type mapType = Type.getType(Map.class);
             // load object array
             ga.loadLocal(dataIndex.get(), OBJECT_ARRAY_TYPE);
+            if (enterArgument != null) {
+              // value for @Advice.Enter is stored as the first element
+              ga.dup();
+              ga.push(0);
+              Type type = argumentTypes[enterArgument.adviceIndex];
+              ga.arrayLoad(type);
+              ga.checkCast(type);
+              ga.storeArg(enterArgument.adviceIndex);
+            }
+
+            // object array on stack
             ga.dup();
+            Type mapType = Type.getType(Map.class);
             // we want the last element of the array
             ga.arrayLength();
             ga.visitInsn(Opcodes.ICONST_1);
@@ -459,27 +553,42 @@ class AdviceTransformer {
     return ga;
   }
 
-  private static void instrument(MethodNode methodNode, ClassVisitor classVisitor) {
+  private static void instrument(
+      TransformationContext context, MethodNode methodNode, ClassVisitor classVisitor) {
     String originalDescriptor = methodNode.desc;
     String[] exceptionsArray = methodNode.exceptions.toArray(new String[0]);
 
-    // only instrument if method returns void, in most of the instrumentations we need to change
-    // the return type of the method which will only work if the method returns void
-    if (Type.VOID_TYPE.equals(Type.getReturnType(methodNode.desc))) {
-      List<OutputArgument> writableArguments = getWritableArguments(methodNode);
-      OutputArgument writableReturn = getWritableReturnValue(methodNode);
-      List<AdviceLocal> adviceLocals = getLocals(methodNode);
-      boolean isEnterAdvice = isEnterAdvice(methodNode);
+    List<OutputArgument> writableArguments = getWritableArguments(methodNode);
+    OutputArgument writableReturn = getWritableReturnValue(methodNode);
+    OutputArgument enterArgument = getEnterArgument(methodNode);
+    List<AdviceLocal> adviceLocals = getLocals(methodNode);
+    boolean isEnterAdvice = isEnterAdvice(methodNode);
+    boolean isExitAdvice = isExitAdvice(methodNode);
+    Type returnType = Type.getReturnType(methodNode.desc);
+
+    // currently we don't support rewriting enter advice returning a primitive type
+    if (isEnterAdvice
+        && !(returnType.getSort() == Type.VOID
+            || returnType.getSort() == Type.OBJECT
+            || returnType.getSort() == Type.ARRAY)) {
+      context.disableReturnTypeChange();
+    }
+    // context is shared by enter and exit advice, if entry advice was rejected don't attempt to
+    // rewrite usages of @Advice.Enter in the exit advice
+    if (!context.canChangeReturnType()) {
+      enterArgument = null;
+    }
 
+    if (context.canChangeReturnType() || (isExitAdvice && Type.VOID_TYPE.equals(returnType))) {
       if (!writableArguments.isEmpty()
           || writableReturn != null
+          || !Type.VOID_TYPE.equals(returnType)
           || (!adviceLocals.isEmpty() && isEnterAdvice)) {
         Type[] argumentTypes = Type.getArgumentTypes(methodNode.desc);
         if (!adviceLocals.isEmpty() && isEnterAdvice) {
           // Set type of arguments annotated with @Advice.Local to Object. These arguments are
-          // likely
-          // to be helper classes which currently breaks because the invokedynamic call in advised
-          // class needs access to the parameter types of the advice method.
+          // likely to be helper classes which currently breaks because the invokedynamic call in
+          // advised class needs access to the parameter types of the advice method.
           for (AdviceLocal adviceLocal : adviceLocals) {
             argumentTypes[adviceLocal.adviceIndex] = OBJECT_TYPE;
           }
@@ -496,6 +605,7 @@ class AdviceTransformer {
                 exceptionsArray);
         MethodVisitor mv =
             instrumentOurParameters(
+                context,
                 tmp,
                 methodNode,
                 originalDescriptor,
@@ -510,7 +620,7 @@ class AdviceTransformer {
 
       // this is the only transformation that does not change the return type of the advice method,
       // thus it is also the only transformation that can be applied on top of the other transforms
-      if (!adviceLocals.isEmpty() && isExitAdvice(methodNode)) {
+      if ((!adviceLocals.isEmpty() || enterArgument != null) && isExitAdvice) {
         // Set type of arguments annotated with @Advice.Local to Object. These arguments are likely
         // to be helper classes which currently breaks because the invokedynamic call in advised
         // class needs access to the parameter types of the advice method.
@@ -518,6 +628,9 @@ class AdviceTransformer {
         for (AdviceLocal adviceLocal : adviceLocals) {
           newArgumentTypes[adviceLocal.adviceIndex] = OBJECT_TYPE;
         }
+        if (enterArgument != null) {
+          newArgumentTypes[enterArgument.adviceIndex] = OBJECT_TYPE;
+        }
         List<Type> typeList = new ArrayList<>(Arrays.asList(newArgumentTypes));
         // add Object array as the last argument, this array is used to pass info from the enter
         // advice
@@ -535,7 +648,8 @@ class AdviceTransformer {
                 methodNode.signature,
                 exceptionsArray);
         MethodVisitor mv =
-            instrumentAdviceLocals(false, tmp, methodNode, originalDescriptor, adviceLocals, -1);
+            instrumentAdviceLocals(
+                false, tmp, methodNode, originalDescriptor, adviceLocals, enterArgument, -1);
         methodNode.accept(mv);
 
         methodNode = tmp;
@@ -549,12 +663,13 @@ class AdviceTransformer {
             methodNode.desc,
             methodNode.signature,
             exceptionsArray);
-    mv = delegateAdvice(mv);
+    mv = delegateAdvice(context, mv);
 
     methodNode.accept(mv);
   }
 
   private static MethodVisitor instrumentOurParameters(
+      TransformationContext context,
       MethodVisitor target,
       MethodNode source,
       String originalDesc,
@@ -562,7 +677,9 @@ class AdviceTransformer {
       OutputArgument writableReturn,
       List<AdviceLocal> adviceLocals) {
 
-    int returnArraySize = 0;
+    // position 0 in enter advice is reserved for the return value of the method
+    // to avoid complicating things further we aren't going to figure out whether it is really used
+    int returnArraySize = isEnterAdvice(source) ? 1 : 0;
     if (writableReturn != null) {
       target = instrumentWritableReturn(target, source, writableReturn, returnArraySize);
       returnArraySize++;
@@ -573,28 +690,34 @@ class AdviceTransformer {
     }
     if (!adviceLocals.isEmpty() && isEnterAdvice(source)) {
       target =
-          instrumentAdviceLocals(true, target, source, originalDesc, adviceLocals, returnArraySize);
+          instrumentAdviceLocals(
+              true, target, source, originalDesc, adviceLocals, null, returnArraySize);
       returnArraySize++;
     }
-    target = addReturnArray(target, returnArraySize);
+    target = addReturnArray(context, target, returnArraySize);
 
     return target;
   }
 
   /** Return the value of the {@code readOnly} attribute of the annotation. */
   private static boolean isWriteable(AnnotationNode annotationNode) {
+    Object value = getAttributeValue(annotationNode, "readOnly");
+    return Boolean.FALSE.equals(value);
+  }
+
+  private static Object getAttributeValue(AnnotationNode annotationNode, String attributeName) {
     if (annotationNode.values != null && !annotationNode.values.isEmpty()) {
       List<Object> values = annotationNode.values;
       for (int i = 0; i < values.size(); i += 2) {
-        String attributeName = (String) values.get(i);
-        Object attributeValue = values.get(i + 1);
-        if ("readOnly".equals(attributeName)) {
-          return Boolean.FALSE.equals(attributeValue);
+        String name = (String) values.get(i);
+        Object value = values.get(i + 1);
+        if (attributeName.equals(name)) {
+          return value;
         }
       }
     }
 
-    return false;
+    return null;
   }
 
   /** Return the value of the {@code value} attribute of the annotation. */
@@ -613,15 +736,34 @@ class AdviceTransformer {
     return null;
   }
 
-  private static MethodVisitor addReturnArray(MethodVisitor target, int returnArraySize) {
+  private static MethodVisitor addReturnArray(
+      TransformationContext context, MethodVisitor target, int returnArraySize) {
     return new MethodVisitor(Opcodes.ASM9, target) {
       @Override
       public void visitInsn(int opcode) {
         if (Opcodes.RETURN == opcode) {
+          // change the return value of the method to Object[]
           GeneratorAdapter ga = new GeneratorAdapter(mv, 0, null, "()V");
           ga.push(returnArraySize);
           ga.newArray(OBJECT_TYPE);
           opcode = Opcodes.ARETURN;
+        } else if (context.canChangeReturnType() && Opcodes.ARETURN == opcode) {
+          // change the return value of the method to Object[] that on the 0 index contains the
+          // original return value
+
+          // stack: original return value
+          GeneratorAdapter ga = new GeneratorAdapter(mv, 0, null, "()V");
+          ga.push(returnArraySize);
+          ga.newArray(OBJECT_TYPE);
+          // stack: original return value, array
+          ga.dupX1();
+          // stack: array, original return value, array
+          ga.swap();
+          // stack: array, array, original return value
+          ga.push(0); // original return value is stored as the first element
+          ga.swap();
+          ga.arrayStore(OBJECT_TYPE);
+          // stack: array
         }
         super.visitInsn(opcode);
       }
@@ -630,9 +772,11 @@ class AdviceTransformer {
 
   /**
    * If method is annotated with {@link Advice.OnMethodEnter} or {@link Advice.OnMethodExit} set
-   * {@code inline} attribute on the annotation to {@code false}.
+   * {@code inline} attribute on the annotation to {@code false}. If method is annotated with {@link
+   * Advice.OnMethodEnter} and has {@code skipOn} attribute set {@code skipOnIndex} attribute on the
+   * annotation to {@code 0}.
    */
-  private static MethodVisitor delegateAdvice(MethodVisitor target) {
+  private static MethodVisitor delegateAdvice(TransformationContext context, MethodVisitor target) {
     return new MethodVisitor(Opcodes.ASM9, target) {
       @Override
       public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) {
@@ -644,12 +788,15 @@ class AdviceTransformer {
         }
         return new AnnotationVisitor(api, av) {
           boolean hasInline = false;
+          boolean hasSkipOn = false;
 
           @Override
           public void visit(String name, Object value) {
             if ("inline".equals(name)) {
               value = Boolean.FALSE;
               hasInline = true;
+            } else if ("skipOn".equals(name) && value != void.class) {
+              hasSkipOn = true;
             }
             super.visit(name, value);
           }
@@ -659,6 +806,9 @@ class AdviceTransformer {
             if (!hasInline) {
               visit("inline", Boolean.FALSE);
             }
+            if (context.canChangeReturnType() && hasSkipOn) {
+              visit("skipOnIndex", 0);
+            }
             super.visitEnd();
           }
         };
@@ -691,5 +841,17 @@ class AdviceTransformer {
     };
   }
 
+  private static class TransformationContext {
+    private boolean canChangeReturnType = true;
+
+    void disableReturnTypeChange() {
+      canChangeReturnType = false;
+    }
+
+    boolean canChangeReturnType() {
+      return canChangeReturnType;
+    }
+  }
+
   private AdviceTransformer() {}
 }