From c7b8be2f15510536dbc5e315f46a00050b36484d Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 4 Jul 2024 16:41:25 +0200 Subject: [PATCH] Remove explicit capture of fields Previously we were capturing directly fields of the current class in a specific way and use them for condition evaluation, while this was captured as argument. Now we only capture this and all evaluations are based on implicit this dereferencing when needed. NB: Serialization of complex classes no longer capture static fields --- .../bootstrap/debugger/CapturedContext.java | 67 ++--- .../debugger/el/ProbeConditionTest.java | 154 +++++----- .../debugger/el/RefResolverHelper.java | 17 +- .../el/expressions/HasAllExpressionTest.java | 2 +- .../el/expressions/HasAnyExpressionTest.java | 8 +- .../expressions/ValueRefExpressionTest.java | 40 ++- .../agent/JsonSnapshotSerializer.java | 23 -- .../CapturedContextInstrumentor.java | 111 ++----- .../debugger/util/SerializerWithLimits.java | 4 +- .../agent/CaptureAssertionHelper.java | 279 ------------------ .../debugger/agent/CapturedSnapshotTest.java | 46 +-- .../agent/LogProbesInstrumentationTest.java | 4 +- ...panDecorationProbeInstrumentationTest.java | 9 +- .../debugger/el/ELIntegrationSanityTest.java | 40 ++- .../util/MoshiSnapshotTestHelper.java | 25 +- .../debugger/util/StringTokenWriterTest.java | 2 +- .../datadog/debugger/CapturedSnapshot19.java | 4 + .../smoketest/LogProbesIntegrationTest.java | 2 - 18 files changed, 229 insertions(+), 608 deletions(-) delete mode 100644 dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CaptureAssertionHelper.java diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java index 3168d379a66..d96a9675c54 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java @@ -29,7 +29,6 @@ public class CapturedContext implements ValueReferenceResolver { private Map locals; private CapturedThrowable throwable; private Map staticFields; - private Map fields; private Limits limits = Limits.DEFAULT; private String thisClassName; private String traceId; @@ -43,20 +42,17 @@ public CapturedContext( CapturedValue[] arguments, CapturedValue[] locals, CapturedValue returnValue, - CapturedThrowable throwable, - CapturedValue[] fields) { + CapturedThrowable throwable) { addArguments(arguments); addLocals(locals); addReturn(returnValue); this.throwable = throwable; - addFields(fields); } private CapturedContext(CapturedContext other, Map extensions) { this.arguments = other.arguments; this.locals = other.getLocals(); this.throwable = other.throwable; - this.fields = other.fields; this.extensions.putAll(other.extensions); this.extensions.putAll(extensions); } @@ -168,15 +164,16 @@ private Object tryRetrieve(String name) { if (result != null) { return result; } - if (fields != null && !fields.isEmpty()) { - result = fields.get(name); - } - if (result != null) { - return result; - } if (staticFields != null && !staticFields.isEmpty()) { result = staticFields.get(name); } + CapturedValue thisValue; + if (arguments != null && (thisValue = arguments.get("this")) != null) { + result = getMember(thisValue.getValue(), name); + if (result != Values.UNDEFINED_OBJECT) { + return result; + } + } return result != null ? result : Values.UNDEFINED_OBJECT; } @@ -236,25 +233,15 @@ public void addStaticFields(CapturedValue[] values) { } } - public void addFields(CapturedValue[] values) { - if (values == null) { - return; - } - for (CapturedValue value : values) { - putInFields(value.name, value); - } - traceId = extractSpecialId("dd.trace_id"); - spanId = extractSpecialId("dd.span_id"); + public void addTraceId(CapturedValue capturedValue) { + traceId = extractStringId(capturedValue); } - private String extractSpecialId(String idName) { - if (fields == null) { - return null; - } - CapturedValue capturedValue = fields.get(idName); - if (capturedValue == null) { - return null; - } + public void addSpanId(CapturedValue capturedValue) { + spanId = extractStringId(capturedValue); + } + + private String extractStringId(CapturedValue capturedValue) { Object value = capturedValue.getValue(); return value instanceof String ? (String) value : null; } @@ -280,10 +267,6 @@ public Map getStaticFields() { return staticFields; } - public Map getFields() { - return fields; - } - public Limits getLimits() { return limits; } @@ -314,9 +297,6 @@ public void freeze(TimeoutChecker timeoutChecker) { if (staticFields != null) { staticFields.values().forEach(capturedValue -> capturedValue.freeze(timeoutChecker)); } - if (fields != null) { - fields.values().forEach(capturedValue -> capturedValue.freeze(timeoutChecker)); - } } public Status evaluate( @@ -359,13 +339,13 @@ public boolean equals(Object o) { CapturedContext context = (CapturedContext) o; return Objects.equals(arguments, context.arguments) && Objects.equals(locals, context.locals) - && Objects.equals(throwable, context.throwable) - && Objects.equals(fields, context.fields); + && Objects.equals(staticFields, context.staticFields) + && Objects.equals(throwable, context.throwable); } @Override public int hashCode() { - return Objects.hash(arguments, locals, throwable, fields); + return Objects.hash(arguments, locals, throwable, staticFields); } @Override @@ -377,8 +357,8 @@ public String toString() { + locals + ", throwable=" + throwable - + ", fields=" - + fields + + ", staticFields=" + + staticFields + '}'; } @@ -396,13 +376,6 @@ private void putInArguments(String name, CapturedValue value) { arguments.put(name, value); } - private void putInFields(String name, CapturedValue value) { - if (fields == null) { - fields = new HashMap<>(); - } - fields.put(name, value); - } - private void putInStaticFields(String name, CapturedValue value) { if (staticFields == null) { staticFields = new HashMap<>(); diff --git a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/ProbeConditionTest.java b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/ProbeConditionTest.java index d77e5e4247b..2372be7b861 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/ProbeConditionTest.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/ProbeConditionTest.java @@ -33,25 +33,23 @@ public class ProbeConditionTest { @Test void testExecuteCondition() throws Exception { ProbeCondition probeCondition = load("/test_conditional_01.json"); - - Collection tags = Arrays.asList("hello", "world", "ko"); - Map fields = new HashMap<>(); - fields.put("tags", tags); - fields.put("field", 10); - class Obj { + class Obj1 { + Collection tags = Arrays.asList("hello", "world", "ko"); + private int field = 10; List field2 = new ArrayList<>(); } ValueReferenceResolver ctx = - RefResolverHelper.createResolver(singletonMap("this", new Obj()), null, fields); + RefResolverHelper.createResolver(singletonMap("this", new Obj1()), null); assertTrue(probeCondition.execute(ctx)); - Collection tags2 = Arrays.asList("hey", "world", "ko"); - fields = new HashMap<>(); - fields.put("tags", tags2); - fields.put("field", 10); + class Obj2 { + Collection tags = Arrays.asList("hey", "world", "ko"); + private int field = 10; + List field2 = new ArrayList<>(); + } ValueReferenceResolver ctx2 = - RefResolverHelper.createResolver(singletonMap("this", new Obj()), null, fields); + RefResolverHelper.createResolver(singletonMap("this", new Obj2()), null); assertFalse(probeCondition.execute(ctx2)); } @@ -63,9 +61,7 @@ class Obj { } ValueReferenceResolver ctx = RefResolverHelper.createResolver( - singletonMap("this", new Obj()), - singletonMap("container", new Container("world")), - null); + singletonMap("this", new Obj()), singletonMap("container", new Container("world"))); assertTrue(probeCondition.execute(ctx)); class Obj2 { @@ -73,9 +69,7 @@ class Obj2 { } ValueReferenceResolver ctx2 = RefResolverHelper.createResolver( - singletonMap("this", new Obj2()), - singletonMap("container", new Container("world")), - null); + singletonMap("this", new Obj2()), singletonMap("container", new Container("world"))); RuntimeException runtimeException = assertThrows(RuntimeException.class, () -> probeCondition.execute(ctx2)); assertEquals("Cannot dereference to field: container", runtimeException.getMessage()); @@ -89,11 +83,7 @@ class Obj { String strField = "foo"; } Obj obj = new Obj(); - Map fields = new HashMap<>(); - fields.put("intField1", obj.intField1); - fields.put("strField", obj.strField); - ValueReferenceResolver ctx = - RefResolverHelper.createResolver(singletonMap("this", obj), null, fields); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(singletonMap("this", obj), null); assertTrue(probeCondition.execute(ctx)); } @@ -105,31 +95,39 @@ class Obj { } ValueReferenceResolver ctx = RefResolverHelper.createResolver( - singletonMap("this", new Obj()), singletonMap("nullField", null), null); + singletonMap("this", new Obj()), singletonMap("nullField", null)); assertTrue(probeCondition.execute(ctx)); } @Test void testIndex() throws Exception { ProbeCondition probeCondition = load("/test_conditional_07.json"); - Map fields = new HashMap<>(); - fields.put("intArray", new int[] {1, 1, 1}); - fields.put("strArray", new String[] {"foo", "bar"}); Map strMap = new HashMap<>(); - strMap.put("foo", "bar"); - strMap.put("bar", "foobar"); - fields.put("strMap", strMap); - fields.put("idx", 1); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, fields); + class Obj { + int[] intArray = new int[] {1, 1, 1}; + String[] strArray = new String[] {"foo", "bar"}; + Map strMap = new HashMap<>(); + + { + strMap.put("foo", "bar"); + strMap.put("bar", "foobar"); + } + + int idx = 1; + } + ValueReferenceResolver ctx = + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null); assertTrue(probeCondition.execute(ctx)); } @Test void testStringOperation() throws Exception { ProbeCondition probeCondition = load("/test_conditional_08.json"); - Map fields = new HashMap<>(); - fields.put("strField", "foobar"); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, fields); + class Obj { + String strField = "foobar"; + } + ValueReferenceResolver ctx = + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null); assertTrue(probeCondition.execute(ctx)); } @@ -148,9 +146,11 @@ void testJsonAdapter() throws IOException { @Test void testJsonParsing() throws IOException { ProbeCondition probeCondition = load("/test_conditional_02.json"); - Collection vets = Arrays.asList("vet1", "vet2", "vet3"); + class Obj { + Collection vets = Arrays.asList("vet1", "vet2", "vet3"); + } ValueReferenceResolver ctx = - RefResolverHelper.createResolver(null, null, singletonMap("vets", vets)); + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null); // the condition checks if length of vets > 2 assertTrue(probeCondition.execute(ctx)); @@ -192,7 +192,7 @@ void redaction() throws IOException { ProbeCondition probeCondition = load("/test_conditional_09.json"); Map args = new HashMap<>(); args.put("password", "secret123"); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(args, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(args, null); EvaluationException evaluationException = assertThrows(EvaluationException.class, () -> probeCondition.execute(ctx)); assertEquals( @@ -207,56 +207,70 @@ void stringPrimitives() throws IOException { args.put("uuid", UUID.fromString("a3cbe9e7-edd3-4bef-8e5b-59bfcb04cf91")); args.put("duration", Duration.ofSeconds(42)); args.put("clazz", "foo".getClass()); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(args, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(args, null); assertTrue(probeCondition.execute(ctx)); } @Test void testBooleanOperation() throws Exception { ProbeCondition probeCondition = load("/test_conditional_11.json"); - Map fields = new HashMap<>(); - fields.put("strField", "foobar"); - fields.put("emptyStr", ""); - fields.put("emptyList", new ArrayList<>()); - fields.put("emptyMap", new HashMap<>()); - fields.put("emptySet", new HashSet<>()); - fields.put("emptyArray", new Object[0]); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, fields); + class Obj { + String strField = "foobar"; + String emptyStr = ""; + List emptyList = new ArrayList<>(); + Map emptyMap = new HashMap<>(); + Set emptySet = new HashSet<>(); + Object[] emptyArray = new Object[0]; + } + ValueReferenceResolver ctx = + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null); assertTrue(probeCondition.execute(ctx)); } @Test void testLiterals() throws Exception { ProbeCondition probeCondition = load("/test_conditional_13.json"); - Map fields = new HashMap<>(); - fields.put("boolVal", true); - fields.put("intVal", 1); - fields.put("longVal", 1L); - fields.put("doubleVal", 1.0); - fields.put("strVal", "foo"); - fields.put("objVal", null); - fields.put("charVal", 'a'); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, fields); + class Obj { + boolean boolVal = true; + int intVal = 1; + long longVal = 1L; + double doubleVal = 1.0; + String strVal = "foo"; + Object objVal = null; + char charVal = 'a'; + } + ValueReferenceResolver ctx = + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null); assertTrue(probeCondition.execute(ctx)); } @Test void testLenCount() throws Exception { ProbeCondition probeCondition = load("/test_conditional_14.json"); - Map fields = new HashMap<>(); - fields.put("intArray", new int[] {1, 1, 1}); - fields.put("strArray", new String[] {"foo", "bar"}); - Map strMap = new HashMap<>(); - strMap.put("foo", "bar"); - strMap.put("bar", "foobar"); - fields.put("strMap", strMap); - Set strSet = new HashSet<>(); - strSet.add("foo"); - fields.put("strSet", strSet); - List strList = new ArrayList<>(); - strList.add("foo"); - fields.put("strList", strList); - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, fields); + class Obj { + int[] intArray = new int[] {1, 1, 1}; + String[] strArray = new String[] {"foo", "bar"}; + Map strMap = new HashMap<>(); + + { + strMap.put("foo", "bar"); + strMap.put("bar", "foobar"); + } + + Set strSet = new HashSet<>(); + + { + strSet.add("foo"); + } + + List strList = new ArrayList<>(); + + { + strList.add("foo"); + } + } + ValueReferenceResolver ctx = + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null); assertTrue(probeCondition.execute(ctx)); } diff --git a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/RefResolverHelper.java b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/RefResolverHelper.java index 714ae3c8b4e..c54b4f18f00 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/RefResolverHelper.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/RefResolverHelper.java @@ -3,12 +3,12 @@ import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.el.ValueReferenceResolver; import datadog.trace.bootstrap.debugger.util.Redaction; -import java.lang.reflect.Field; import java.util.*; public class RefResolverHelper { public static ValueReferenceResolver createResolver(Object instance) { + /* List fields = new ArrayList<>(); Class clazz = instance.getClass(); while (clazz != null) { @@ -33,11 +33,15 @@ public static ValueReferenceResolver createResolver(Object instance) { ex.printStackTrace(); } } - return new CapturedContext(null, null, null, null, fieldValues); + + */ + CapturedContext.CapturedValue thisValue = + CapturedContext.CapturedValue.of("this", instance.getClass().getTypeName(), instance); + return new CapturedContext(new CapturedContext.CapturedValue[] {thisValue}, null, null, null); } public static ValueReferenceResolver createResolver( - Map args, Map locals, Map fields) { + Map args, Map locals) { CapturedContext.CapturedValue[] argValues = null; if (args != null) { argValues = new CapturedContext.CapturedValue[args.size()]; @@ -48,12 +52,7 @@ public static ValueReferenceResolver createResolver( localValues = new CapturedContext.CapturedValue[locals.size()]; fillValues(locals, localValues); } - CapturedContext.CapturedValue[] fieldValues = null; - if (fields != null) { - fieldValues = new CapturedContext.CapturedValue[fields.size()]; - fillValues(fields, fieldValues); - } - return new CapturedContext(argValues, localValues, null, null, fieldValues); + return new CapturedContext(argValues, localValues, null, null); } private static void fillValues( diff --git a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAllExpressionTest.java b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAllExpressionTest.java index 517468802c1..53ad6e7b7e3 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAllExpressionTest.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAllExpressionTest.java @@ -153,7 +153,7 @@ void testListHasAll() { @Test void testMapHasAny() { - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null); Map valueMap = new HashMap<>(); valueMap.put("a", "a"); valueMap.put("b", "a"); diff --git a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAnyExpressionTest.java b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAnyExpressionTest.java index e3c2ddb8554..314bd2dc98e 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAnyExpressionTest.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/HasAnyExpressionTest.java @@ -75,7 +75,7 @@ void testUndefinedHasAny() { @Test void testSingleElementHasAny() { - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null); ValueExpression targetExpression = new ObjectValue(this); HasAnyExpression expression = any(targetExpression, TRUE); assertTrue(expression.evaluate(ctx)); @@ -101,7 +101,7 @@ void testSingleElementHasAny() { @Test void testArrayHasAny() { - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null); ValueExpression targetExpression = DSL.value(new Object[] {this, "hello"}); HasAnyExpression expression = any(targetExpression, TRUE); @@ -126,7 +126,7 @@ void testArrayHasAny() { @Test void testListHasAny() { - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null); ValueExpression targetExpression = DSL.value(Arrays.asList(this, "hello")); HasAnyExpression expression = any(targetExpression, TRUE); @@ -151,7 +151,7 @@ void testListHasAny() { @Test void testMapHasAny() { - ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null, null); + ValueReferenceResolver ctx = RefResolverHelper.createResolver(null, null); Map valueMap = new HashMap<>(); valueMap.put("a", "a"); valueMap.put("b", null); diff --git a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/ValueRefExpressionTest.java b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/ValueRefExpressionTest.java index 04653933f14..06eccb177dc 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/ValueRefExpressionTest.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/ValueRefExpressionTest.java @@ -3,6 +3,7 @@ import static com.datadog.debugger.el.DSL.*; import static com.datadog.debugger.el.PrettyPrintVisitor.print; import static com.datadog.debugger.el.TestHelper.setFieldInConfig; +import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.*; import com.datadog.debugger.el.DSL; @@ -48,31 +49,23 @@ void testPredicatedRef() { RuntimeException runtimeException = assertThrows(RuntimeException.class, () -> isEmptyInvalid.evaluate(ctx)); - assertEquals("Cannot find symbol: x", runtimeException.getMessage()); + assertEquals("Cannot dereference to field: x", runtimeException.getMessage()); runtimeException = assertThrows(RuntimeException.class, () -> and(isEmptyInvalid, isEmpty).evaluate(ctx)); - assertEquals("Cannot find symbol: x", runtimeException.getMessage()); + assertEquals("Cannot dereference to field: x", runtimeException.getMessage()); runtimeException = assertThrows(RuntimeException.class, () -> or(isEmptyInvalid, isEmpty).evaluate(ctx)); - assertEquals("Cannot find symbol: x", runtimeException.getMessage()); + assertEquals("Cannot dereference to field: x", runtimeException.getMessage()); assertEquals("isEmpty(x)", print(isEmptyInvalid)); } @Test void contextRef() { - ExObjectWithRefAndValue instance = new ExObjectWithRefAndValue(null, "hello"); - long limit = 511L; - String msg = "Hello there"; - int i = 6; - - String limitArg = "limit"; - String msgArg = "msg"; - String iVar = "i"; - - Map values = new HashMap<>(); - values.put(limitArg, limit); - values.put(msgArg, msg); - values.put(iVar, i); + class Obj { + long limit = 511L; + String msg = "Hello there"; + int i = 6; + } long duration = TimeUnit.NANOSECONDS.convert(680, TimeUnit.MILLISECONDS); boolean returnVal = true; @@ -82,7 +75,8 @@ void contextRef() { exts.put(ValueReferences.DURATION_EXTENSION_NAME, duration); exts.put(ValueReferences.EXCEPTION_EXTENSION_NAME, exception); ValueReferenceResolver resolver = - RefResolverHelper.createResolver(null, null, values).withExtensions(exts); + RefResolverHelper.createResolver(singletonMap("this", new Obj()), null) + .withExtensions(exts); ValueRefExpression expression = DSL.ref(ValueReferences.DURATION_REF); assertEquals(duration, expression.evaluate(resolver).getValue()); @@ -93,15 +87,15 @@ void contextRef() { expression = DSL.ref(ValueReferences.EXCEPTION_REF); assertEquals(exception, expression.evaluate(resolver).getValue()); assertEquals("@exception", print(expression)); - expression = DSL.ref(limitArg); - assertEquals(limit, expression.evaluate(resolver).getValue()); + expression = DSL.ref("limit"); + assertEquals(511L, expression.evaluate(resolver).getValue()); assertEquals("limit", print(expression)); - expression = DSL.ref(msgArg); - assertEquals(msg, expression.evaluate(resolver).getValue()); + expression = DSL.ref("msg"); + assertEquals("Hello there", expression.evaluate(resolver).getValue()); assertEquals("msg", print(expression)); - expression = DSL.ref(iVar); + expression = DSL.ref("i"); assertEquals( - (long) i, expression.evaluate(resolver).getValue()); // int value is widened to long + (long) 6, expression.evaluate(resolver).getValue()); // int value is widened to long assertEquals("i", print(expression)); ValueRefExpression invalidExpression = ref(ValueReferences.synthetic("invalid")); RuntimeException runtimeException = diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/JsonSnapshotSerializer.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/JsonSnapshotSerializer.java index f2627d581eb..c0d3a104f36 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/JsonSnapshotSerializer.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/JsonSnapshotSerializer.java @@ -8,7 +8,6 @@ import datadog.trace.api.Config; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.DebuggerContext; -import java.util.Map; /** Serializes snapshots in Json using Moshi */ public class JsonSnapshotSerializer implements DebuggerContext.ValueSerializer { @@ -47,28 +46,6 @@ private void handleDuration(Snapshot snapshot, IntakeRequest request) { private void handleCorrelationFields(Snapshot snapshot, IntakeRequest request) { request.traceId = snapshot.getTraceId(); request.spanId = snapshot.getSpanId(); - CapturedContext entry = snapshot.getCaptures().getEntry(); - if (entry != null) { - removeTraceSpanId(entry); - } - if (snapshot.getCaptures().getLines() != null) { - for (CapturedContext context : snapshot.getCaptures().getLines().values()) { - removeTraceSpanId(context); - } - } - removeTraceSpanId(snapshot.getCaptures().getReturn()); - } - - private void removeTraceSpanId(CapturedContext context) { - if (context == null) { - return; - } - Map fields = context.getFields(); - if (fields == null) { - return; - } - fields.remove(DD_TRACE_ID); - fields.remove(DD_SPAN_ID); } public static class IntakeRequest { diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java index 39bec13b85e..48a86ffab6d 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java @@ -552,7 +552,7 @@ private InsnList collectCapturedContext(Snapshot.Kind kind, AbstractInsnNode loc // stack: [capturedcontext] collectStaticFields(insnList); // stack: [capturedcontext] - collectFields(insnList); + collectSpecialFields(insnList); // stack: [capturedcontext] if (kind != Snapshot.Kind.UNHANDLED_EXCEPTION) { /* @@ -857,7 +857,7 @@ private void collectStaticFields(InsnList insnList) { // stack: [capturedcontext] } - private void collectFields(InsnList insnList) { + private void collectSpecialFields(InsnList insnList) { // expected stack top: [capturedcontext] /* * We are cheating a bit with CorrelationAccess - utilizing the knowledge that it is a singleton loaded by the @@ -869,96 +869,27 @@ private void collectFields(InsnList insnList) { // static method and no correlation info, no need to capture fields return; } - List fieldsToCapture = - extractInstanceField(classNode, isStatic, classLoader, limits); - if (fieldsToCapture.isEmpty()) { - // bail out if no fields - return; - } + extractSpecialId(insnList, "dd.trace_id", "getTraceId", "addTraceId"); + // stack: [capturedcontext] + extractSpecialId(insnList, "dd.span_id", "getSpanId", "addSpanId"); + // stack: [capturedcontext] + } + + private void extractSpecialId( + InsnList insnList, String fieldName, String getMethodName, String addMethodName) { insnList.add(new InsnNode(Opcodes.DUP)); // stack: [capturedcontext, capturedcontext] - ldc(insnList, fieldsToCapture.size()); - // stack: [capturedcontext, capturedcontext, int] - insnList.add(new TypeInsnNode(Opcodes.ANEWARRAY, CAPTURED_VALUE.getInternalName())); - // stack: [capturedcontext, capturedcontext, array] - int counter = 0; - for (FieldNode fieldNode : fieldsToCapture) { - insnList.add(new InsnNode(Opcodes.DUP)); - // stack: [capturedcontext, capturedcontext, array, array] - ldc(insnList, counter++); - // stack: [capturedcontext, capturedcontext, array, array, int] - if (!isAccessible(fieldNode)) { - insnList.add(new VarInsnNode(Opcodes.ALOAD, 0)); - // stack: [capturedcontext, capturedcontext, array, array, int, this] - ldc(insnList, fieldNode.name); - // stack: [capturedcontext, capturedcontext, array, array, int, this, string] - invokeStatic( - insnList, - REFLECTIVE_FIELD_VALUE_RESOLVER_TYPE, - "getFieldAsCapturedValue", - CAPTURED_VALUE, - OBJECT_TYPE, - STRING_TYPE); - // stack: [capturedcontext, capturedcontext, array, array, int, CapturedValue] - insnList.add(new InsnNode(Opcodes.AASTORE)); - // stack: [capturedcontext, capturedcontext, array] - continue; - } - ldc(insnList, fieldNode.name); - // stack: [capturedcontext, capturedcontext, array, array, int, string] - Type fieldType = Type.getType(fieldNode.desc); - ldc(insnList, fieldType.getClassName()); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name] - if (Redaction.isRedactedKeyword(fieldNode.name)) { - addCapturedValueRedacted(insnList); - } else { - switch (fieldNode.name) { - case "dd.trace_id": - { - invokeStatic(insnList, CORRELATION_ACCESS_TYPE, "instance", CORRELATION_ACCESS_TYPE); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, - // access] - invokeVirtual(insnList, CORRELATION_ACCESS_TYPE, "getTraceId", STRING_TYPE); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, - // field_value] - break; - } - case "dd.span_id": - { - invokeStatic(insnList, CORRELATION_ACCESS_TYPE, "instance", CORRELATION_ACCESS_TYPE); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, - // access] - invokeVirtual(insnList, CORRELATION_ACCESS_TYPE, "getSpanId", STRING_TYPE); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, - // field_value] - break; - } - default: - { - insnList.add(new VarInsnNode(Opcodes.ALOAD, 0)); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, - // this] - insnList.add( - new FieldInsnNode( - Opcodes.GETFIELD, classNode.name, fieldNode.name, fieldNode.desc)); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, - // field_value] - } - } - tryBox(fieldType, insnList); - // stack: [capturedcontext, capturedcontext, array, array, int, string, type_name, object] - addCapturedValueOf(insnList, limits); - } - // stack: [capturedcontext, capturedcontext, array, array, int, CapturedValue] - insnList.add(new InsnNode(Opcodes.AASTORE)); - // stack: [capturedcontext, capturedcontext, array] - } - invokeVirtual( - insnList, - CAPTURED_CONTEXT_TYPE, - "addFields", - Type.VOID_TYPE, - Types.asArray(CAPTURED_VALUE, 1)); + ldc(insnList, fieldName); + // stack: [capturedcontext, capturedcontext, name] + ldc(insnList, STRING_TYPE.getClassName()); + // stack: [capturedcontext, capturedcontext, name, type_name] + invokeStatic(insnList, CORRELATION_ACCESS_TYPE, "instance", CORRELATION_ACCESS_TYPE); + // stack: [capturedcontext, capturedcontext, name, type_name, access] + invokeVirtual(insnList, CORRELATION_ACCESS_TYPE, getMethodName, STRING_TYPE); + // stack: [capturedcontext, capturedcontext, name, type_name, id] + addCapturedValueOf(insnList, limits); + // stack: [capturedcontext, capturedcontext, captured_value] + invokeVirtual(insnList, CAPTURED_CONTEXT_TYPE, addMethodName, Type.VOID_TYPE, CAPTURED_VALUE); // stack: [capturedcontext] } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java index 86188d500e2..c39ce9ab0f6 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java @@ -91,8 +91,8 @@ default boolean objectFilterInField(Field field) throws Exception { if ("$jacocoData".equals(field.getName()) && Modifier.isTransient(field.getModifiers())) { return false; } - // skip constant fields - if (Modifier.isStatic(field.getModifiers()) && Modifier.isFinal(field.getModifiers())) { + // skip static fields + if (Modifier.isStatic(field.getModifiers())) { return false; } return true; diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CaptureAssertionHelper.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CaptureAssertionHelper.java deleted file mode 100644 index 5323545131d..00000000000 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CaptureAssertionHelper.java +++ /dev/null @@ -1,279 +0,0 @@ -package com.datadog.debugger.agent; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; - -import com.datadog.debugger.sink.Snapshot; -import datadog.trace.bootstrap.debugger.CapturedContext; -import java.util.EnumSet; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import utils.SourceCompiler; - -class CaptureAssertionHelper { - private static final CapturedContext.CapturedThrowable HANDLED_EXCEPTION = - new CapturedContext.CapturedThrowable(new IllegalArgumentException()); - private static final CapturedContext.CapturedThrowable UNHANDLED_EXCEPTION = - new CapturedContext.CapturedThrowable(new RuntimeException()); - - private final Map - enterArgs = new HashMap<>(); - private final Map - returnArgs = new HashMap<>(); - private final Map - returnValues = new HashMap<>(); - private CapturedContext.CapturedValue[] beforeLocals = null; - private CapturedContext.CapturedValue[] afterLocals = null; - private CapturedContext.CapturedValue[] returnLocals = null; - - private final DebuggerTransformerTest.InstrumentationKind instrumentationKind; - private final SourceCompiler.DebugInfo debugInfo; - private final String argumentType; - private final Object argumentInputValue; - private final Object argumentOutputValue; - private final String returnType; - private final Object returnValue; - private final String[] expectedArgNames; - private final int expectedLineFrom; - private final int expectedLineTill; - private final CapturedContext.CapturedValue[] correlationFields; - private final List snapshots; - - CaptureAssertionHelper( - DebuggerTransformerTest.InstrumentationKind instrumentationKind, - SourceCompiler.DebugInfo debugInfo, - String argumentType, - Object argumentInputValue, - Object argumentOutputValue, - String returnType, - Object returnValue, - int expectedLineFrom, - int expectedLineTill, - CapturedContext.CapturedValue[] correlationFields, - List snapshots) { - this.instrumentationKind = instrumentationKind; - this.debugInfo = debugInfo; - this.argumentType = argumentType; - this.argumentInputValue = argumentInputValue; - this.argumentOutputValue = argumentOutputValue; - this.returnType = returnType; - this.returnValue = returnValue; - this.expectedLineFrom = expectedLineFrom; - this.expectedLineTill = expectedLineTill; - this.correlationFields = correlationFields; - this.snapshots = snapshots; - - this.expectedArgNames = - hasVariableInfo(debugInfo) ? new String[] {"arg1", "switcher"} : new String[] {"p0", "p1"}; - - setupArguments(); - setupLocals(instrumentationKind); - setupReturnValues(); - } - - void assertCaptures(DebuggerTransformerTest.ExceptionKind exceptionKind) { - // Map> captureMap = getCaptureMap(exceptionKind); - Snapshot.Captures captures = snapshots.get(exceptionKind.ordinal()).getCaptures(); - assertEntryExit(exceptionKind, captures); - assertExceptionHandling(exceptionKind, captures); - assertLines(exceptionKind, captures); - } - - private void assertEntryExit( - DebuggerTransformerTest.ExceptionKind exceptionKind, Snapshot.Captures captures) { - CapturedContext expectedEntryContext = - new CapturedContext(enterArgs.get(exceptionKind), null, null, null, correlationFields); - if (expectedLineFrom != -1) { - // no entry/exit for line probe - return; - } - addThis(expectedEntryContext, captures.getEntry()); - assertEquals(expectedEntryContext, captures.getEntry()); - if (exceptionKind != DebuggerTransformerTest.ExceptionKind.UNHANDLED) { - // no return context is captured for unhandled exceptions - CapturedContext expectedExitContext = - new CapturedContext( - returnArgs.get(exceptionKind), - returnLocals, - returnValues.get(exceptionKind), - null, - correlationFields); - addThis(expectedExitContext, captures.getReturn()); - assertEquals(expectedExitContext, captures.getReturn()); - } else { - assertNotNull(captures.getReturn().getCapturedThrowable()); - } - } - - private void addThis(CapturedContext expected, CapturedContext context) { - if (context.getArguments().containsKey("this")) { - expected.getArguments().put("this", context.getArguments().get("this")); - } - } - - private void assertExceptionHandling( - DebuggerTransformerTest.ExceptionKind exceptionKind, Snapshot.Captures captures) { - if (expectedLineFrom != -1) { - // no exception for line probe - return; - } - if (exceptionKind == DebuggerTransformerTest.ExceptionKind.UNHANDLED) { - CapturedContext expectedUnhandled = - new CapturedContext( - returnArgs.get(exceptionKind), null, null, UNHANDLED_EXCEPTION, correlationFields); - assertEquals( - expectedUnhandled.getCapturedThrowable().getMessage(), - captures.getReturn().getCapturedThrowable().getMessage()); - assertEquals( - expectedUnhandled.getCapturedThrowable().getType(), - captures.getReturn().getCapturedThrowable().getType()); - } else { - assertNull(captures.getReturn().getCapturedThrowable()); - } - if (exceptionKind == DebuggerTransformerTest.ExceptionKind.HANDLED) { - assertEquals( - HANDLED_EXCEPTION.getMessage(), captures.getCaughtExceptions().get(0).getMessage()); - assertEquals(HANDLED_EXCEPTION.getType(), captures.getCaughtExceptions().get(0).getType()); - } else { - assertNull(captures.getCaughtExceptions()); - } - } - - private void assertLines( - DebuggerTransformerTest.ExceptionKind exceptionKind, Snapshot.Captures captures) { - boolean isSingleline = expectedLineFrom == expectedLineTill; - if (instrumentationKind != DebuggerTransformerTest.InstrumentationKind.ENTRY_EXIT - && hasLineInfo(debugInfo)) { - CapturedContext expectedBefore = - new CapturedContext( - enterArgs.get(exceptionKind), beforeLocals, null, null, correlationFields); - assertEquals(expectedBefore, captures.getLines().get(expectedLineFrom)); - - if (!isSingleline - && (exceptionKind == DebuggerTransformerTest.ExceptionKind.NONE - || instrumentationKind == DebuggerTransformerTest.InstrumentationKind.LINE)) { - CapturedContext expectedAfter = - new CapturedContext( - enterArgs.get(exceptionKind), afterLocals, null, null, correlationFields); - assertEquals(expectedAfter, captures.getLines().get(expectedLineTill)); - } else { - // unhandled exception will cause jumping out before hitting the last line in the range - if (!isSingleline) { - assertNull(captures.getLines().get(expectedLineTill)); - } - } - } else { - assertNull(captures.getLines()); - } - } - - private void setupReturnValues() { - for (DebuggerTransformerTest.ExceptionKind exceptionKind : - EnumSet.allOf(DebuggerTransformerTest.ExceptionKind.class)) { - switch (exceptionKind) { - case HANDLED: - case NONE: - { - returnValues.put( - exceptionKind, CapturedContext.CapturedValue.of(returnType, returnValue)); - break; - } - } - } - } - - private void setupLocals(DebuggerTransformerTest.InstrumentationKind instrumentationKind) { - if (hasVariableInfo(debugInfo)) { - switch (instrumentationKind) { - case LINE: - { - returnLocals = - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.VAR_NAME, argumentType, argumentOutputValue) - }; - beforeLocals = - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.SCOPED_VAR_NAME, - DebuggerTransformerTest.SCOPED_VAR_TYPE, - DebuggerTransformerTest.SCOPED_VAR_VALUE), - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.VAR_NAME, argumentType, argumentInputValue) - }; - afterLocals = - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.SCOPED_VAR_NAME, - DebuggerTransformerTest.SCOPED_VAR_TYPE, - DebuggerTransformerTest.SCOPED_VAR_VALUE), - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.VAR_NAME, argumentType, argumentOutputValue) - }; - break; - } - case ENTRY_EXIT: - { - returnLocals = - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.VAR_NAME, argumentType, argumentOutputValue) - }; - beforeLocals = - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.VAR_NAME, argumentType, argumentInputValue) - }; - afterLocals = - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of( - DebuggerTransformerTest.VAR_NAME, argumentType, argumentOutputValue) - }; - break; - } - } - } - } - - private void setupArguments() { - for (DebuggerTransformerTest.ExceptionKind exceptionKind : - EnumSet.allOf(DebuggerTransformerTest.ExceptionKind.class)) { - enterArgs.put( - exceptionKind, - new CapturedContext.CapturedValue[] { - CapturedContext.CapturedValue.of(expectedArgNames[0], argumentType, argumentInputValue), - CapturedContext.CapturedValue.of(expectedArgNames[1], "int", exceptionKind.ordinal()) - }); - returnArgs.put( - exceptionKind, - new CapturedContext.CapturedValue[] { - // the first argument will not get mutated if an exception is thrown before reaching the - // line of code - CapturedContext.CapturedValue.of( - expectedArgNames[0], - argumentType, - exceptionKind == DebuggerTransformerTest.ExceptionKind.NONE - ? argumentOutputValue - : argumentInputValue), - // exception handler will overwrite the second argument with value of -1 - CapturedContext.CapturedValue.of( - expectedArgNames[1], - "int", - exceptionKind == DebuggerTransformerTest.ExceptionKind.HANDLED - ? -1 - : exceptionKind.ordinal()) - }); - } - } - - private static boolean hasVariableInfo(SourceCompiler.DebugInfo debugInfo) { - return debugInfo == SourceCompiler.DebugInfo.ALL - || debugInfo == SourceCompiler.DebugInfo.VARIABLES; - } - - private static boolean hasLineInfo(SourceCompiler.DebugInfo debugInfo) { - return debugInfo == SourceCompiler.DebugInfo.ALL || debugInfo == SourceCompiler.DebugInfo.LINES; - } -} diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java index b2f14d4bbea..5a9e85d1f6d 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java @@ -296,10 +296,12 @@ public void inheritedConstructor() throws Exception { Snapshot snapshot = assertOneSnapshot(listener); assertCaptureFields( snapshot.getCaptures().getEntry(), "obj2", "java.lang.Object", (String) null); - CapturedContext.CapturedValue obj2 = snapshot.getCaptures().getReturn().getFields().get("obj2"); - Map fields = getFields(obj2); - assertEquals(24, fields.get("intValue").getValue()); - assertEquals(3.14, fields.get("doubleValue").getValue()); + CapturedContext.CapturedValue obj2 = + getFields(snapshot.getCaptures().getReturn().getArguments().get("this")).get("obj2"); + Map obj2Fields = + (Map) obj2.getValue(); + assertEquals(24, obj2Fields.get("intValue").getValue()); + assertEquals(3.14, obj2Fields.get("doubleValue").getValue()); } @Test @@ -1058,7 +1060,7 @@ public void lineProbeCondition() throws IOException, URISyntaxException { public void staticFieldCondition() throws IOException, URISyntaxException { final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot19"; LogProbe logProbe = - createProbeBuilder(PROBE_ID, CLASS_NAME, "main", "int (java.lang.String)") + createProbeBuilder(PROBE_ID, CLASS_NAME, "process", "int (java.lang.String)") .when( new ProbeCondition( DSL.when(DSL.eq(DSL.ref("strField"), DSL.value("foo"))), "strField == 'foo'")) @@ -1162,11 +1164,11 @@ public void wellKnownClassesCondition() throws IOException, URISyntaxException { assertEquals(3, result); Snapshot snapshot = assertOneSnapshot(listener); CapturedContext.CapturedValue maybeStrField = - snapshot.getCaptures().getReturn().getFields().get("maybeStr"); - assertEquals(Optional.class.getTypeName(), maybeStrField.getType()); - CapturedContext.CapturedValue valued = VALUE_ADAPTER.fromJson(maybeStrField.getStrValue()); - CapturedContext.CapturedValue value = - ((Map) valued.getValue()).get("value"); + getFields(snapshot.getCaptures().getReturn().getArguments().get("this")).get("maybeStr"); + assertEquals(Optional.class.getTypeName(), maybeStrField.getDeclaredType()); + Map maybeStrFields = + (Map) maybeStrField.getValue(); + CapturedContext.CapturedValue value = maybeStrFields.get("value"); assertEquals("maybe foo", value.getValue()); } @@ -1343,7 +1345,7 @@ public void fields() throws IOException, URISyntaxException { int result = Reflect.onClass(testClass).call("main", "f").get(); assertEquals(42, result); Snapshot snapshot = assertOneSnapshot(listener); - assertCaptureFieldCount(snapshot.getCaptures().getEntry(), 7); // +2 for correlation ids + assertCaptureFieldCount(snapshot.getCaptures().getEntry(), 5); assertCaptureFields(snapshot.getCaptures().getEntry(), "intValue", "int", "24"); assertCaptureFields(snapshot.getCaptures().getEntry(), "doubleValue", "double", "3.14"); assertCaptureFields( @@ -1355,7 +1357,7 @@ public void fields() throws IOException, URISyntaxException { Arrays.asList("foo", "bar")); assertCaptureFields( snapshot.getCaptures().getEntry(), "strMap", "java.util.HashMap", Collections.emptyMap()); - assertCaptureFieldCount(snapshot.getCaptures().getReturn(), 7); // +2 for correlation ids + assertCaptureFieldCount(snapshot.getCaptures().getReturn(), 5); assertCaptureFields(snapshot.getCaptures().getReturn(), "intValue", "int", "48"); assertCaptureFields(snapshot.getCaptures().getReturn(), "doubleValue", "double", "3.14"); assertCaptureFields(snapshot.getCaptures().getReturn(), "strValue", "java.lang.String", "done"); @@ -1384,11 +1386,11 @@ public void inheritedFields() throws IOException, URISyntaxException { assertEquals(42, result); Snapshot snapshot = assertOneSnapshot(listener); // Only Declared fields in the current class are captured, not inherited fields - assertCaptureFieldCount(snapshot.getCaptures().getEntry(), 7); // +2 for correlation ids + assertCaptureFieldCount(snapshot.getCaptures().getEntry(), 5); assertCaptureFields( snapshot.getCaptures().getEntry(), "strValue", "java.lang.String", "foobar"); assertCaptureFields(snapshot.getCaptures().getEntry(), "intValue", "int", "24"); - assertCaptureFieldCount(snapshot.getCaptures().getReturn(), 7); // +2 for correlation ids + assertCaptureFieldCount(snapshot.getCaptures().getReturn(), 5); assertCaptureFields( snapshot.getCaptures().getReturn(), "strValue", "java.lang.String", "barfoo"); assertCaptureFields(snapshot.getCaptures().getEntry(), "intValue", "int", "24"); @@ -2372,14 +2374,14 @@ private void assertCaptureLocals( private void assertCaptureFields( CapturedContext context, String name, String typeName, String value) { - CapturedContext.CapturedValue field = context.getFields().get(name); + CapturedContext.CapturedValue field = getFields(context.getArguments().get("this")).get(name); assertEquals(typeName, field.getType()); assertEquals(value, MoshiSnapshotTestHelper.getValue(field)); } private void assertCaptureFields( CapturedContext context, String name, String typeName, Collection collection) { - CapturedContext.CapturedValue field = context.getFields().get(name); + CapturedContext.CapturedValue field = getFields(context.getArguments().get("this")).get(name); assertEquals(typeName, field.getType()); Iterator iterator = collection.iterator(); for (Object obj : getCollection(field)) { @@ -2393,7 +2395,7 @@ private void assertCaptureFields( private void assertCaptureFields( CapturedContext context, String name, String typeName, Map expectedMap) { - CapturedContext.CapturedValue field = context.getFields().get(name); + CapturedContext.CapturedValue field = getFields(context.getArguments().get("this")).get(name); assertEquals(typeName, field.getType()); Map map = getMap(field); assertEquals(expectedMap.size(), map.size()); @@ -2405,7 +2407,7 @@ private void assertCaptureFields( private void assertCaptureFieldsNotCaptured( CapturedContext context, String name, String expectedReasonRegEx) { - CapturedContext.CapturedValue field = context.getFields().get(name); + CapturedContext.CapturedValue field = getFields(context.getArguments().get("this")).get(name); assertTrue( field.getNotCapturedReason().matches(expectedReasonRegEx), field.getNotCapturedReason()); } @@ -2418,7 +2420,7 @@ private void assertCaptureStaticFieldsNotCaptured( } private void assertCaptureFieldCount(CapturedContext context, int expectedFieldCount) { - assertEquals(expectedFieldCount, context.getFields().size()); + assertEquals(expectedFieldCount, getFields(context.getArguments().get("this")).size()); } private void assertCaptureReturnValue(CapturedContext context, String typeName, String value) { @@ -2488,6 +2490,9 @@ public static Map getFields( private static Collection getCollection(CapturedContext.CapturedValue capturedValue) { try { + if (capturedValue.getStrValue() == null) { + return (Collection) capturedValue.getValue(); + } Map capturedValueMap = GENERIC_ADAPTER.fromJson(capturedValue.getStrValue()); List elements = (List) capturedValueMap.get("elements"); if (elements == null) { @@ -2515,6 +2520,9 @@ private static Collection getCollection(CapturedContext.CapturedValue capture private Map getMap(CapturedContext.CapturedValue capturedValue) { try { + if (capturedValue.getStrValue() == null) { + return (Map) capturedValue.getValue(); + } Map capturedValueMap = GENERIC_ADAPTER.fromJson(capturedValue.getStrValue()); List entries = (List) capturedValueMap.get("entries"); if (entries == null) { diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java index ff28e33af44..f3c6642d34f 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java @@ -378,7 +378,7 @@ public void lineTemplateThisLog() throws IOException, URISyntaxException { Assertions.assertEquals(42, result); Snapshot snapshot = assertOneSnapshot(listener); assertEquals( - "this is log line for this={STATIC_STR=strStatic, intValue=48, doubleValue=3.14, strValue=done, strList=...}, ...", + "this is log line for this={intValue=48, doubleValue=3.14, strValue=done, strList=..., strMap=...}", snapshot.getMessage()); } @@ -404,7 +404,7 @@ public void conditionWithLogTemplateEvalError() throws IOException, URISyntaxExc Assertions.assertEquals(2, snapshot.getCaptures().getEntry().getArguments().size()); Assertions.assertEquals(1, snapshot.getEvaluationErrors().size()); Assertions.assertEquals( - "Cannot find symbol: typoArg", snapshot.getEvaluationErrors().get(0).getMessage()); + "Cannot dereference to field: typoArg", snapshot.getEvaluationErrors().get(0).getMessage()); } @Test diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java index ea91a721f41..98c4f59c7f0 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java @@ -164,7 +164,8 @@ public void methodTagEvalError() throws IOException, URISyntaxException { assertEquals(84, result); MutableSpan span = traceInterceptor.getFirstSpan(); assertNull(span.getTags().get("tag1")); - assertEquals("Cannot find symbol: noarg", span.getTags().get("_dd.di.tag1.evaluation_error")); + assertEquals( + "Cannot dereference to field: noarg", span.getTags().get("_dd.di.tag1.evaluation_error")); } @Test @@ -181,7 +182,8 @@ public void methodActiveSpanInvalidCondition() throws IOException, URISyntaxExce assertEquals(1, mockSink.getSnapshots().size()); Snapshot snapshot = mockSink.getSnapshots().get(0); assertEquals(1, snapshot.getEvaluationErrors().size()); - assertEquals("Cannot find symbol: noarg", snapshot.getEvaluationErrors().get(0).getMessage()); + assertEquals( + "Cannot dereference to field: noarg", snapshot.getEvaluationErrors().get(0).getMessage()); } @Test @@ -295,7 +297,8 @@ public void lineActiveSpanInvalidCondition() throws IOException, URISyntaxExcept assertEquals(1, mockSink.getSnapshots().size()); Snapshot snapshot = mockSink.getSnapshots().get(0); assertEquals(1, snapshot.getEvaluationErrors().size()); - assertEquals("Cannot find symbol: noarg", snapshot.getEvaluationErrors().get(0).getMessage()); + assertEquals( + "Cannot dereference to field: noarg", snapshot.getEvaluationErrors().get(0).getMessage()); } @Test diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/el/ELIntegrationSanityTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/el/ELIntegrationSanityTest.java index 420fda4f009..9ac26df3239 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/el/ELIntegrationSanityTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/el/ELIntegrationSanityTest.java @@ -1,14 +1,13 @@ package com.datadog.debugger.el; +import static com.datadog.debugger.agent.CapturedSnapshotTest.getFields; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; import com.datadog.debugger.agent.JsonSnapshotSerializer; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.DebuggerContext; import datadog.trace.bootstrap.debugger.Limits; import datadog.trace.bootstrap.debugger.util.TimeoutChecker; -import java.lang.reflect.Field; import java.time.Duration; import java.time.temporal.ChronoUnit; import java.util.ArrayList; @@ -45,25 +44,19 @@ void extractAfterEl() throws IllegalAccessException { DebuggerContext.initValueSerializer(serializer); Person p = new Person(); // set the limit not to follow references to fields - Limits initialLimits = new Limits(1, 0, Integer.MAX_VALUE, Integer.MAX_VALUE); + Limits initialLimits = new Limits(2, 0, Integer.MAX_VALUE, Integer.MAX_VALUE); // create new captured context CapturedContext capturedContext = new CapturedContext(); - // this will resolve only the first-level fields of the given object - List flds = new ArrayList<>(); - Field[] fields = Person.class.getDeclaredFields(); - for (Field field : fields) { - field.setAccessible(true); - flds.add( - CapturedContext.CapturedValue.of( - field.getName(), - field.getType().getName(), - field.get(p), - initialLimits.maxReferenceDepth, - initialLimits.maxCollectionSize, - initialLimits.maxLength, - initialLimits.maxFieldCount)); - } - capturedContext.addFields(flds.toArray(new CapturedContext.CapturedValue[0])); + CapturedContext.CapturedValue thisValue = + CapturedContext.CapturedValue.of( + "this", + Person.class.getName(), + p, + initialLimits.maxReferenceDepth, + initialLimits.maxCollectionSize, + initialLimits.maxLength, + initialLimits.maxFieldCount); + capturedContext.addArguments(new CapturedContext.CapturedValue[] {thisValue}); // '.name.value' is not present in the snapshot - it needs to be retrieved via reflection Value val = DSL.getMember(DSL.ref("name"), "value").evaluate(capturedContext); @@ -75,9 +68,10 @@ void extractAfterEl() throws IllegalAccessException { // after freezing the original value is removed and only the serialized json representation // remains - assertNull(capturedContext.getFields().get("name").getValue()); - assertEquals( - "{\"type\":\"com.datadog.debugger.el.ELIntegrationSanityTest$Name\",\"fields\":{\"value\":{\"type\":\"java.lang.String\",\"value\":\"name\"}}}", - capturedContext.getFields().get("name").getStrValue()); + Map thisFields = + getFields(capturedContext.getArguments().get("this")); + Map name = + (Map) thisFields.get("name").getValue(); + assertEquals(p.name.value, name.get("value").getValue()); } } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java index 9805504abe6..99cec359067 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java @@ -52,11 +52,16 @@ public class MoshiSnapshotTestHelper { public static String getValue(CapturedContext.CapturedValue capturedValue) { CapturedContext.CapturedValue valued = null; try { - valued = VALUE_ADAPTER.fromJson(capturedValue.getStrValue()); - if (valued.getNotCapturedReason() != null) { - Assertions.fail("NotCapturedReason: " + valued.getNotCapturedReason()); + Object obj; + if (capturedValue.getStrValue() != null) { + valued = VALUE_ADAPTER.fromJson(capturedValue.getStrValue()); + if (valued.getNotCapturedReason() != null) { + Assertions.fail("NotCapturedReason: " + valued.getNotCapturedReason()); + } + obj = valued.getValue(); + } else { + obj = capturedValue.getValue(); } - Object obj = valued.getValue(); if (obj != null && obj.getClass().isArray()) { if (obj.getClass().getComponentType().isPrimitive()) { return primitiveArrayToString(obj); @@ -189,11 +194,11 @@ public CapturedContext fromJson(JsonReader jsonReader) throws IOException { List argValues = new ArrayList<>(); while (jsonReader.hasNext()) { String argName = jsonReader.peekJson().nextName(); - if ("this".equals(argName)) { - jsonReader.nextName(); // consume "this" - fromJsonFields(jsonReader, capturedContext); - continue; - } + // if ("this".equals(argName)) { + // jsonReader.nextName(); // consume "this" + // fromJsonFields(jsonReader, capturedContext); + // continue; + // } argName = jsonReader.nextName(); CapturedContext.CapturedValue capturedValue = valueAdapter.fromJson(jsonReader); if (capturedValue != null) { @@ -234,7 +239,7 @@ private void fromJsonFields(JsonReader jsonReader, CapturedContext capturedConte } case FIELDS: { - capturedContext.addFields(fromJsonCapturedValues(jsonReader)); + // capturedContext.addFields(fromJsonCapturedValues(jsonReader)); break; } case NOT_CAPTURED_REASON: diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/StringTokenWriterTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/StringTokenWriterTest.java index b3bce5e78a4..ebba9d26ec0 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/StringTokenWriterTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/StringTokenWriterTest.java @@ -44,7 +44,7 @@ public void fields() throws Exception { @Test public void deepFields() throws Exception { assertEquals( - "{list=..., strVal=strval, intVal=24, nullField=null, mapVal=..., objArray=...}", + "{strVal=strval, intVal=24, nullField=null, mapVal=..., objArray=...}", serializeValue(new Person(), DEPTH_1)); } diff --git a/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot19.java b/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot19.java index 5909e5644f2..3143f27eb2d 100644 --- a/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot19.java +++ b/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot19.java @@ -10,6 +10,10 @@ public static int main(String arg) { if ("inherited".equals(arg)) { return new Inherited().f(); } + return new CapturedSnapshot19().process(arg); + } + + int process(String arg) { return 42; } diff --git a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/LogProbesIntegrationTest.java b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/LogProbesIntegrationTest.java index 4162c53c1c3..75d5ae5dd18 100644 --- a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/LogProbesIntegrationTest.java +++ b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/LogProbesIntegrationTest.java @@ -73,7 +73,6 @@ void testFullMethod() throws Exception { assertFullMethodCaptureArgs(snapshot.getCaptures().getEntry()); assertNull(snapshot.getCaptures().getEntry().getLocals()); assertNull(snapshot.getCaptures().getEntry().getCapturedThrowable()); - assertNull(snapshot.getCaptures().getEntry().getFields()); assertFullMethodCaptureArgs(snapshot.getCaptures().getReturn()); assertCaptureReturnValue( snapshot.getCaptures().getReturn(), @@ -82,7 +81,6 @@ void testFullMethod() throws Exception { assertNotNull(snapshot.getCaptures().getReturn().getLocals()); assertEquals(1, snapshot.getCaptures().getReturn().getLocals().size()); // @return assertNull(snapshot.getCaptures().getReturn().getCapturedThrowable()); - assertNull(snapshot.getCaptures().getReturn().getFields()); snapshotReceived.set(true); }); AtomicBoolean statusResult = registerCheckReceivedInstalledEmitting();