Avoid allocating a stack argument if errors are not recorded

Author: amarziali

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java

@@ -14,13 +14,16 @@ public class InstrumentationErrors {
    * Record an error from {@link datadog.trace.agent.tooling.bytebuddy.ExceptionHandlers} for test
    * visibility.
    */
+  @SuppressWarnings("unused")
+  public static boolean isEnabled() {
+    return recordErrors;
+  }
+
   @SuppressWarnings("unused")
   public static void recordError(final Throwable throwable) {
-    if (recordErrors) {
-      StringWriter stackTrace = new StringWriter();
-      throwable.printStackTrace(new PrintWriter(stackTrace));
-      ERRORS.add(stackTrace.toString());
-    }
+    StringWriter stackTrace = new StringWriter();
+    throwable.printStackTrace(new PrintWriter(stackTrace));
+    ERRORS.add(stackTrace.toString());
   }
 
   // Visible for testing

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java

@@ -39,11 +39,12 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context)
               final boolean exitOnFailure = InstrumenterConfig.get().isInternalExitOnFailure();
               final String logMethod = exitOnFailure ? "error" : "debug";
 
-              // Writes the following bytecode (note that two statements are conditionally written):
+              // Writes the following bytecode (note that some statements are conditionally
+              // written):
               //
               // BlockingExceptionHandler.rethrowIfBlockingException(t); // when appSecEnabled=true
               // try {
-              //   InstrumentationErrors.recordError(t);
+              //   if (InstrumentationErrors.isEnabled()) InstrumentationErrors.recordError(t);
               //   org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class)
               //     .error("Failed to handle exception in instrumentation for ...", t);
               //   System.exit(1); // when exitOnFailure=true
@@ -69,15 +70,28 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context)
                     "(Ljava/lang/Throwable;)Ljava/lang/Throwable;");
               }
 
+              final Label skipRecord = new Label();
+
               mv.visitTryCatchBlock(logStart, logEnd, eatException, "java/lang/Throwable");
               mv.visitLabel(logStart);
+              mv.visitMethodInsn(
+                  Opcodes.INVOKESTATIC,
+                  "datadog/trace/bootstrap/InstrumentationErrors",
+                  "isEnabled",
+                  "()Z",
+                  false);
+              mv.visitJumpInsn(Opcodes.IFEQ, skipRecord);
               mv.visitInsn(Opcodes.DUP); // stack: (top) throwable,throwable
-              // invoke recordError on our exception tracker
               mv.visitMethodInsn(
                   Opcodes.INVOKESTATIC,
                   "datadog/trace/bootstrap/InstrumentationErrors",
                   "recordError",
-                  "(Ljava/lang/Throwable;)V");
+                  "(Ljava/lang/Throwable;)V",
+                  false);
+              mv.visitLabel(skipRecord);
+              if (frames) {
+                mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/Throwable"});
+              }
 
               // stack: (top) throwable
               mv.visitLdcInsn(Type.getType("L" + HANDLER_NAME + ";"));