Skip to content

Commit fd61031

Browse files
jpbempeljpbempel
committed
Fix CodeOrigin for @trace annotation
add a param for captureCodeOrigin method to indicate if we need to instrument for the CodeOriginProbe. the instrumentation for the @trace annotation insert a call to start and activate the span. so we just need to call captureCodeOrigin after the activation so we benefit from tracer instrumentation without generating a call to CodeOriginProbe. That way are in sync with the tracer instrumentation. add smoke test
1 parent e037162 commit fd61031

File tree

8 files changed

+110
-27
lines changed

8 files changed

+110
-27
lines changed

‎dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public interface ExceptionDebugger {
100100
public interface CodeOriginRecorder {
101101
String captureCodeOrigin(boolean entry);
102102

103-
String captureCodeOrigin(Method method, boolean entry);
103+
String captureCodeOrigin(Method method, boolean entry, boolean instrument);
104104
}
105105

106106
private static volatile ProbeResolver probeResolver;
@@ -405,10 +405,14 @@ public static String captureCodeOrigin(boolean entry) {
405405
}
406406

407407
public static String captureCodeOrigin(Method method, boolean entry) {
408+
return captureCodeOrigin(method, entry, true);
409+
}
410+
411+
public static String captureCodeOrigin(Method method, boolean entry, boolean instrument) {
408412
try {
409413
CodeOriginRecorder recorder = codeOriginRecorder;
410414
if (recorder != null) {
411-
return recorder.captureCodeOrigin(method, entry);
415+
return recorder.captureCodeOrigin(method, entry, instrument);
412416
}
413417
} catch (Exception ex) {
414418
LOGGER.debug("Error in captureCodeOrigin: ", ex);

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/DefaultCodeOriginRecorder.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,20 +71,25 @@ public String captureCodeOrigin(boolean entry) {
7171
element.getMethodName(),
7272
null,
7373
String.valueOf(element.getLineNumber()));
74-
probe = createProbe(fingerprint, entry, where);
74+
probe = createProbe(fingerprint, entry, where, true);
7575

7676
LOG.debug("Creating probe for location {}", where);
7777
}
7878
return probe.getId();
7979
}
8080

8181
@Override
82-
public String captureCodeOrigin(Method method, boolean entry) {
82+
public String captureCodeOrigin(Method method, boolean entry, boolean instrument) {
8383
String fingerprint = method.toString();
8484
CodeOriginProbe probe = probesByFingerprint.get(fingerprint);
8585
if (probe == null) {
86-
probe = createProbe(fingerprint, entry, Where.of(method));
86+
probe = createProbe(fingerprint, entry, Where.of(method), instrument);
8787
LOG.debug("Creating probe for method {}", fingerprint);
88+
} else if (!instrument) {
89+
// direct call to fill code origin info without using probe instrumentation
90+
// buildLocation should be called before in order to gather location info
91+
probe.commit(
92+
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
8893
}
8994
return probe.getId();
9095
}
@@ -105,11 +110,12 @@ public void registerLogProbe(CodeOriginProbe probe) {
105110
.build());
106111
}
107112

108-
private CodeOriginProbe createProbe(String fingerPrint, boolean entry, Where where) {
113+
private CodeOriginProbe createProbe(
114+
String fingerPrint, boolean entry, Where where, boolean instrument) {
109115
CodeOriginProbe probe;
110116
AgentSpan span = AgentTracer.activeSpan();
111117

112-
probe = new CodeOriginProbe(ProbeId.newId(), entry, where);
118+
probe = new CodeOriginProbe(ProbeId.newId(), entry, where, instrument);
113119
addFingerprint(fingerPrint, probe);
114120
CodeOriginProbe installed = probes.putIfAbsent(probe.getId(), probe);
115121

‎dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,23 @@
2727
public class CodeOriginProbe extends ProbeDefinition {
2828
private static final Logger LOGGER = LoggerFactory.getLogger(CodeOriginProbe.class);
2929

30+
private final boolean instrument;
3031
private final boolean entrySpanProbe;
31-
3232
private String signature;
3333

34-
public CodeOriginProbe(ProbeId probeId, boolean entry, Where where) {
34+
public CodeOriginProbe(ProbeId probeId, boolean entry, Where where, boolean instrument) {
3535
super(LANGUAGE, probeId, (Tag[]) null, where, MethodLocation.ENTRY);
36+
this.instrument = instrument;
3637
this.entrySpanProbe = entry;
3738
}
3839

3940
@Override
4041
public Status instrument(
4142
MethodInfo methodInfo, List<DiagnosticMessage> diagnostics, List<ProbeId> probeIds) {
42-
return new CodeOriginInstrumentor(this, methodInfo, diagnostics, probeIds).instrument();
43+
if (instrument) {
44+
return new CodeOriginInstrumentor(this, methodInfo, diagnostics, probeIds).instrument();
45+
}
46+
return Status.INSTALLED;
4347
}
4448

4549
@Override
@@ -55,6 +59,10 @@ public void commit(
5559
List<AgentSpan> agentSpans =
5660
entrySpanProbe ? asList(span, span.getLocalRootSpan()) : singletonList(span);
5761

62+
if (location == null) {
63+
LOGGER.debug("Code origin probe {} has no location", id);
64+
return;
65+
}
5866
for (AgentSpan s : agentSpans) {
5967
if (s.getTag(DD_CODE_ORIGIN_TYPE) == null) {
6068
s.setTag(DD_CODE_ORIGIN_TYPE, entrySpanProbe ? "entry" : "exit");

‎dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/origin/CodeOriginTest.java‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ public void withDebug1() throws Exception {
105105
final String className = "com.datadog.debugger.CodeOrigin02";
106106
installProbes();
107107
final Class<?> testClass = compileAndLoadClass(className);
108-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("entry"), true);
109-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("exit"), false);
108+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("entry"), true, true);
109+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("exit"), false, true);
110110
checkResults(testClass, "fullTrace", 0);
111111
checkResults(testClass, "debug_1", 2);
112112
}
@@ -117,8 +117,8 @@ public void withLogProbe() throws Exception {
117117
installProbes(
118118
createProbeBuilder(PROBE_ID, CLASS_NAME, "entry", "()").captureSnapshot(true).build());
119119
final Class<?> testClass = compileAndLoadClass(CLASS_NAME);
120-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("entry"), true);
121-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("exit"), false);
120+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("entry"), true, true);
121+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("exit"), false, true);
122122
checkResults(testClass, "debug_1", 3);
123123
}
124124

@@ -127,10 +127,13 @@ public void doubleEntry() throws IOException, URISyntaxException {
127127
final String className = "com.datadog.debugger.CodeOrigin05";
128128

129129
installProbes(
130-
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(className, "entry", "()", "53")),
131-
new CodeOriginProbe(CODE_ORIGIN_ID2, false, Where.of(className, "exit", "()", "62")),
130+
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(className, "entry", "()", "53"), true),
131+
new CodeOriginProbe(CODE_ORIGIN_ID2, false, Where.of(className, "exit", "()", "62"), true),
132132
new CodeOriginProbe(
133-
CODE_ORIGIN_DOUBLE_ENTRY_ID, true, Where.of(className, "doubleEntry", "()", "66")));
133+
CODE_ORIGIN_DOUBLE_ENTRY_ID,
134+
true,
135+
Where.of(className, "doubleEntry", "()", "66"),
136+
true));
134137
final Class<?> testClass = compileAndLoadClass(className);
135138
checkResults(testClass, "fullTrace", 0);
136139
List<? extends MutableSpan> trace = traceInterceptor.getTrace();
@@ -143,7 +146,7 @@ public void doubleEntry() throws IOException, URISyntaxException {
143146
public void stackDepth() throws IOException, URISyntaxException {
144147
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
145148
installProbes(
146-
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(CLASS_NAME, "exit", "()", "39")));
149+
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(CLASS_NAME, "exit", "()", "39"), true));
147150

