Skip to content

Commit

Permalink
Merge pull request #6178 from DataDog/jpbempel/synthetic-vars-for-all…
Browse files Browse the repository at this point in the history 
…-probes

Add support for synthetic vars for metric probes
  • Loading branch information
@jpbempel
jpbempel committed Nov 9, 2023
2 parents 94ed1d0 + 6f812d5 commit ffaa3d0
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 10 deletions.
2 changes: 1 addition & 1 deletion ...er/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
96 changes: 87 additions & 9 deletions ...agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand All @@ -149,37 +153,43 @@ 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
insnList.add(new VarInsnNode(loadOpCode, tmpIdx));
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
Expand All @@ -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;
Expand Down Expand Up @@ -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()));
}
Expand Down Expand Up @@ -332,10 +350,13 @@ public VisitorResult(ASMHelper.Type type, InsnList insnList) {
private static class MetricValueVisitor implements Visitor<VisitorResult> {
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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
}
}
57 changes: 57 additions & 0 deletions ...nt-debugger/src/test/java/com/datadog/debugger/agent/MetricProbesInstrumentationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "<init>", "()")
.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";
Expand Down

0 comments on commit ffaa3d0

Please sign in to comment.