Skip to content

Commit 311d3bd

Browse files
authored
Change MethodParameters detection reporting (#10849)
Change MethodParameters detection reporting When MethodParameters is detected we bail out instrumentation but report diagnostics message as error. Logging is now in debug instead of warning or error Co-authored-by: jean-philippe.bempel <jean-philippe.bempel@datadoghq.com>
1 parent fc35e2e commit 311d3bd

File tree

3 files changed

+24
-11
lines changed

3 files changed

+24
-11
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,9 @@ public byte[] transform(
275275
return null;
276276
}
277277
ClassNode classNode = parseClassFile(classFilePath, classfileBuffer);
278-
checkMethodParameters(classNode);
278+
if (!checkMethodParameters(classNode, definitions, fullyQualifiedClassName)) {
279+
return null;
280+
}
279281
if (!checkRecordTypeAnnotation(classNode, definitions, fullyQualifiedClassName)) {
280282
return null;
281283
}
@@ -306,10 +308,11 @@ public byte[] transform(
306308
* instrumented the class, we will retransform for removing the instrumentation and then the
307309
* attribute is stripped. That's why we are preventing it even at load time.
308310
*/
309-
private void checkMethodParameters(ClassNode classNode) {
311+
private boolean checkMethodParameters(
312+
ClassNode classNode, List<ProbeDefinition> definitions, String fullyQualifiedClassName) {
310313
if (JAVA_AT_LEAST_19) {
311314
// bug is fixed since JDK19, no need to perform check
312-
return;
315+
return true;
313316
}
314317
boolean isRecord = ASMHelper.isRecord(classNode);
315318
// capping scanning of methods to 100 to avoid generated class with thousand of methods
@@ -328,13 +331,17 @@ private void checkMethodParameters(ClassNode classNode) {
328331
if (methodNode.parameters != null
329332
&& !methodNode.parameters.isEmpty()
330333
&& SpringHelper.isSpringUsingOnlyMethodParameters(DebuggerAgent.getInstrumentation())) {
331-
throw new RuntimeException(
334+
reportInstrumentationFails(
335+
definitions,
336+
fullyQualifiedClassName,
332337
"Method Parameters attribute detected, instrumentation not supported");
338+
return false;
333339
} else {
334340
// we found at leat a method with one parameter if name is not present we can stop there
335341
break;
336342
}
337343
}
344+
return true;
338345
}
339346

340347
/*

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,11 @@ public void addDiagnostics(ProbeId probeId, List<DiagnosticMessage> messages) {
220220
for (DiagnosticMessage msg : messages) {
221221
switch (msg.getKind()) {
222222
case INFO:
223-
LOGGER.info(msg.getMessage());
224-
break;
225223
case WARN:
226-
LOGGER.warn(msg.getMessage());
224+
LOGGER.debug(msg.getMessage());
227225
break;
228226
case ERROR:
229-
LOGGER.error(msg.getMessage());
227+
LOGGER.debug(msg.getMessage());
230228
reportError(probeId, msg);
231229
break;
232230
}

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3059,7 +3059,7 @@ public void methodParametersAttribute() throws Exception {
30593059
verify(probeStatusSink, times(1)).addError(probeIdCaptor.capture(), strCaptor.capture());
30603060
assertEquals(PROBE_ID.getId(), probeIdCaptor.getAllValues().get(0).getId());
30613061
assertEquals(
3062-
"Instrumentation failed for CapturedSnapshot01: java.lang.RuntimeException: Method Parameters attribute detected, instrumentation not supported",
3062+
"Instrumentation failed for CapturedSnapshot01: Method Parameters attribute detected, instrumentation not supported",
30633063
strCaptor.getAllValues().get(0));
30643064
} else {
30653065
Snapshot snapshot = assertOneSnapshot(listener);
@@ -3069,9 +3069,17 @@ public void methodParametersAttribute() throws Exception {
30693069

30703070
@Test
30713071
@EnabledForJreRange(min = JRE.JAVA_17)
3072-
public void methodParametersAttributeRecord() throws IOException, URISyntaxException {
3072+
public void methodParametersAttributeRecord() throws Exception {
30733073
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot29";
30743074
final String RECORD_NAME = "com.datadog.debugger.MyRecord1";
3075+
Config config = mock(Config.class);
3076+
when(config.isDebuggerCodeOriginEnabled()).thenReturn(false);
3077+
when(config.isDebuggerExceptionEnabled()).thenReturn(false);
3078+
when(config.isDynamicInstrumentationEnabled()).thenReturn(false);
3079+
Instrumentation inst = mock(Instrumentation.class);
3080+
Class<?> springClass = Class.forName("org.springframework.core.SpringVersion");
3081+
when(inst.getAllLoadedClasses()).thenReturn(new Class[] {springClass});
3082+
DebuggerAgent.run(config, inst, null);
30753083
TestSnapshotListener listener = installMethodProbeAtExit(RECORD_NAME, "<init>", null);
30763084
Map<String, byte[]> buffers =
30773085
compile(CLASS_NAME, SourceCompiler.DebugInfo.ALL, "17", Arrays.asList("-parameters"));
@@ -3089,7 +3097,7 @@ public void methodParametersAttributeRecord() throws IOException, URISyntaxExcep
30893097
verify(probeStatusSink, times(1)).addError(probeIdCaptor.capture(), strCaptor.capture());
30903098
assertEquals(PROBE_ID.getId(), probeIdCaptor.getAllValues().get(0).getId());
30913099
assertEquals(
3092-
"Instrumentation failed for com.datadog.debugger.MyRecord1: java.lang.RuntimeException: Method Parameters attribute detected, instrumentation not supported",
3100+
"Instrumentation failed for com.datadog.debugger.MyRecord1: Method Parameters attribute detected, instrumentation not supported",
30933101
strCaptor.getAllValues().get(0));
30943102
}
30953103
}

0 commit comments

Comments
 (0)