148151
Class<?> testClass = compileAndLoadClass("com.datadog.debugger.CodeOrigin04");
149152
countFrames(testClass, 10);
@@ -187,7 +190,8 @@ public void testCaptureCodeOriginWithExplicitInfo()
187190
installProbes();
188191
CodeOriginProbe probe =
189192
codeOriginRecorder.getProbe(
190-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true));
193+
codeOriginRecorder.captureCodeOrigin(
194+
testClass.getMethod("main", int.class), true, true));
191195
assertNotNull(probe, "The probe should have been created.");
192196
assertTrue(probe.entrySpanProbe(), "Should be an entry probe.");
193197
}
@@ -199,18 +203,18 @@ public void testDuplicateInstrumentations()
199203
final Class<?> testClass = compileAndLoadClass(CLASS_NAME);
200204
installProbes();
201205
String probe1 =
202-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true);
206+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true, true);
203207
String probe2 =
204-
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true);
208+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true, true);
205209
assertEquals(probe1, probe2);
206210
}
207211

208212
@NotNull
209213
private CodeOriginProbe[] codeOriginProbes(String type) {
210214
CodeOriginProbe entry =
211-
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(type, "entry", "()", "53"));
215+
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(type, "entry", "()", "53"), true);
212216
CodeOriginProbe exit =
213-
new CodeOriginProbe(CODE_ORIGIN_ID2, false, Where.of(type, "exit", "()", "60"));
217+
new CodeOriginProbe(CODE_ORIGIN_ID2, false, Where.of(type, "exit", "()", "60"), true);
214218
return new CodeOriginProbe[] {entry, exit};
215219
}
216220

‎dd-java-agent/instrumentation/micronaut/http-server-netty-4.0/src/test/groovy/MicronautTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class MicronautTest extends HttpServerTest<Object> {
3535
}
3636

