feat(asm): Clean up visitors

Author: PerfectSlayer

dd-java-agent/instrumentation/jetty-common/src/main/java/datadog/trace/instrumentation/jetty/HandleRequestVisitor.java

@@ -4,8 +4,6 @@
 
 import datadog.context.Context;
 import datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge;
-import java.io.BufferedWriter;
-import java.io.IOException;
 import java.util.List;
 import net.bytebuddy.jar.asm.Label;
 import net.bytebuddy.jar.asm.MethodVisitor;
@@ -19,7 +17,7 @@
  * and replaces it with:
  *
  * <pre>
- * if (JettyBlockingHelper.block(this.getRequest(), this.getResponse(), context) {
+ * if (JettyBlockingHelper.block(this.getRequest(), this.getResponse(), Java8BytecodeBridge.getCurrentContext()) {
  *   // nothing
  * } else {
  *   server.handle(this);
@@ -31,25 +29,10 @@
 public class HandleRequestVisitor extends MethodVisitor {
   private static final Logger log = LoggerFactory.getLogger(HandleRequestVisitor.class);
 
-  //  private boolean lookForStore;
-  //  private int contextVar = -1;
   private final int classVersion;
   private final String connClassInternalName;
   private boolean success;
 
-  private BufferedWriter debugWriter;
-
-  private void debug(String msg) {
-    if (debugWriter == null) {
-      return;
-    }
-    try {
-      debugWriter.write(msg);
-      debugWriter.newLine();
-    } catch (IOException ignored) {
-    }
-  }
-
   public HandleRequestVisitor(
       int api,
       int classVersion,
@@ -58,18 +41,6 @@ public HandleRequestVisitor(
     super(api, methodVisitor);
     this.classVersion = classVersion;
     this.connClassInternalName = connClassInternalName;
-
-    //    try {
-    //      String path =
-    //
-    // "/Users/bruce.bujon/go/src/github.com/DataDog/dd-trace-java/bbujon/debug/HandleRequestVisitor-"
-    //              + System.nanoTime()
-    //              + ".txt";
-    //      this.debugWriter = new BufferedWriter(new OutputStreamWriter(new
-    // FileOutputStream(path)));
-    //      debug("Initializing");
-    //    } catch (IOException ignored) {
-    //    }
   }
 
   DelayLoadsMethodVisitor delayVisitorDelegate() {
@@ -79,30 +50,12 @@ DelayLoadsMethodVisitor delayVisitorDelegate() {
   @Override
   public void visitMethodInsn(
       int opcode, String owner, String name, String descriptor, boolean isInterface) {
-    //    if (contextVar == -1) {
-    //      lookForStore =
-    //          !lookForStore
-    //              && opcode == Opcodes.INVOKEVIRTUAL
-    //              && name.equals("startSpan")
-    //              && descriptor.endsWith("Ldatadog.context.Context;");
-    //    } else
     if (opcode == Opcodes.INVOKEVIRTUAL
         && owner.equals("org/eclipse/jetty/server/Server")
         && name.equals("handle")
         && descriptor.equals("(L" + this.connClassInternalName + ";)V")) {
       DelayLoadsMethodVisitor mv = delayVisitorDelegate();
       List<Integer> savedLoads = mv.transferLoads();
-      debug("visitMethodInsn");
-      debug(
-          "opcode: "
-              + opcode
-              + " owner: "
-              + owner
-              + " name: "
-              + name
-              + " descriptor: "
-              + descriptor);
-      debug("savedLoads size: " + savedLoads.size());
       if (savedLoads.size() != 2) {
         mv.commitLoads(savedLoads);
         super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
@@ -111,6 +64,7 @@ public void visitMethodInsn(
 
       Label afterHandle = new Label();
 
+      // Add Request, Response and Context onto the stack
       super.visitVarInsn(Opcodes.ALOAD, 0);
       super.visitMethodInsn(
           Opcodes.INVOKEVIRTUAL,
@@ -132,8 +86,7 @@ public void visitMethodInsn(
           "getCurrentContext",
           "()Ldatadog/context/Context;",
           false);
-      //      super.visitVarInsn(Opcodes.ALOAD, contextVar);
-
+      // Call JettyBlockingHelper.block(request, response, context)
       super.visitMethodInsn(
           Opcodes.INVOKESTATIC,
           Type.getInternalName(JettyBlockingHelper.class),
@@ -142,10 +95,13 @@ public void visitMethodInsn(
               + Type.getDescriptor(Context.class)
               + ")Z",
           false);
+      // Jump after handle if blocked
       super.visitJumpInsn(Opcodes.IFNE, afterHandle);
 
+      // Inject default handle instructions
       mv.commitLoads(savedLoads);
       super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
+      // Add after handle label
       super.visitLabel(afterHandle);
       if (needsStackFrames()) {
         super.visitFrame(Opcodes.F_SAME, 0, null, 0, null);
@@ -156,29 +112,13 @@ public void visitMethodInsn(
 
     super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
   }
-  //
-  //  @Override
-  //  public void visitVarInsn(int opcode, int varIndex) {
-  //    if (lookForStore && opcode == Opcodes.ASTORE) {
-  //      contextVar = varIndex;
-  //      lookForStore = false;
-  //    }
-  //
-  //    super.visitVarInsn(opcode, varIndex);
-  //  }
 
   @Override
   public void visitEnd() {
     if (!success) {
       log.warn(
           "Transformation of Jetty's connection class was not successful. Blocking will likely not work");
     }
-    if (this.debugWriter != null) {
-      try {
-        this.debugWriter.close();
-      } catch (IOException ignored) {
-      }
-    }
     super.visitEnd();
   }