Skip to content

Commit 1271b9d

Browse files
jpbempeljpbempel
committed
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
1 parent f007c41 commit 1271b9d

5 files changed

Lines changed: 234 additions & 98 deletions

File tree

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

Lines changed: 17 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,10 +16,14 @@
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;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
1422

1523
public abstract class CodeOriginInstrumentation extends Tracing
1624
implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
1725

26+
private static final Logger log = LoggerFactory.getLogger(CodeOriginInstrumentation.class);
1827
private final OneOf<NamedElement> matcher;
1928

2029
@SuppressForbidden
@@ -37,13 +46,20 @@ public String hierarchyMarkerType() {
3746

3847
@Override
3948
public ElementMatcher<TypeDescription> hierarchyMatcher() {
40-
return HierarchyMatchers.declaresMethod(HierarchyMatchers.isAnnotatedWith(matcher));
49+
return HierarchyMatchers.declaresMethod(HierarchyMatchers.isAnnotatedWith(matcher))
50+
.or(
51+
HierarchyMatchers.implementsInterface(
52+
HierarchyMatchers.declaresMethod(HierarchyMatchers.isAnnotatedWith(matcher))));
4153
}
4254

4355
@Override
4456
public void methodAdvice(MethodTransformer transformer) {
4557
transformer.applyAdvice(
4658
HierarchyMatchers.isAnnotatedWith(matcher),
4759
"datadog.trace.instrumentation.codeorigin.EntrySpanOriginAdvice");
60+
transformer.applyAdvice(
61+
ElementMatchers.isDeclaredBy(
62+
hasSuperType(isInterface().and(declaresMethod(isAnnotatedWith(matcher))))),
63+
"datadog.trace.instrumentation.codeorigin.EntrySpanOriginAdvice");
4864
}
4965
}

‎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: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
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+
return ProcessBuilderHelper.createProcessBuilder(
29+
commandParams, logFilePath, getAppClass(), params);
30+
}
31+
32+
@Test
33+
@DisplayName("testRegularController")
34+
@DisabledIf(
35+
value = "datadog.environment.JavaVirtualMachine#isJ9",
36+
disabledReason = "Flaky on J9 JVMs")
37+
void testRegularController() throws Exception {
38+
registerTraceListener(this::receiveGreetingTrace);
39+
String httpPort = startSpringApp(Collections.emptyList());
40+
sendRequest(httpPort, "/greeting"); // trigger CodeOriginProbe instrumentation
41+
waitForSpecificLogLine(
42+
logFilePath,
43+
"DEBUG com.datadog.debugger.agent.ConfigurationUpdater - Re-transformation done",
44+
Duration.ofMillis(100),
45+
Duration.ofSeconds(30)); // wait for instrumentation to be done
46+
sendRequest(httpPort, "/greeting"); // generate first span with tags
47+
processRequests(
48+
() -> traceReceived, () -> String.format("Timeout! traceReceived=%s", traceReceived));
49+
}
50+
51+
@Test
52+
@DisplayName("testInterfacedController")
53+
@DisabledIf(
54+
value = "datadog.environment.JavaVirtualMachine#isJ9",
55+
disabledReason = "Flaky on J9 JVMs")
56+
void testInterfacedController() throws Exception {
57+
registerTraceListener(this::receiveProcessTrace);
58+
String httpPort = startSpringApp(Collections.emptyList());
59+
sendRequest(httpPort, "/process"); // trigger CodeOriginProbe instrumentation
60+
waitForSpecificLogLine(
61+
logFilePath,
62+
"DEBUG com.datadog.debugger.agent.ConfigurationUpdater - Re-transformation done",
63+
Duration.ofMillis(100),
64+
Duration.ofSeconds(30)); // wait for instrumentation to be done
65+
sendRequest(httpPort, "/process"); // generate first span with tags
66+
processRequests(
67+
() -> traceReceived, () -> String.format("Timeout! traceReceived=%s", traceReceived));
68+
}
69+
70+
private void receiveGreetingTrace(DecodedTrace decodedTrace) {
71+
for (DecodedSpan span : decodedTrace.getSpans()) {
72+
if (span.getName().equals("spring.handler")
73+
&& span.getResource().equals("WebController.greeting")
74+
&& span.getMeta().containsKey(DDTags.DD_CODE_ORIGIN_FRAME_TYPE)) {
75+
assertEquals("entry", span.getMeta().get(DDTags.DD_CODE_ORIGIN_TYPE));
76+
assertEquals(
77+
"datadog.smoketest.debugger.controller.WebController",
78+
span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_TYPE));
79+
assertEquals("WebController.java", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_FILE));
80+
assertEquals("10", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_LINE));
81+
assertEquals("greeting", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_METHOD));
82+
assertEquals("()", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_SIGNATURE));
83+
assertFalse(traceReceived);
84+
traceReceived = true;
85+
}
86+
}
87+
}
88+
89+
private void receiveProcessTrace(DecodedTrace decodedTrace) {
90+
for (DecodedSpan span : decodedTrace.getSpans()) {
91+
if (span.getName().equals("spring.handler")
92+
&& span.getResource().equals("InterfacedController.process")
93+
&& span.getMeta().containsKey(DDTags.DD_CODE_ORIGIN_FRAME_TYPE)) {
94+
assertEquals("entry", span.getMeta().get(DDTags.DD_CODE_ORIGIN_TYPE));
95+
assertEquals(
96+
"datadog.smoketest.debugger.controller.InterfacedController",
97+
span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_TYPE));
98+
assertEquals(
99+
"InterfacedController.java", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_FILE));
100+
assertEquals("11", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_LINE));
101+
assertEquals("process", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_METHOD));
102+
assertEquals("()", span.getMeta().get(DDTags.DD_CODE_ORIGIN_FRAME_SIGNATURE));
103+
assertFalse(traceReceived);
104+
traceReceived = true;
105+
}
106+
}
107+
}
108+
}

0 commit comments

Comments
 (0)