3737
@Override
38-
String captureCodeOrigin(Method method, boolean entry) {
38+
String captureCodeOrigin(Method method, boolean entry, boolean instrument) {
3939
invoked = true
4040
return "done"
4141
}

‎dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAdvice.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
44
import static datadog.trace.instrumentation.trace_annotation.TraceDecorator.DECORATE;
55

6+
import datadog.trace.bootstrap.debugger.DebuggerContext;
67
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
78
import java.lang.invoke.MethodType;
89
import java.lang.reflect.Method;
@@ -13,7 +14,9 @@ public class TraceAdvice {
1314

1415
@Advice.OnMethodEnter(suppress = Throwable.class)
1516
public static AgentScope onEnter(@Advice.Origin final Method method) {
16-
return activateSpan(DECORATE.startMethodSpan(method));
17+
AgentScope agentScope = activateSpan(DECORATE.startMethodSpan(method));
18+
DebuggerContext.captureCodeOrigin(method, true, false);
19+
return agentScope;
1720
}
1821

1922
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

‎dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceDecorator.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import datadog.trace.api.InstrumenterConfig;
66
import datadog.trace.api.Trace;
7-
import datadog.trace.bootstrap.debugger.spanorigin.CodeOriginInfo;
87
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
98
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
109
import datadog.trace.bootstrap.instrumentation.decorator.AsyncResultDecorator;
@@ -91,7 +90,6 @@ public AgentSpan startMethodSpan(Method method) {
9190
afterStart(span);
9291
span.setResourceName(resourceName);
9392

94-
CodeOriginInfo.entry(method);
9593
if (measured || InstrumenterConfig.get().isMethodMeasured(method)) {
9694
span.setMeasured(true);
9795
}

‎dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/CodeOriginIntegrationTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package datadog.smoketest;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
5+
import datadog.trace.api.DDTags;
6+
import datadog.trace.test.agent.decoder.DecodedSpan;
7+
import java.nio.file.Path;
8+
import java.util.List;
9+
import java.util.concurrent.atomic.AtomicBoolean;
10+
import org.junit.jupiter.api.DisplayName;
11+
import org.junit.jupiter.api.Test;
12+
13+
public class CodeOriginIntegrationTest extends ServerAppDebuggerIntegrationTest {
14+
15+
private static final String DD_CODE_ORIGIN_FRAMES_0_FILE =
16+
String.format(DDTags.DD_CODE_ORIGIN_FRAME, 0, "file");
17+
private static final String DD_CODE_ORIGIN_FRAMES_0_METHOD =
18+
String.format(DDTags.DD_CODE_ORIGIN_FRAME, 0, "method");
19+
private static final String DD_CODE_ORIGIN_FRAMES_0_SIGNATURE =
20+
String.format(DDTags.DD_CODE_ORIGIN_FRAME, 0, "signature");
21+
private static final String DD_CODE_ORIGIN_FRAMES_0_LINE =
22+
String.format(DDTags.DD_CODE_ORIGIN_FRAME, 0, "line");
23+
24+
@Override
25+
protected ProcessBuilder createProcessBuilder(Path logFilePath, String... params) {
26+
List<String> commandParams = getDebuggerCommandParams();
27+
commandParams.add("-Ddd.trace.enabled=true"); // explicitly enable tracer
28+
commandParams.add("-Ddd.code.origin.for.spans.enabled=true");
29+
return ProcessBuilderHelper.createProcessBuilder(
30+
commandParams, logFilePath, getAppClass(), params);
31+
}
32+
33+
@Test
34+
@DisplayName("testCodeOriginTraceAnnotation")
35+
void testCodeOriginTraceAnnotation() throws Exception {
36+
execute(appUrl, TRACED_METHOD_NAME);
37+
waitForInstrumentation(appUrl);
38+
execute(appUrl, TRACED_METHOD_NAME);
39+
AtomicBoolean codeOrigin = new AtomicBoolean();
40+
registerTraceListener(
41+
decodedTrace -> {
42+
for (DecodedSpan span : decodedTrace.getSpans()) {
43+
if (isTracedFullMethodSpan(span)) {
44+
if (span.getMeta().containsKey(DDTags.DD_CODE_ORIGIN_TYPE)) {
45+
assertEquals("entry", span.getMeta().get(DDTags.DD_CODE_ORIGIN_TYPE));
46+
assertEquals(
47+
"ServerDebuggerTestApplication.java",
48+
span.getMeta().get(DD_CODE_ORIGIN_FRAMES_0_FILE));
49+
assertEquals("runTracedMethod", span.getMeta().get(DD_CODE_ORIGIN_FRAMES_0_METHOD));
50+
assertEquals(
51+
"(java.lang.String)", span.getMeta().get(DD_CODE_ORIGIN_FRAMES_0_SIGNATURE));
52+
assertEquals("133", span.getMeta().get(DD_CODE_ORIGIN_FRAMES_0_LINE));
53+
codeOrigin.set(true);
54+
}
55+
}
56+
}
57+
});
58+
processRequests(codeOrigin::get);
59+
}
60+
}

0 commit comments

Comments
 (0)