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 0bb90dc0305..7c01cd8ee56 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 @@ -206,7 +206,7 @@ public void addReturn(CapturedValue retValue) { if (locals == null) { locals = new HashMap<>(); } - locals.put("@return", retValue); // special local name for the return value + locals.put(ValueReferences.RETURN_REF, retValue); // special local name for the return value extensions.put(ValueReferences.RETURN_EXTENSION_NAME, retValue); } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java index 38642365cc4..6d361e5c1a9 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java @@ -11,6 +11,8 @@ import static com.datadog.debugger.instrumentation.ASMHelper.ldc; import static com.datadog.debugger.instrumentation.Types.*; import static datadog.trace.util.Strings.getClassName; +import static org.objectweb.asm.Type.DOUBLE_TYPE; +import static org.objectweb.asm.Type.LONG_TYPE; import com.datadog.debugger.el.InvalidValueException; import com.datadog.debugger.el.Visitor; @@ -46,6 +48,7 @@ import com.datadog.debugger.el.values.StringValue; import com.datadog.debugger.probe.MetricProbe; import com.datadog.debugger.probe.Where; +import datadog.trace.bootstrap.debugger.MethodLocation; import datadog.trace.bootstrap.debugger.el.ValueReferences; import java.lang.reflect.Field; import java.util.ArrayList; @@ -141,6 +144,7 @@ protected InsnList getBeforeReturnInsnList(AbstractInsnNode node) { int size = 1; int storeOpCode = 0; int loadOpCode = 0; + Type returnType = null; switch (node.getOpcode()) { case Opcodes.RET: case Opcodes.RETURN: @@ -149,29 +153,35 @@ protected InsnList getBeforeReturnInsnList(AbstractInsnNode node) { storeOpCode = Opcodes.LSTORE; loadOpCode = Opcodes.LLOAD; size = 2; + returnType = Type.LONG_TYPE; break; case Opcodes.DRETURN: storeOpCode = Opcodes.DSTORE; loadOpCode = Opcodes.DLOAD; size = 2; + returnType = Type.DOUBLE_TYPE; break; case Opcodes.IRETURN: storeOpCode = Opcodes.ISTORE; loadOpCode = Opcodes.ILOAD; + returnType = Type.INT_TYPE; break; case Opcodes.FRETURN: storeOpCode = Opcodes.FSTORE; loadOpCode = Opcodes.FLOAD; + returnType = Type.FLOAT_TYPE; break; case Opcodes.ARETURN: storeOpCode = Opcodes.ASTORE; loadOpCode = Opcodes.ALOAD; + returnType = OBJECT_TYPE; break; default: throw new UnsupportedOperationException("Unsupported opcode: " + node.getOpcode()); } - InsnList insnList = wrapTryCatch(callMetric(metricProbe)); int tmpIdx = newVar(size); + InsnList insnList = + wrapTryCatch(callMetric(metricProbe, new ReturnContext(tmpIdx, loadOpCode, returnType))); // store return value from the stack to local before wrapped call insnList.insert(new VarInsnNode(storeOpCode, tmpIdx)); // restore return value to the stack after wrapped call @@ -179,7 +189,7 @@ protected InsnList getBeforeReturnInsnList(AbstractInsnNode node) { return insnList; } - private InsnList callCount(MetricProbe metricProbe) { + private InsnList callCount(MetricProbe metricProbe, ReturnContext returnContext) { if (metricProbe.getValue() == null) { InsnList insnList = new InsnList(); // consider the metric as an increment counter one @@ -203,16 +213,20 @@ private InsnList callCount(MetricProbe metricProbe) { // stack [] return insnList; } - return internalCallMetric(metricProbe); + return internalCallMetric(metricProbe, returnContext); } - private InsnList internalCallMetric(MetricProbe metricProbe) { + private InsnList internalCallMetric(MetricProbe metricProbe, ReturnContext returnContext) { InsnList insnList = new InsnList(); InsnList nullBranch = new InsnList(); VisitorResult result; Type resultType; try { - result = metricProbe.getValue().getExpr().accept(new MetricValueVisitor(this, nullBranch)); + result = + metricProbe + .getValue() + .getExpr() + .accept(new MetricValueVisitor(this, nullBranch, returnContext)); } catch (InvalidValueException | UnsupportedOperationException ex) { reportError(ex.getMessage()); return EMPTY_INSN_LIST; @@ -271,16 +285,20 @@ private Type convertIfRequired(Type currentType, InsnList insnList) { } private InsnList callMetric(MetricProbe metricProbe) { + return callMetric(metricProbe, null); + } + + private InsnList callMetric(MetricProbe metricProbe, ReturnContext returnContext) { switch (metricProbe.getKind()) { case COUNT: - return callCount(metricProbe); + return callCount(metricProbe, returnContext); case GAUGE: case HISTOGRAM: case DISTRIBUTION: if (metricProbe.getValue() == null) { return EMPTY_INSN_LIST; } - return internalCallMetric(metricProbe); + return internalCallMetric(metricProbe, returnContext); default: reportError(String.format("Unknown metric kind: %s", metricProbe.getKind())); } @@ -332,10 +350,13 @@ public VisitorResult(ASMHelper.Type type, InsnList insnList) { private static class MetricValueVisitor implements Visitor { private final MetricInstrumentor instrumentor; private final InsnList nullBranch; + private final ReturnContext returnContext; - public MetricValueVisitor(MetricInstrumentor instrumentor, InsnList nullBranch) { + public MetricValueVisitor( + MetricInstrumentor instrumentor, InsnList nullBranch, ReturnContext returnContext) { this.instrumentor = instrumentor; this.nullBranch = nullBranch; + this.returnContext = returnContext; } @Override @@ -463,7 +484,11 @@ public VisitorResult visit(ValueRefExpression valueRefExpression) { insnList.add(new VarInsnNode(Opcodes.ALOAD, 0)); // stack [this] } else { - currentType = tryRetrieve(name, insnList); + if (name.startsWith(ValueReferences.SYNTHETIC_PREFIX)) { + currentType = tryRetrieveSynthetic(name, insnList); + } else { + currentType = tryRetrieve(name, insnList); + } if (currentType == null) { throw new InvalidValueException("Cannot resolve symbol " + name); } @@ -727,5 +752,58 @@ private ASMHelper.Type tryRetrieveField( } return returnType; } + + private ASMHelper.Type tryRetrieveSynthetic(String name, InsnList insnList) { + if (name.equals(ValueReferences.RETURN_REF)) { + if (instrumentor.metricProbe.getEvaluateAt() != MethodLocation.EXIT) { + return null; + } + if (returnContext == null) { + return null; + } + VarInsnNode varInsnNode = + new VarInsnNode(returnContext.opLoad, returnContext.returnLocalVarIdx); + insnList.add(varInsnNode); + // stack [return_value] + return new ASMHelper.Type(returnContext.type); + } + if (name.equals(ValueReferences.DURATION_REF)) { + if (instrumentor.metricProbe.getEvaluateAt() != MethodLocation.EXIT) { + return null; + } + // call System.nanoTime at the beginning of the method + int var = instrumentor.newVar(LONG_TYPE); + InsnList nanoTimeList = new InsnList(); + invokeStatic(nanoTimeList, Type.getType(System.class), "nanoTime", LONG_TYPE); + nanoTimeList.add(new VarInsnNode(Opcodes.LSTORE, var)); + instrumentor.methodNode.instructions.insert(instrumentor.methodEnterLabel, nanoTimeList); + // diff nanoTime before calling metric + invokeStatic(insnList, Type.getType(System.class), "nanoTime", LONG_TYPE); + // stack [long] + insnList.add(new VarInsnNode(Opcodes.LLOAD, var)); + // stack [long, long] + insnList.add(new InsnNode(Opcodes.LSUB)); + // stack [long] + insnList.add(new InsnNode(Opcodes.L2D)); + insnList.add(new LdcInsnNode(1_000_000D)); + // stack [long, double] + insnList.add(new InsnNode(Opcodes.DDIV)); + // stack [double] + return new ASMHelper.Type(DOUBLE_TYPE); + } + return null; + } + } + + private static class ReturnContext { + private final int returnLocalVarIdx; + private final int opLoad; + private final Type type; + + public ReturnContext(int returnLocalVarIdx, int opLoad, Type type) { + this.returnLocalVarIdx = returnLocalVarIdx; + this.opLoad = opLoad; + this.type = type; + } } } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/MetricProbesInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/MetricProbesInstrumentationTest.java index d898d7d904a..0fbb89f2a2e 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/MetricProbesInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/MetricProbesInstrumentationTest.java @@ -366,6 +366,63 @@ public void methodFieldRefValueDistributionDoubleMetric() throws IOException, UR Assertions.assertArrayEquals(new String[] {METRIC_PROBEID_TAG}, listener.lastTags); } + @Test + public void methodSyntheticReturnGaugeMetric() throws IOException, URISyntaxException { + final String CLASS_NAME = "CapturedSnapshot06"; + String METRIC_NAME = "syn_gauge"; + MetricProbe metricProbe = + createMetricBuilder(METRIC_ID, METRIC_NAME, GAUGE) + .where(CLASS_NAME, "f", "()") + .valueScript(new ValueScript(DSL.ref("@return"), "@return")) + .evaluateAt(MethodLocation.EXIT) + .build(); + MetricForwarderListener listener = installMetricProbes(metricProbe); + Class testClass = compileAndLoadClass(CLASS_NAME); + int result = Reflect.on(testClass).call("main", "f").get(); + Assertions.assertEquals(42, result); + Assertions.assertTrue(listener.gauges.containsKey(METRIC_NAME)); + Assertions.assertEquals(42, listener.gauges.get(METRIC_NAME).longValue()); + Assertions.assertArrayEquals(new String[] {METRIC_PROBEID_TAG}, listener.lastTags); + } + + @Test + public void methodSyntheticReturnInvalidType() throws IOException, URISyntaxException { + final String CLASS_NAME = "CapturedSnapshot06"; + final String INHERITED_CLASS_NAME = CLASS_NAME + "$Inherited"; + String METRIC_NAME = "syn_gauge"; + MetricProbe metricProbe = + createMetricBuilder(METRIC_ID, METRIC_NAME, GAUGE) + .where(INHERITED_CLASS_NAME, "", "()") + .valueScript(new ValueScript(DSL.ref("@return"), "@return")) + .evaluateAt(MethodLocation.EXIT) + .build(); + MetricForwarderListener listener = installMetricProbes(metricProbe); + Class testClass = compileAndLoadClass(CLASS_NAME); + int result = Reflect.on(testClass).call("main", "").get(); + Assertions.assertEquals(42, result); + Assertions.assertFalse(listener.gauges.containsKey(METRIC_NAME)); + verify(probeStatusSink).addError(eq(METRIC_ID), eq("Cannot resolve symbol @return")); + } + + @Test + public void methodSyntheticDurationGaugeMetric() throws IOException, URISyntaxException { + final String CLASS_NAME = "CapturedSnapshot06"; + String METRIC_NAME = "syn_gauge"; + MetricProbe metricProbe = + createMetricBuilder(METRIC_ID, METRIC_NAME, GAUGE) + .where(CLASS_NAME, "f", "()") + .valueScript(new ValueScript(DSL.ref("@duration"), "@duration")) + .evaluateAt(MethodLocation.EXIT) + .build(); + MetricForwarderListener listener = installMetricProbes(metricProbe); + Class testClass = compileAndLoadClass(CLASS_NAME); + int result = Reflect.on(testClass).call("main", "f").get(); + Assertions.assertEquals(42, result); + Assertions.assertTrue(listener.doubleGauges.containsKey(METRIC_NAME)); + Assertions.assertTrue(listener.doubleGauges.get(METRIC_NAME).doubleValue() > 0); + Assertions.assertArrayEquals(new String[] {METRIC_PROBEID_TAG}, listener.lastTags); + } + @Test public void lineArgumentRefValueCountMetric() throws IOException, URISyntaxException { final String CLASS_NAME = "CapturedSnapshot03";