Add special case for Record

canonical record constructor has method parameters attribute

Author: jpbempel

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

@@ -5,6 +5,7 @@
 import static java.util.stream.Collectors.toList;
 
 import com.datadog.debugger.el.ProbeCondition;
+import com.datadog.debugger.instrumentation.ASMHelper;
 import com.datadog.debugger.instrumentation.DiagnosticMessage;
 import com.datadog.debugger.instrumentation.InstrumentationResult;
 import com.datadog.debugger.instrumentation.MethodInfo;
@@ -294,6 +295,7 @@ private void checkMethodParameters(ClassNode classNode) {
       // bug is fixed since JDK19, no need to perform check
       return;
     }
+    boolean isRecord = ASMHelper.isRecord(classNode);
     // capping scanning of methods to 100 to avoid generated class with thousand of methods
     // assuming that in those first 100 methods there is at least one with at least one parameter
     for (int methodIdx = 0; methodIdx < classNode.methods.size() && methodIdx < 100; methodIdx++) {
@@ -302,6 +304,11 @@ private void checkMethodParameters(ClassNode classNode) {
       if (argumentCount == 0) {
         continue;
       }
+      if (isRecord && methodNode.name.equals("<init>")) {
+        // skip record constructors, cannot rely on them because of the canonical one
+        // use the equals method for this
+        continue;
+      }
       if (methodNode.parameters != null && !methodNode.parameters.isEmpty()) {
         throw new RuntimeException(
             "Method Parameters attribute detected, cannot instrument class " + classNode.name);

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ASMHelper.java

@@ -138,6 +138,10 @@ public static boolean isFinalField(Field field) {
     return Modifier.isFinal(field.getModifiers());
   }
 
+  public static boolean isRecord(ClassNode classNode) {
+    return (classNode.access & Opcodes.ACC_RECORD) > 0;
+  }
+
   public static void invokeStatic(
       InsnList insnList,
       org.objectweb.asm.Type owner,

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

@@ -2973,6 +2973,33 @@ public void methodParametersAttribute() throws IOException, URISyntaxException {
     }
   }
 
+  @Test
+  @EnabledForJreRange(min = JRE.JAVA_17)
+  public void methodParametersAttributeRecord() throws IOException, URISyntaxException {
+    final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot29";
+    final String RECORD_NAME = "com.datadog.debugger.MyRecord1";
+    TestSnapshotListener listener = installMethodProbeAtExit(RECORD_NAME, "<init>", null);
+    Map<String, byte[]> buffers =
+        compile(CLASS_NAME, SourceCompiler.DebugInfo.ALL, "17", Arrays.asList("-parameters"));
+    Class<?> testClass = loadClass(CLASS_NAME, buffers);
+    int result = Reflect.onClass(testClass).call("main", "1").get();
+    assertEquals(42, result);
+    if (JavaVirtualMachine.isJavaVersionAtLeast(19)) {
+      Snapshot snapshot = assertOneSnapshot(listener);
+      assertCaptureArgs(
+          snapshot.getCaptures().getReturn(), "firstName", String.class.getTypeName(), "john");
+    } else {
+      assertEquals(0, listener.snapshots.size());
+      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(
+          "Instrumentation fails for com.datadog.debugger.MyRecord1",
+          strCaptor.getAllValues().get(0));
+    }
+  }
+
   private TestSnapshotListener setupInstrumentTheWorldTransformer(
       String excludeFileName, String includeFileName) {
     Config config = mock(Config.class);

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

@@ -40,6 +40,8 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledForJreRange;
+import org.junit.jupiter.api.condition.JRE;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
@@ -640,8 +642,7 @@ public void methodParametersAttribute()
     ConfigurationUpdater configurationUpdater = createConfigUpdater(debuggerSinkWithMockStatusSink);
     configurationUpdater.accept(
         REMOTE_CONFIG,
-        singletonList(
-            LogProbe.builder().probeId(PROBE_ID).where("CapturedSnapshot01", "main").build()));
+        singletonList(LogProbe.builder().probeId(PROBE_ID).where(CLASS_NAME, "main").build()));
     if (JavaVirtualMachine.isJavaVersionAtLeast(19)) {
       ArgumentCaptor<Class<?>[]> captor = ArgumentCaptor.forClass(Class[].class);
       verify(inst, times(1)).retransformClasses(captor.capture());
@@ -653,6 +654,29 @@ public void methodParametersAttribute()
     }
   }
 
+  @Test
+  @EnabledForJreRange(min = JRE.JAVA_17)
+  public void methodParametersAttributeRecord()
+      throws IOException, URISyntaxException, UnmodifiableClassException {
+    // make sure record method are not detected as having methodParameters attribute.
+    // /!\ record canonical constructor has the MethodParameters attribute,
+    // but not returned by Class::getDeclaredMethods()
+    final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot29";
+    final String RECORD_NAME = "com.datadog.debugger.MyRecord1";
+    Map<String, byte[]> buffers = compile(CLASS_NAME, SourceCompiler.DebugInfo.ALL, "17");
+    Class<?> testClass = loadClass(RECORD_NAME, buffers);
+    when(inst.getAllLoadedClasses()).thenReturn(new Class[] {testClass});
+    ConfigurationUpdater configurationUpdater = createConfigUpdater(debuggerSinkWithMockStatusSink);
+    configurationUpdater.accept(
+        REMOTE_CONFIG,
+        singletonList(LogProbe.builder().probeId(PROBE_ID).where(RECORD_NAME, "<init>").build()));
+    verify(inst).getAllLoadedClasses();
+    ArgumentCaptor<Class<?>[]> captor = ArgumentCaptor.forClass(Class[].class);
+    verify(inst, times(1)).retransformClasses(captor.capture());
+    List<Class<?>[]> allValues = captor.getAllValues();
+    assertEquals(testClass, allValues.get(0));
+  }
+
   private DebuggerTransformer createTransformer(
       Config tracerConfig,
       Configuration configuration,