Skip to content

Commit 65b9b8a

Browse files
jpbempeljpbempel
authored
Fix CodeOrigin for interface endpoints (#11017)
Fix CodeOrigin for interface endpoints If endpoints are declared in interface with annotations, CodeOrigin instrumentation does not trigger and no code origin information are added to spans. We need to match also this case to trigger creation of code origin probe for handler in the concrete class implementing the interface Added smoke tests to cover those cases for Spring apps add feature flag for interface support interface support add overhead at startup so need to be manually enabled Co-authored-by: jean-philippe.bempel <jean-philippe.bempel@datadoghq.com>
1 parent 8c298a2 commit 65b9b8a

File tree

9 files changed

+262
-100
lines changed

9 files changed

+262
-100
lines changed

‎dd-java-agent/instrumentation/datadog/dynamic-instrumentation/span-origin/src/main/java/datadog/trace/instrumentation/codeorigin/CodeOriginInstrumentation.java‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
package datadog.trace.instrumentation.codeorigin;
22

3+
import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
4+
import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
5+
import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
6+
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
7+
38
import datadog.trace.agent.tooling.Instrumenter;
49
import datadog.trace.agent.tooling.InstrumenterModule.Tracing;
510
import datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers;
@@ -11,6 +16,7 @@
1116
import net.bytebuddy.description.NamedElement;
1217
import net.bytebuddy.description.type.TypeDescription;
1318
import net.bytebuddy.matcher.ElementMatcher;
19+
import net.bytebuddy.matcher.ElementMatchers;
1420

1521
public abstract class CodeOriginInstrumentation extends Tracing
1622
implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
@@ -37,13 +43,28 @@ public String hierarchyMarkerType() {
3743

3844
@Override
3945
public ElementMatcher<TypeDescription> hierarchyMatcher() {
40-
return HierarchyMatchers.declaresMethod(HierarchyMatchers.isAnnotatedWith(matcher));
46+
ElementMatcher.Junction<TypeDescription> matcher =
47+
HierarchyMatchers.declaresMethod(HierarchyMatchers.isAnnotatedWith(this.matcher));
48+
if (InstrumenterConfig.get().isCodeOriginInterfaceSupport()) {
49+
matcher =
50+
matcher.or(
51+
HierarchyMatchers.implementsInterface(
52+
HierarchyMatchers.declaresMethod(
53+
HierarchyMatchers.isAnnotatedWith(this.matcher))));
54+
}
55+
return matcher;
4156
}
4257

4358
@Override
4459
public void methodAdvice(MethodTransformer transformer) {
4560
transformer.applyAdvice(
4661
HierarchyMatchers.isAnnotatedWith(matcher),
4762
"datadog.trace.instrumentation.codeorigin.EntrySpanOriginAdvice");
63+
if (InstrumenterConfig.get().isCodeOriginInterfaceSupport()) {
64+
transformer.applyAdvice(
65+
ElementMatchers.isDeclaredBy(
66+
hasSuperType(isInterface().and(declaresMethod(isAnnotatedWith(matcher))))),
67+
"datadog.trace.instrumentation.codeorigin.EntrySpanOriginAdvice");
68+
}
4869
}
4970
}

‎dd-smoke-tests/debugger-integration-tests/src/main/java/datadog/smoketest/debugger/controller/InterfacedController.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package datadog.smoketest.debugger.controller;
2+
3+
import org.springframework.web.bind.annotation.RequestMapping;
4+
import org.springframework.web.bind.annotation.RestController;
5+
6+
@RestController
7+
public class InterfacedController implements InterfaceApi {
8+
9+
@Override
10+
public String process() {
11+
return "OK";
12+
}
13+
}
14+
15+
interface InterfaceApi {
16+
@RequestMapping("/process")
17+
String process();
18+
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package datadog.smoketest;
2+
3+
import com.datadog.debugger.probe.LogProbe;
4+
import datadog.trace.agent.test.utils.PortUtils;
5+
import datadog.trace.util.TagsHelper;
6+
import java.io.IOException;
7+
import java.nio.file.Files;
8+
import java.nio.file.Path;
9+
import java.time.Duration;
10+
import java.util.List;
11+
import java.util.concurrent.locks.LockSupport;
12+
import okhttp3.OkHttpClient;
13+
import okhttp3.Request;
14+
15+
public class SpringBasedIntegrationTest extends BaseIntegrationTest {
16+
protected static final String DEBUGGER_TEST_APP_CLASS =
17+
"datadog.smoketest.debugger.SpringBootTestApplication";
18+
19+
@Override
20+
protected String getAppClass() {
21+
return DEBUGGER_TEST_APP_CLASS;
22+
}
23+
24+
@Override
25+
protected String getAppId() {
26+
return TagsHelper.sanitize("SpringBootTestApplication");
27+
}
28+
29+
protected String startSpringApp(List<LogProbe> logProbes) throws Exception {
30+
return startSpringApp(logProbes, false);
31+
}
32+
33+
protected String startSpringApp(List<LogProbe> logProbes, boolean enableProcessTags)
34+
throws Exception {
35+
setCurrentConfiguration(createConfig(logProbes));
36+
String httpPort = String.valueOf(PortUtils.randomOpenPort());
37+
ProcessBuilder processBuilder = createProcessBuilder(logFilePath, "--server.port=" + httpPort);
38+
if (!enableProcessTags) {
39+
processBuilder.environment().put("DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED", "false");
40+
}
41+
targetProcess = processBuilder.start();
42+
// assert in logs app started
43+
waitForSpecificLogLine(
44+
logFilePath,
45+
"datadog.smoketest.debugger.SpringBootTestApplication - Started SpringBootTestApplication",
46+
Duration.ofMillis(100),
47+
Duration.ofSeconds(30));
48+
return httpPort;
49+
}
50+
51+
protected void sendRequest(String httpPort, String urlPath) {
52+
OkHttpClient client = new OkHttpClient.Builder().build();
53+
Request request =
54+
new Request.Builder().url("http://localhost:" + httpPort + urlPath).get().build();
55+
try {
56+
client.newCall(request).execute();
57+
} catch (Exception ex) {
58+
ex.printStackTrace();
59+
}
60+
}
61+
62+
protected static void waitForSpecificLogLine(
63+
Path logFilePath, String line, Duration sleep, Duration timeout) throws IOException {
64+
boolean[] result = new boolean[] {false};
65+
long total = sleep.toNanos() == 0 ? 0 : timeout.toNanos() / sleep.toNanos();
66+
int i = 0;
67+
while (i < total && !result[0]) {
68+
Files.lines(logFilePath)
69+
.forEach(
70+
it -> {
71+
if (it.contains(line)) {
72+
result[0] = true;
73+
}
74+
});
75+
LockSupport.parkNanos(sleep.toNanos());
76+
i++;
77+
}
78+
}
79+
}

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package datadog.smoketest;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertFalse;
5+
6+
import datadog.trace.api.DDTags;
7+
import datadog.trace.test.agent.decoder.DecodedSpan;
8+
import datadog.trace.test.agent.decoder.DecodedTrace;
9+
import datadog.trace.test.util.NonRetryable;
10+
import java.nio.file.Path;
11+
import java.time.Duration;
12+
import java.util.Collections;
13+
import java.util.List;
14+
import org.junit.jupiter.api.DisplayName;
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.api.condition.DisabledIf;
17+
18+
@NonRetryable
19+
public class SpringCodeOriginIntegrationTest extends SpringBasedIntegrationTest {
20+
21+
private boolean traceReceived;
22+
23+
@Override
24+
protected ProcessBuilder createProcessBuilder(Path logFilePath, String... params) {
25+
List<String> commandParams = getDebuggerCommandParams();
26+
commandParams.add("-Ddd.trace.enabled=true"); // explicitly enable tracer
27+
commandParams.add("-Ddd.code.origin.for.spans.enabled=true");
28+
commandParams.add("-Ddd.code.origin.for.spans.interface.support=true");
29+
return ProcessBuilderHelper.createProcessBuilder(
30+
commandParams, logFilePath, getAppClass(), params);
31+
}
32+
33+
@Test
34+
@DisplayName("testRegularController")
35+
@DisabledIf(
36+
value = "datadog.environment.JavaVirtualMachine#isJ9",
37+
disabledReason = "Flaky on J9 JVMs")
38+
void testRegularController() throws Exception {
39+
registerTraceListener(this::receiveGreetingTrace);
40+
String httpPort = startSpringApp(Collections.emptyList());
41+
sendRequest(httpPort, "/greeting"); // trigger CodeOriginProbe instrumentation
42+
waitForSpecificLogLine(
43+
logFilePath,
44+
"DEBUG com.datadog.debugger.agent.ConfigurationUpdater - Re-transformation done",
45+
Duration.ofMillis(100),
46+
Duration.ofSeconds(30)); // wait for instrumentation to be done
47+
sendRequest(httpPort, "/greeting"); // generate first span with tags
48+
processRequests(
49+
() -> traceReceived, () -> String.format("Timeout! traceReceived=%s", traceReceived));
50+
}
51+
52+
@Test
53+
@DisplayName("testInterfacedController")
54+
@DisabledIf(
55+
value = "datadog.environment.JavaVirtualMachine#isJ9",
56+
disabledReason = "Flaky on J9 JVMs")
57+
void testInterfacedController() throws Exception {
58+
registerTraceListener(this::receiveProcessTrace);
59+
String httpPort = startSpringApp(Collections.emptyList());
60+
sendRequest(httpPort, "/process"); // trigger CodeOriginProbe instrumentation
61+
waitForSpecificLogLine(
62+
logFilePath,
63+
"DEBUG com.datadog.debugger.agent.ConfigurationUpdater - Re-transformation done",
64+
Duration.ofMillis(100),
65+
Duration.ofSeconds(30)); // wait for instrumentation to be done
66+
sendRequest(httpPort, "/process"); // generate first span with tags
67+
processRequests(
68+
() -> traceReceived, () -> String.format("Timeout! traceReceived=%s", traceReceived));
69+
}
70+
71+
private void receiveGreetingTrace(DecodedTrace decodedTrace) {
72+
for (DecodedSpan span : decodedTrace.getSpans()) {
73+
if (span.getName().equals("spring.handler")
74+
&& span.getResource().equals("WebController.greeting")
75+
&& span.getMeta().containsKey(DDTags.DD_CODE_ORIGIN_FRAME_TYPE)) {
76+
assertEquals("entry", span.getMeta().get(DDTags.DD_CODE_ORIGIN_TYPE));
77+
assertEquals(
78+
"datadog.smoketest.debugger.controller.WebController",
79+
span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_TYPE));
80+
assertEquals("WebController.java", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_FILE));
81+
assertEquals("10", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_LINE));
82+
assertEquals("greeting", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_METHOD));
83+
assertEquals("()", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_SIGNATURE));
84+
assertFalse(traceReceived);
85+
traceReceived = true;
86+
}
87+
}
88+
}
89+
90+
private void receiveProcessTrace(DecodedTrace decodedTrace) {
91+
for (DecodedSpan span : decodedTrace.getSpans()) {
92+
if (span.getName().equals("spring.handler")
93+
&& span.getResource().equals("InterfacedController.process")
94+
&& span.getMeta().containsKey(DDTags.DD_CODE_ORIGIN_FRAME_TYPE)) {
95+
assertEquals("entry", span.getMeta().get(DDTags.DD_CODE_ORIGIN_TYPE));
96+
assertEquals(
97+
"datadog.smoketest.debugger.controller.InterfacedController",
98+
span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_TYPE));
99+
assertEquals(
100+
"InterfacedController.java", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_FILE));
101+
assertEquals("11", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_LINE));
102+
assertEquals("process", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_METHOD));
103+
assertEquals("()", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_SIGNATURE));
104+
assertFalse(traceReceived);
105+
traceReceived = true;
106+
}
107+
}
108+
}
109+
}

0 commit comments

Comments
 (0)