add error messages

Author: jpbempel

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java

@@ -183,7 +183,7 @@ private void handleProbesChanges(ConfigurationComparer changes, Configuration ne
     }
     List<Class<?>> changedClasses =
         finder.getAllLoadedChangedClasses(instrumentation.getAllLoadedClasses(), changes);
-    changedClasses = detectMethodParameters(changedClasses);
+    changedClasses = detectMethodParameters(changes, changedClasses);
     retransformClasses(changedClasses);
     // ensures that we have at least re-transformed 1 class
     if (changedClasses.size() > 0) {
@@ -197,7 +197,8 @@ private void handleProbesChanges(ConfigurationComparer changes, Configuration ne
    * Spring 6/Spring boot 3 rely exclusively on this attribute and may throw an exception
    * if no attribute found.
    */
-  private List<Class<?>> detectMethodParameters(List<Class<?>> changedClasses) {
+  private List<Class<?>> detectMethodParameters(
+      ConfigurationComparer changes, List<Class<?>> changedClasses) {
     if (JAVA_AT_LEAST_19) {
       // bug is fixed since JDK19, no need to perform detection
       return changedClasses;
@@ -220,6 +221,10 @@ private List<Class<?>> detectMethodParameters(List<Class<?>> changedClasses) {
               method.getName(),
               parameters[0].getName());
           // skip the class: compiled with -parameters
+          reportError(
+              changes,
+              "Method Parameters detected, instrumentation not supported for "
+                  + changedClass.getTypeName());
           addClass = false;
         }
         // we found at leat a method with one parameter if name is not present we can stop there
@@ -245,6 +250,16 @@ private void reportReceived(ConfigurationComparer changes) {
     }
   }
 
+  private void reportError(ConfigurationComparer changes, String errorMsg) {
+    for (ProbeDefinition def : changes.getAddedDefinitions()) {
+      if (def instanceof ExceptionProbe) {
+        // do not report received for exception probes
+        continue;
+      }
+      sink.addError(def.getProbeId(), errorMsg);
+    }
+  }
+
   private void installNewDefinitions(Configuration newConfiguration) {
     DebuggerContext.initClassFilter(new DenyListHelper(newConfiguration.getDenyList()));
     if (appliedDefinitions.isEmpty()) {

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java

@@ -82,7 +82,7 @@
 public class DebuggerTransformer implements ClassFileTransformer {
   private static final Logger LOGGER = LoggerFactory.getLogger(DebuggerTransformer.class);
   private static final String CANNOT_FIND_METHOD = "Cannot find method %s::%s%s";
-  private static final String INSTRUMENTATION_FAILS = "Instrumentation fails for %s";
+  private static final String INSTRUMENTATION_FAILS = "Instrumentation failed for %s: %s";
   private static final String CANNOT_FIND_LINE = "No executable code was found at %s:L%s";
   private static final Pattern COMMA_PATTERN = Pattern.compile(",");
   private static final List<Class<?>> PROBE_ORDER =
@@ -276,7 +276,7 @@ public byte[] transform(
           "type {} matched but no transformation for definitions: {}", classFilePath, definitions);
     } catch (Throwable ex) {
       LOGGER.warn("Cannot transform: ", ex);
-      reportInstrumentationFails(definitions, fullyQualifiedClassName);
+      reportInstrumentationFails(definitions, fullyQualifiedClassName, ex.toString());
     }
     return null;
   }
@@ -311,7 +311,7 @@ private void checkMethodParameters(ClassNode classNode) {
       }
       if (methodNode.parameters != null && !methodNode.parameters.isEmpty()) {
         throw new RuntimeException(
-            "Method Parameters attribute detected, cannot instrument class " + classNode.name);
+            "Method Parameters attribute detected, instrumentation not supported");
       } else {
         // we found at leat a method with one parameter if name is not present we can stop there
         break;
@@ -535,7 +535,7 @@ private byte[] writeClassFile(
       classNode.accept(visitor);
     } catch (Throwable t) {
       LOGGER.error("Cannot write classfile for class: {} Exception: ", classFilePath, t);
-      reportInstrumentationFails(definitions, Strings.getClassName(classFilePath));
+      reportInstrumentationFails(definitions, Strings.getClassName(classFilePath), t.toString());
       return null;
     }
     byte[] data = writer.toByteArray();
@@ -659,8 +659,9 @@ private void reportLocationNotFound(
     // on a separate class files because probe was set on an inner/top-level class
   }
 
-  private void reportInstrumentationFails(List<ProbeDefinition> definitions, String className) {
-    String msg = String.format(INSTRUMENTATION_FAILS, className);
+  private void reportInstrumentationFails(
+      List<ProbeDefinition> definitions, String className, String errorMsg) {
+    String msg = String.format(INSTRUMENTATION_FAILS, className, errorMsg);
     reportErrorForAllProbes(definitions, msg);
   }
 

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java

@@ -208,6 +208,10 @@ public void addBlocked(ProbeId probeId) {
     probeStatusSink.addBlocked(probeId);
   }
 
+  public void addError(ProbeId probeId, String msg) {
+    probeStatusSink.addError(probeId, msg);
+  }
+
   public void removeDiagnostics(ProbeId probeId) {
     probeStatusSink.removeDiagnostics(probeId);
   }

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java

@@ -2969,7 +2969,9 @@ public void methodParametersAttribute() throws IOException, URISyntaxException {
       ArgumentCaptor<String> strCaptor = ArgumentCaptor.forClass(String.class);
       verify(probeStatusSink, times(1)).addError(probeIdCaptor.capture(), strCaptor.capture());
       assertEquals(PROBE_ID.getId(), probeIdCaptor.getAllValues().get(0).getId());
-      assertEquals("Instrumentation fails for CapturedSnapshot01", strCaptor.getAllValues().get(0));
+      assertEquals(
+          "Instrumentation failed for CapturedSnapshot01: java.lang.RuntimeException: Method Parameters attribute detected, instrumentation not supported",
+          strCaptor.getAllValues().get(0));
     }
   }
 

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/ConfigurationUpdaterTest.java

@@ -651,6 +651,13 @@ public void methodParametersAttribute()
     } else {
       verify(inst).getAllLoadedClasses();
       verify(inst, times(0)).retransformClasses(any());
+      ArgumentCaptor<ProbeId> probeIdCaptor = ArgumentCaptor.forClass(ProbeId.class);
+      ArgumentCaptor<String> strCaptor = ArgumentCaptor.forClass(String.class);
+      verify(probeStatusSink, times(1)).addError(probeIdCaptor.capture(), strCaptor.capture());
+      assertEquals(PROBE_ID.getId(), probeIdCaptor.getAllValues().get(0).getId());
+      assertEquals(
+          "Method Parameters detected, instrumentation not supported for CapturedSnapshot01",
+          strCaptor.getAllValues().get(0));
     }
   }
 

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/DebuggerTransformerTest.java

@@ -338,9 +338,21 @@ public void classGenerationFailed() {
     assertEquals("logprobe1", probeIdCaptor.getAllValues().get(0).getId());
     assertEquals("logprobe2", probeIdCaptor.getAllValues().get(1).getId());
     assertEquals(PROBE_ID.getId(), probeIdCaptor.getAllValues().get(2).getId());
-    assertEquals("Instrumentation fails for " + CLASS_NAME, strCaptor.getAllValues().get(0));
-    assertEquals("Instrumentation fails for " + CLASS_NAME, strCaptor.getAllValues().get(1));
-    assertEquals("Instrumentation fails for " + CLASS_NAME, strCaptor.getAllValues().get(2));
+    assertEquals(
+        "Instrumentation failed for "
+            + CLASS_NAME
+            + ": java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0",
+        strCaptor.getAllValues().get(0));
+    assertEquals(
+        "Instrumentation failed for "
+            + CLASS_NAME
+            + ": java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0",
+        strCaptor.getAllValues().get(1));
+    assertEquals(
+        "Instrumentation failed for "
+            + CLASS_NAME
+            + ": java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0",
+        strCaptor.getAllValues().get(2));
   }
 
   @Test