Skip to content

Commit 507746d

Browse files
jpbempeljpbempel
committed
Add span smoke tests
Also refactor the status of the instrumentation to be returned by instrument method. It avoids to send a probe status INSTALLED first then received later the ERROR one. Report a probe status error when trying to instrument a single line span probe.
1 parent dc125de commit 507746d

File tree

17 files changed

+237
-100
lines changed

17 files changed

+237
-100
lines changed

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java

+3
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,9 @@ private void installNewDefinitions() {
182182

183183
private void recordInstrumentationProgress(
184184
ProbeDefinition definition, InstrumentationResult instrumentationResult) {
185+
if (instrumentationResult.isError()) {
186+
return;
187+
}
185188
instrumentationResults.put(definition.getId(), instrumentationResult);
186189
if (instrumentationResult.isInstalled()) {
187190
sink.addInstalled(definition.getProbeId());

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTracer.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.datadog.debugger.agent;
22

3+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.NOOP_TRACER;
4+
35
import datadog.trace.bootstrap.debugger.DebuggerContext;
46
import datadog.trace.bootstrap.debugger.DebuggerSpan;
57
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
@@ -8,12 +10,12 @@
810
import datadog.trace.bootstrap.instrumentation.api.ScopeSource;
911

1012
public class DebuggerTracer implements DebuggerContext.Tracer {
11-
private static final String OPERATION_NAME = "dd.dynamic.span";
13+
public static final String OPERATION_NAME = "dd.dynamic.span";
1214

1315
@Override
1416
public DebuggerSpan createSpan(String resourceName, String[] tags) {
1517
AgentTracer.TracerAPI tracerAPI = AgentTracer.get();
16-
if (tracerAPI == null) {
18+
if (tracerAPI == null || tracerAPI == NOOP_TRACER) {
1719
return DebuggerSpan.NOOP_SPAN;
1820
}
1921
AgentSpan dynamicSpan =

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ private InstrumentationResult applyInstrumentation(
486486
capturedContextProbes.add(definition);
487487
} else {
488488
List<DiagnosticMessage> probeDiagnostics = diagnostics.get(definition.getProbeId());
489-
definition.instrument(classLoader, classNode, methodNode, probeDiagnostics);
489+
status = definition.instrument(classLoader, classNode, methodNode, probeDiagnostics);
490490
}
491491
}
492492
if (capturedContextProbes.size() > 0) {
@@ -495,8 +495,9 @@ private InstrumentationResult applyInstrumentation(
495495
ProbeDefinition referenceDefinition = selectReferenceDefinition(capturedContextProbes);
496496
List<DiagnosticMessage> probeDiagnostics =
497497
diagnostics.get(referenceDefinition.getProbeId());
498-
referenceDefinition.instrument(
499-
classLoader, classNode, methodNode, probeDiagnostics, probesIds);
498+
status =
499+
referenceDefinition.instrument(
500+
classLoader, classNode, methodNode, probeDiagnostics, probesIds);
500501
}
501502
} catch (Throwable t) {
502503
log.warn("Exception during instrumentation: ", t);

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java

+16-11
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,21 @@ public CapturedContextInstrumentor(
7878
}
7979

8080
@Override
81-
public void instrument() {
81+
public InstrumentationResult.Status instrument() {
8282
if (isLineProbe) {
8383
fillLineMap();
84-
addLineCaptures(lineMap);
85-
} else {
86-
instrumentMethodEnter();
87-
instrumentTryCatchHandlers();
88-
processInstructions();
89-
addFinallyHandler(returnHandlerLabel);
84+
if (!addLineCaptures(lineMap)) {
85+
return InstrumentationResult.Status.ERROR;
86+
}
87+
installFinallyBlocks();
88+
return InstrumentationResult.Status.INSTALLED;
9089
}
90+
instrumentMethodEnter();
91+
instrumentTryCatchHandlers();
92+
processInstructions();
93+
addFinallyHandler(returnHandlerLabel);
9194
installFinallyBlocks();
95+
return InstrumentationResult.Status.INSTALLED;
9296
}
9397

9498
private void installFinallyBlocks() {
@@ -99,15 +103,15 @@ private void installFinallyBlocks() {
99103
}
100104
}
101105

102-
private void addLineCaptures(LineMap lineMap) {
106+
private boolean addLineCaptures(LineMap lineMap) {
103107
Where.SourceLine[] targetLines = definition.getWhere().getSourceLines();
104108
if (targetLines == null) {
105-
// no line capture to perform
106-
return;
109+
reportError("Missing line(s) in probe definition.");
110+
return false;
107111
}
108112
if (lineMap.isEmpty()) {
109113
reportError("Missing line debug information.");
110-
return;
114+
return false;
111115
}
112116
for (Where.SourceLine sourceLine : targetLines) {
113117
int from = sourceLine.getFrom();
@@ -167,6 +171,7 @@ private void addLineCaptures(LineMap lineMap) {
167171
methodNode.instructions.insert(afterLabel, insnList);
168172
}
169173
}
174+
return true;
170175
}
171176

172177
@Override

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/InstrumentationResult.java

+4
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ public InstrumentationResult(
5656
this.methodName = methodName;
5757
}
5858

59+
public boolean isError() {
60+
return status == Status.ERROR;
61+
}
62+
5963
public boolean isBlocked() {
6064
return status == Status.BLOCKED;
6165
}

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/Instrumentor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public Instrumentor(
6969
localVarsBySlot = extractLocalVariables(argTypes);
7070
}
7171

72-
public abstract void instrument();
72+
public abstract InstrumentationResult.Status instrument();
7373

7474
private LocalVariableNode[] extractLocalVariables(Type[] argTypes) {
7575
if (methodNode.localVariables == null || methodNode.localVariables.isEmpty()) {

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java

+25-24
Original file line numberDiff line numberDiff line change
@@ -89,29 +89,29 @@ public MetricInstrumentor(
8989
}
9090

9191
@Override
92-
public void instrument() {
92+
public InstrumentationResult.Status instrument() {
9393
if (isLineProbe) {
9494
fillLineMap();
95-
addLineMetric(lineMap);
96-
} else {
97-
switch (definition.getEvaluateAt()) {
98-
case ENTRY:
99-
case DEFAULT:
100-
{
101-
InsnList insnList = wrapTryCatch(callMetric(metricProbe));
102-
methodNode.instructions.insert(methodEnterLabel, insnList);
103-
break;
104-
}
105-
case EXIT:
106-
{
107-
processInstructions();
108-
break;
109-
}
110-
default:
111-
throw new IllegalArgumentException(
112-
"Invalid evaluateAt attribute: " + definition.getEvaluateAt());
113-
}
95+
return addLineMetric(lineMap);
96+
}
97+
switch (definition.getEvaluateAt()) {
98+
case ENTRY:
99+
case DEFAULT:
100+
{
101+
InsnList insnList = wrapTryCatch(callMetric(metricProbe));
102+
methodNode.instructions.insert(methodEnterLabel, insnList);
103+
break;
104+
}
105+
case EXIT:
106+
{
107+
processInstructions();
108+
break;
109+
}
110+
default:
111+
throw new IllegalArgumentException(
112+
"Invalid evaluateAt attribute: " + definition.getEvaluateAt());
114113
}
114+
return InstrumentationResult.Status.INSTALLED;
115115
}
116116

117117
private InsnList wrapTryCatch(InsnList insnList) {
@@ -287,15 +287,15 @@ private InsnList callMetric(MetricProbe metricProbe) {
287287
return null;
288288
}
289289

290-
private void addLineMetric(LineMap lineMap) {
290+
private InstrumentationResult.Status addLineMetric(LineMap lineMap) {
291291
Where.SourceLine[] targetLines = metricProbe.getWhere().getSourceLines();
292292
if (targetLines == null) {
293-
// no line capture to perform
294-
return;
293+
reportError("Missing line(s) in probe definition.");
294+
return InstrumentationResult.Status.ERROR;
295295
}
296296
if (lineMap.isEmpty()) {
297297
reportError("Missing line debug information.");
298-
return;
298+
return InstrumentationResult.Status.ERROR;
299299
}
300300
for (Where.SourceLine sourceLine : targetLines) {
301301
int from = sourceLine.getFrom();
@@ -316,6 +316,7 @@ private void addLineMetric(LineMap lineMap) {
316316
methodNode.instructions.insert(afterLabel, insnList);
317317
}
318318
}
319+
return InstrumentationResult.Status.INSTALLED;
319320
}
320321

321322
private static class VisitorResult {

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/SpanInstrumentor.java

+25-21
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,24 @@ public SpanInstrumentor(
3636
}
3737

3838
@Override
39-
public void instrument() {
39+
public InstrumentationResult.Status instrument() {
4040
if (isLineProbe) {
4141
fillLineMap();
42-
addRangeSpan(lineMap);
43-
} else {
44-
spanVar = newVar(DEBUGGER_SPAN_TYPE);
45-
processInstructions();
46-
LabelNode initSpanLabel = new LabelNode();
47-
InsnList insnList = createSpan(initSpanLabel);
48-
LabelNode endLabel = new LabelNode();
49-
methodNode.instructions.insert(methodNode.instructions.getLast(), endLabel);
50-
51-
LabelNode handlerLabel = new LabelNode();
52-
InsnList handler = createCatchHandler(handlerLabel);
53-
methodNode.instructions.add(handler);
54-
methodNode.tryCatchBlocks.add(
55-
new TryCatchBlockNode(initSpanLabel, endLabel, handlerLabel, null));
56-
methodNode.instructions.insert(methodEnterLabel, insnList);
42+
return addRangeSpan(lineMap);
5743
}
44+
spanVar = newVar(DEBUGGER_SPAN_TYPE);
45+
processInstructions();
46+
LabelNode initSpanLabel = new LabelNode();
47+
InsnList insnList = createSpan(initSpanLabel);
48+
LabelNode endLabel = new LabelNode();
49+
methodNode.instructions.insert(methodNode.instructions.getLast(), endLabel);
50+
LabelNode handlerLabel = new LabelNode();
51+
InsnList handler = createCatchHandler(handlerLabel);
52+
methodNode.instructions.add(handler);
53+
methodNode.tryCatchBlocks.add(
54+
new TryCatchBlockNode(initSpanLabel, endLabel, handlerLabel, null));
55+
methodNode.instructions.insert(methodEnterLabel, insnList);
56+
return InstrumentationResult.Status.INSTALLED;
5857
}
5958

6059
private InsnList createCatchHandler(LabelNode handlerLabel) {
@@ -94,25 +93,29 @@ private InsnList createSpan(LabelNode initSpanLabel) {
9493
return insnList;
9594
}
9695

97-
private void addRangeSpan(LineMap lineMap) {
96+
private InstrumentationResult.Status addRangeSpan(LineMap lineMap) {
9897
Where.SourceLine[] targetLines = definition.getWhere().getSourceLines();
9998
if (targetLines == null || targetLines.length == 0) {
100-
// no line capture to perform
101-
return;
99+
reportError("Missing line(s) in probe definition.");
100+
return InstrumentationResult.Status.ERROR;
102101
}
103102
if (lineMap.isEmpty()) {
104103
reportError("Missing line debug information.");
105-
return;
104+
return InstrumentationResult.Status.ERROR;
106105
}
107106
for (Where.SourceLine sourceLine : targetLines) {
108107
int from = sourceLine.getFrom();
109108
int till = sourceLine.getTill();
109+
if (from == till) {
110+
reportError("Single line span is not supported, you need to provide a range.");
111+
return InstrumentationResult.Status.ERROR;
112+
}
110113
LabelNode beforeLabel = lineMap.getLineLabel(from);
111114
LabelNode afterLabel = lineMap.getLineLabel(till);
112115
if (beforeLabel == null || afterLabel == null) {
113116
reportError(
114117
"No line info for " + (sourceLine.isSingleLine() ? "line " : "range ") + sourceLine);
115-
return;
118+
return InstrumentationResult.Status.ERROR;
116119
}
117120
spanVar = newVar(DEBUGGER_SPAN_TYPE);
118121
LabelNode initSpanLabel = new LabelNode();
@@ -127,6 +130,7 @@ private void addRangeSpan(LineMap lineMap) {
127130
debuggerSpanFinish(finishSpanInsnList);
128131
methodNode.instructions.insert(afterLabel, finishSpanInsnList);
129132
}
133+
return InstrumentationResult.Status.INSTALLED;
130134
}
131135

132136
@Override

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.datadog.debugger.el.ValueScript;
1111
import com.datadog.debugger.instrumentation.CapturedContextInstrumentor;
1212
import com.datadog.debugger.instrumentation.DiagnosticMessage;
13+
import com.datadog.debugger.instrumentation.InstrumentationResult;
1314
import com.datadog.debugger.sink.Sink;
1415
import com.datadog.debugger.sink.Snapshot;
1516
import com.squareup.moshi.Json;
@@ -343,13 +344,13 @@ public Sampling getSampling() {
343344
}
344345

345346
@Override
346-
public void instrument(
347+
public InstrumentationResult.Status instrument(
347348
ClassLoader classLoader,
348349
ClassNode classNode,
349350
MethodNode methodNode,
350351
List<DiagnosticMessage> diagnostics,
351352
List<String> probeIds) {
352-
new CapturedContextInstrumentor(
353+
return new CapturedContextInstrumentor(
353354
this,
354355
classLoader,
355356
classNode,

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/MetricProbe.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.datadog.debugger.agent.Generated;
44
import com.datadog.debugger.el.ValueScript;
55
import com.datadog.debugger.instrumentation.DiagnosticMessage;
6+
import com.datadog.debugger.instrumentation.InstrumentationResult;
67
import com.datadog.debugger.instrumentation.MetricInstrumentor;
78
import datadog.trace.bootstrap.debugger.MethodLocation;
89
import datadog.trace.bootstrap.debugger.ProbeId;
@@ -135,13 +136,13 @@ public ValueScript getValue() {
135136
}
136137

137138
@Override
138-
public void instrument(
139+
public InstrumentationResult.Status instrument(
139140
ClassLoader classLoader,
140141
ClassNode classNode,
141142
MethodNode methodNode,
142143
List<DiagnosticMessage> diagnostics,
143144
List<String> probeIds) {
144-
new MetricInstrumentor(this, classLoader, classNode, methodNode, diagnostics, probeIds)
145+
return new MetricInstrumentor(this, classLoader, classNode, methodNode, diagnostics, probeIds)
145146
.instrument();
146147
}
147148

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,15 @@ private static void initTagMap(Map<String, String> tagMap, Tag[] tags) {
122122
}
123123
}
124124

125-
public void instrument(
125+
public InstrumentationResult.Status instrument(
126126
ClassLoader classLoader,
127127
ClassNode classNode,
128128
MethodNode methodNode,
129129
List<DiagnosticMessage> diagnostics) {
130-
instrument(classLoader, classNode, methodNode, diagnostics, singletonList(getId()));
130+
return instrument(classLoader, classNode, methodNode, diagnostics, singletonList(getId()));
131131
}
132132

133-
public abstract void instrument(
133+
public abstract InstrumentationResult.Status instrument(
134134
ClassLoader classLoader,
135135
ClassNode classNode,
136136
MethodNode methodNode,

Diff for: ‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.datadog.debugger.el.ProbeCondition;
88
import com.datadog.debugger.instrumentation.CapturedContextInstrumentor;
99
import com.datadog.debugger.instrumentation.DiagnosticMessage;
10+
import com.datadog.debugger.instrumentation.InstrumentationResult;
1011
import com.datadog.debugger.sink.Snapshot;
1112
import datadog.trace.api.Pair;
1213
import datadog.trace.bootstrap.debugger.CapturedContext;
@@ -132,13 +133,13 @@ public SpanDecorationProbe(
132133
}
133134

134135
@Override
135-
public void instrument(
136+
public InstrumentationResult.Status instrument(
136137
ClassLoader classLoader,
137138
ClassNode classNode,
138139
MethodNode methodNode,
139140
List<DiagnosticMessage> diagnostics,
140141
List<String> probeIds) {
141-
new CapturedContextInstrumentor(
142+
return new CapturedContextInstrumentor(
142143
this, classLoader, classNode, methodNode, diagnostics, probeIds, false, Limits.DEFAULT)
143144
.instrument();
144145
}

0 commit comments

Comments
 (0)