Skip to content

Commit 8c921d8

Browse files
evanchoolyPerfectSlayer
evanchooly
authored and
PerfectSlayer
committed
Update the CodeOriginProbe fingerprint to not rely on a stack walk (#8016)
1 parent 1f358e7 commit 8c921d8

File tree

6 files changed

+132
-65
lines changed

6 files changed

+132
-65
lines changed

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

+18-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datadog.trace.api.Config;
44
import datadog.trace.bootstrap.debugger.util.TimeoutChecker;
55
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
6+
import java.lang.reflect.Method;
67
import java.time.Duration;
78
import java.time.temporal.ChronoUnit;
89
import java.util.ArrayList;
@@ -72,7 +73,9 @@ public interface ExceptionDebugger {
7273
}
7374

7475
public interface CodeOriginRecorder {
75-
String captureCodeOrigin(String signature);
76+
String captureCodeOrigin(boolean entry);
77+
78+
String captureCodeOrigin(Method method, boolean entry);
7679
}
7780

7881
private static volatile ProbeResolver probeResolver;
@@ -351,11 +354,23 @@ public static void commit(
351354
}
352355
}
353356

354-
public static String captureCodeOrigin(String signature) {
357+
public static String captureCodeOrigin(boolean entry) {
358+
try {
359+
CodeOriginRecorder recorder = codeOriginRecorder;
360+
if (recorder != null) {
361+
return recorder.captureCodeOrigin(entry);
362+
}
363+
} catch (Exception ex) {
364+
LOGGER.debug("Error in captureCodeOrigin: ", ex);
365+
}
366+
return null;
367+
}
368+
369+
public static String captureCodeOrigin(Method method, boolean entry) {
355370
try {
356371
CodeOriginRecorder recorder = codeOriginRecorder;
357372
if (recorder != null) {
358-
return recorder.captureCodeOrigin(signature);
373+
return recorder.captureCodeOrigin(method, entry);
359374
}
360375
} catch (Exception ex) {
361376
LOGGER.debug("Error in captureCodeOrigin: ", ex);

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

+3-9
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,23 @@
11
package datadog.trace.bootstrap.debugger.spanorigin;
22

33
import static datadog.trace.bootstrap.debugger.DebuggerContext.captureCodeOrigin;
4-
import static java.util.Arrays.stream;
54

65
import datadog.trace.api.InstrumenterConfig;
76
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
87
import java.lang.reflect.Method;
9-
import java.util.stream.Collectors;
108

119
public class CodeOriginInfo {
1210
public static void entry(Method method) {
1311
if (InstrumenterConfig.get().isCodeOriginEnabled()) {
14-
String signature =
15-
stream(method.getParameterTypes())
16-
.map(Class::getTypeName)
17-
.collect(Collectors.joining(", ", "(", ")"));
18-
captureCodeOrigin(signature);
12+
captureCodeOrigin(method, true);
1913
}
2014
}
2115

2216
public static void exit(AgentSpan span) {
2317
if (InstrumenterConfig.get().isCodeOriginEnabled()) {
24-
String probeId = captureCodeOrigin(null);
18+
String probeId = captureCodeOrigin(false);
2519
if (span != null) {
26-
span.getLocalRootSpan().setTag(probeId, span);
20+
span.getLocalRootSpan().setTag(probeId, span.getSpanId());
2721
}
2822
}
2923
}

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

+48-34
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
1616
import datadog.trace.util.AgentTaskScheduler;
1717
import datadog.trace.util.stacktrace.StackWalkerFactory;
18+
import java.lang.reflect.Method;
1819
import java.util.Collection;
1920
import java.util.Collections;
2021
import java.util.HashMap;
@@ -27,63 +28,75 @@
2728
public class DefaultCodeOriginRecorder implements CodeOriginRecorder {
2829
private static final Logger LOG = LoggerFactory.getLogger(DefaultCodeOriginRecorder.class);
2930

30-
private final Config config;
31-
3231
private final ConfigurationUpdater configurationUpdater;
3332

3433
private final Map<String, CodeOriginProbe> fingerprints = new HashMap<>();
3534

3635
private final Map<String, CodeOriginProbe> probes = new ConcurrentHashMap<>();
3736

38-
private final AgentTaskScheduler taskScheduler = AgentTaskScheduler.INSTANCE;
37+
private final int maxUserFrames;
3938

4039
public DefaultCodeOriginRecorder(Config config, ConfigurationUpdater configurationUpdater) {
41-
this.config = config;
4240
this.configurationUpdater = configurationUpdater;
41+
maxUserFrames = config.getDebuggerCodeOriginMaxUserFrames();
4342
}
4443

4544
@Override
46-
public String captureCodeOrigin(String signature) {
45+
public String captureCodeOrigin(boolean entry) {
4746
StackTraceElement element = findPlaceInStack();
4847
String fingerprint = Fingerprinter.fingerprint(element);
49-
if (fingerprint == null) {
50-
LOG.debug("Unable to fingerprint stack trace");
51-
return null;
52-
}
5348
CodeOriginProbe probe;
5449

55-
AgentSpan span = AgentTracer.activeSpan();
56-
if (!isAlreadyInstrumented(fingerprint)) {
57-
Where where =
58-
Where.of(
59-
element.getClassName(),
60-
element.getMethodName(),
61-
signature,
62-
String.valueOf(element.getLineNumber()));
63-
50+
if (isAlreadyInstrumented(fingerprint)) {
51+
probe = fingerprints.get(fingerprint);
52+
} else {
6453
probe =
65-
new CodeOriginProbe(
66-
new ProbeId(UUID.randomUUID().toString(), 0),
67-
where.getSignature(),
68-
where,
69-
config.getDebuggerCodeOriginMaxUserFrames());
70-
addFingerprint(fingerprint, probe);
71-
72-
installProbe(probe);
73-
if (span != null) {
74-
// committing here manually so that first run probe encounters decorate the span until the
75-
// instrumentation gets installed
76-
probe.commit(
77-
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
78-
}
54+
createProbe(
55+
fingerprint,
56+
entry,
57+
Where.of(
58+
element.getClassName(),
59+
element.getMethodName(),
60+
null,
61+
String.valueOf(element.getLineNumber())));
62+
}
63+
64+
return probe.getId();
65+
}
7966

67+
@Override
68+
public String captureCodeOrigin(Method method, boolean entry) {
69+
CodeOriginProbe probe;
70+
71+
String fingerPrint = method.toString();
72+
if (isAlreadyInstrumented(fingerPrint)) {
73+
probe = fingerprints.get(fingerPrint);
8074
} else {
81-
probe = fingerprints.get(fingerprint);
75+
probe = createProbe(fingerPrint, entry, Where.of(method));
8276
}
8377

8478
return probe.getId();
8579
}
8680

81+
private CodeOriginProbe createProbe(String fingerPrint, boolean entry, Where where) {
82+
CodeOriginProbe probe;
83+
AgentSpan span = AgentTracer.activeSpan();
84+
85+
probe =
86+
new CodeOriginProbe(
87+
new ProbeId(UUID.randomUUID().toString(), 0), entry, where, maxUserFrames);
88+
addFingerprint(fingerPrint, probe);
89+
90+
installProbe(probe);
91+
// committing here manually so that first run probe encounters decorate the span until the
92+
// instrumentation gets installed
93+
if (span != null) {
94+
probe.commit(
95+
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
96+
}
97+
return probe;
98+
}
99+
87100
private StackTraceElement findPlaceInStack() {
88101
return StackWalkerFactory.INSTANCE.walk(
89102
stream ->
@@ -104,7 +117,8 @@ void addFingerprint(String fingerprint, CodeOriginProbe probe) {
104117
public String installProbe(CodeOriginProbe probe) {
105118
CodeOriginProbe installed = probes.putIfAbsent(probe.getId(), probe);
106119
if (installed == null) {
107-
taskScheduler.execute(() -> configurationUpdater.accept(CODE_ORIGIN, getProbes()));
120+
AgentTaskScheduler.INSTANCE.execute(
121+
() -> configurationUpdater.accept(CODE_ORIGIN, getProbes()));
108122
return probe.getId();
109123
}
110124
return installed.getId();

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

+14-10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import com.datadog.debugger.agent.DebuggerAgent;
1111
import com.datadog.debugger.instrumentation.InstrumentationResult;
12+
import com.datadog.debugger.sink.DebuggerSink;
1213
import com.datadog.debugger.sink.Snapshot;
1314
import datadog.trace.bootstrap.debugger.CapturedContext;
1415
import datadog.trace.bootstrap.debugger.DebuggerContext;
@@ -28,14 +29,11 @@ public class CodeOriginProbe extends LogProbe implements ForceMethodInstrumentat
2829

2930
public final int maxFrames;
3031

31-
private final String signature;
32-
3332
private final boolean entrySpanProbe;
3433

35-
public CodeOriginProbe(ProbeId probeId, String signature, Where where, int maxFrames) {
34+
public CodeOriginProbe(ProbeId probeId, boolean entry, Where where, int maxFrames) {
3635
super(LANGUAGE, probeId, null, where, MethodLocation.EXIT, null, null, true, null, null, null);
37-
this.signature = signature;
38-
this.entrySpanProbe = signature != null;
36+
this.entrySpanProbe = entry;
3937
this.maxFrames = maxFrames;
4038
}
4139

@@ -68,16 +66,19 @@ public void commit(
6866
return;
6967
}
7068
String snapshotId = null;
71-
if (isDebuggerEnabled(span)) {
69+
DebuggerSink sink = DebuggerAgent.getSink();
70+
if (isDebuggerEnabled(span) && sink != null) {
7271
Snapshot snapshot = createSnapshot();
7372
if (fillSnapshot(entryContext, exitContext, caughtExceptions, snapshot)) {
7473
snapshotId = snapshot.getId();
7574
LOGGER.debug("committing code origin probe id={}, snapshot id={}", id, snapshotId);
76-
commitSnapshot(snapshot, DebuggerAgent.getSink());
75+
commitSnapshot(snapshot, sink);
7776
}
7877
}
7978
applySpanOriginTags(span, snapshotId);
80-
DebuggerAgent.getSink().getProbeStatusSink().addEmitting(probeId);
79+
if (sink != null) {
80+
sink.getProbeStatusSink().addEmitting(probeId);
81+
}
8182
span.getLocalRootSpan().setTag(getId(), (String) null); // clear possible span reference
8283
}
8384

@@ -95,8 +96,8 @@ private void applySpanOriginTags(AgentSpan span, String snapshotId) {
9596
s.setTag(format(DD_CODE_ORIGIN_FRAME, i, "method"), info.getMethodName());
9697
s.setTag(format(DD_CODE_ORIGIN_FRAME, i, "line"), info.getLineNumber());
9798
s.setTag(format(DD_CODE_ORIGIN_FRAME, i, "type"), info.getClassName());
98-
if (i == 0 && signature != null) {
99-
s.setTag(format(DD_CODE_ORIGIN_FRAME, i, "signature"), signature);
99+
if (i == 0 && entrySpanProbe) {
100+
s.setTag(format(DD_CODE_ORIGIN_FRAME, i, "signature"), where.getSignature());
100101
}
101102
if (i == 0 && snapshotId != null) {
102103
s.setTag(format(DD_CODE_ORIGIN_FRAME, i, "snapshot_id"), snapshotId);
@@ -111,6 +112,9 @@ public boolean entrySpanProbe() {
111112

112113
/** look "back" to find exit spans that may have already come and gone */
113114
private AgentSpan findSpan(AgentSpan candidate) {
115+
if (candidate == null) {
116+
return null;
117+
}
114118
AgentSpan span = candidate;
115119
AgentSpan localRootSpan = candidate.getLocalRootSpan();
116120
if (localRootSpan.getTag(getId()) != null) {

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

+15-2
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
11
package com.datadog.debugger.probe;
22

3+
import static java.util.Arrays.stream;
4+
35
import com.datadog.debugger.agent.Generated;
46
import com.datadog.debugger.instrumentation.Types;
57
import com.datadog.debugger.util.ClassFileLines;
68
import com.squareup.moshi.JsonAdapter;
79
import com.squareup.moshi.JsonReader;
810
import com.squareup.moshi.JsonWriter;
911
import java.io.IOException;
12+
import java.lang.reflect.Method;
1013
import java.util.ArrayList;
1114
import java.util.Arrays;
1215
import java.util.List;
1316
import java.util.Objects;
1417
import java.util.regex.Matcher;
1518
import java.util.regex.Pattern;
19+
import java.util.stream.Collectors;
1620
import org.objectweb.asm.Opcodes;
1721
import org.objectweb.asm.tree.MethodNode;
1822

@@ -46,8 +50,17 @@ public static Where of(String typeName, String methodName, String signature, Str
4650
return new Where(typeName, methodName, signature, lines, null);
4751
}
4852

53+
public static Where of(Method method) {
54+
return of(
55+
method.getDeclaringClass().getName(),
56+
method.getName(),
57+
stream(method.getParameterTypes())
58+
.map(Class::getTypeName)
59+
.collect(Collectors.joining(", ", "(", ")")));
60+
}
61+
4962
protected static SourceLine[] sourceLines(String[] defs) {
50-
if (defs == null) {
63+
if (defs == null || defs.length == 0) {
5164
return null;
5265
}
5366
SourceLine[] lines = new SourceLine[defs.length];
@@ -72,7 +85,7 @@ public static Where convertLineToMethod(Where lineWhere, ClassFileLines classFil
7285
null);
7386
}
7487
}
75-
throw new IllegalArgumentException("Invalid where to convert from line to method " + lineWhere);
88+
return lineWhere;
7689
}
7790

7891
public String getTypeName() {

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

+34-7
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void stackDepth() throws IOException, URISyntaxException {
119119
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
120120
installProbes(
121121
new CodeOriginProbe(
122-
CODE_ORIGIN_ID1, null, Where.of(CLASS_NAME, "exit", "()", "39"), MAX_FRAMES));
122+
CODE_ORIGIN_ID1, true, Where.of(CLASS_NAME, "exit", "()", "39"), MAX_FRAMES));
123123

124124
Class<?> testClass = compileAndLoadClass("com.datadog.debugger.CodeOrigin04");
125125
countFrames(testClass, 10);
@@ -139,27 +139,54 @@ private void countFrames(Class<?> testClass, int loops) {
139139
}
140140

141141
@Test
142-
public void testCaptureCodeOriginWithSignature() {
142+
public void testCaptureCodeOriginEntry() {
143143
installProbes();
144-
CodeOriginProbe probe = codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin("()"));
144+
CodeOriginProbe probe = codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin(true));
145145
assertNotNull(probe);
146146
assertTrue(probe.entrySpanProbe());
147147
}
148148

149149
@Test
150-
public void testCaptureCodeOriginWithNullSignature() {
150+
public void testCaptureCodeOriginExit() {
151151
installProbes();
152-
CodeOriginProbe probe = codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin(null));
152+
CodeOriginProbe probe =
153+
codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin(false));
153154
assertNotNull(probe);
154155
assertFalse(probe.entrySpanProbe());
155156
}
156157

158+
@Test
159+
public void testCaptureCodeOriginWithExplicitInfo()
160+
throws IOException, URISyntaxException, NoSuchMethodException {
161+
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
162+
final Class<?> testClass = compileAndLoadClass(CLASS_NAME);
163+
installProbes();
164+
CodeOriginProbe probe =
165+
codeOriginRecorder.getProbe(
166+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true));
167+
assertNotNull(probe, "The probe should have been created.");
168+
assertTrue(probe.entrySpanProbe(), "Should be an entry probe.");
169+
}
170+
171+
@Test
172+
public void testDuplicateInstrumentations()
173+
throws IOException, URISyntaxException, NoSuchMethodException {
174+
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
175+
final Class<?> testClass = compileAndLoadClass(CLASS_NAME);
176+
installProbes();
177+
String probe1 =
178+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true);
179+
String probe2 =
180+
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true);
181+
assertEquals(probe1, probe2);
182+
}
183+
157184
@NotNull
158185
private List<LogProbe> codeOriginProbes(String type) {
159186
CodeOriginProbe entry =
160-
new CodeOriginProbe(CODE_ORIGIN_ID1, "()", Where.of(type, "entry", "()", "53"), MAX_FRAMES);
187+
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(type, "entry", "()", "53"), MAX_FRAMES);
161188
CodeOriginProbe exit =
162-
new CodeOriginProbe(CODE_ORIGIN_ID2, null, Where.of(type, "exit", "()", "60"), MAX_FRAMES);
189+
new CodeOriginProbe(CODE_ORIGIN_ID2, false, Where.of(type, "exit", "()", "60"), MAX_FRAMES);
163190
return new ArrayList<>(asList(entry, exit));
164191
}
165192

0 commit comments

Comments
 (0)