Skip to content

Commit ec5eedc

Browse files
evanchoolyevanchooly
authored
Fix the normalization of Where definitions (#7826)
* Fix the normalization of Where definitions * rebased * remove build entry * introduce ClassNameFilter interface move filtering to DebuggerContext tests are passing but jacoco complains * update the resolver * consolidated CodeOriginRecorder and CodeOriginProbeManager jacoco is happy * add try/catch * add a null check * null check causes jacoco to fail. later.
1 parent 6c04a2f commit ec5eedc

26 files changed

+1004
-906
lines changed

‎dd-java-agent/agent-debugger/build.gradle‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,3 @@ jar {
7676

7777
// we want to test with no special reflective access (no --add-opens)
7878
ext.allowReflectiveAccessToJdk = false
79-

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ public interface ClassFilter {
3232
boolean isDenied(String fullyQualifiedClassName);
3333
}
3434

35+
public interface ClassNameFilter {
36+
boolean isExcluded(String className);
37+
}
38+
3539
public enum MetricKind {
3640
COUNT,
3741
GAUGE,
@@ -73,6 +77,7 @@ public interface CodeOriginRecorder {
7377

7478
private static volatile ProbeResolver probeResolver;
7579
private static volatile ClassFilter classFilter;
80+
private static volatile ClassNameFilter classNameFilter;
7681
private static volatile MetricForwarder metricForwarder;
7782
private static volatile Tracer tracer;
7883
private static volatile ValueSerializer valueSerializer;
@@ -95,6 +100,10 @@ public static void initClassFilter(ClassFilter classFilter) {
95100
DebuggerContext.classFilter = classFilter;
96101
}
97102

103+
public static void initClassNameFilter(ClassNameFilter classNameFilter) {
104+
DebuggerContext.classNameFilter = classNameFilter;
105+
}
106+
98107
public static void initValueSerializer(ValueSerializer valueSerializer) {
99108
DebuggerContext.valueSerializer = valueSerializer;
100109
}
@@ -365,4 +374,13 @@ public static void handleException(Throwable t, AgentSpan span) {
365374
LOGGER.debug("Error in handleException: ", ex);
366375
}
367376
}
377+
378+
public static boolean isClassNameExcluded(String className) {
379+
try {
380+
return classNameFilter != null && classNameFilter.isExcluded(className);
381+
} catch (Exception ex) {
382+
LOGGER.debug("Error in isClassNameExcluded: ", ex);
383+
return false;
384+
}
385+
}
368386
}

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import static com.datadog.debugger.agent.ConfigurationAcceptor.Source.REMOTE_CONFIG;
44
import static datadog.trace.util.AgentThreadFactory.AGENT_THREAD_GROUP;
55

6-
import com.datadog.debugger.codeorigin.CodeOriginProbeManager;
76
import com.datadog.debugger.codeorigin.DefaultCodeOriginRecorder;
87
import com.datadog.debugger.exception.DefaultExceptionDebugger;
98
import com.datadog.debugger.exception.ExceptionProbeManager;
@@ -25,6 +24,7 @@
2524
import datadog.trace.api.git.GitInfo;
2625
import datadog.trace.api.git.GitInfoProvider;
2726
import datadog.trace.bootstrap.debugger.DebuggerContext;
27+
import datadog.trace.bootstrap.debugger.DebuggerContext.ClassNameFilter;
2828
import datadog.trace.bootstrap.debugger.util.Redaction;
2929
import datadog.trace.bootstrap.instrumentation.api.Tags;
3030
import datadog.trace.core.DDTraceCoreInfo;
@@ -70,7 +70,7 @@ public static synchronized void run(
7070
config, diagnosticEndpoint, ddAgentFeaturesDiscovery.supportsDebuggerDiagnostics());
7171
DebuggerSink debuggerSink = createDebuggerSink(config, probeStatusSink);
7272
debuggerSink.start();
73-
ClassNameFiltering classNameFiltering = new ClassNameFiltering(config);
73+
ClassNameFilter classNameFilter = new ClassNameFiltering(config);
7474
ConfigurationUpdater configurationUpdater =
7575
new ConfigurationUpdater(
7676
instrumentation,
@@ -87,21 +87,20 @@ public static synchronized void run(
8787
snapshotSerializer = new JsonSnapshotSerializer();
8888
DebuggerContext.initValueSerializer(snapshotSerializer);
8989
DebuggerContext.initTracer(new DebuggerTracer(debuggerSink.getProbeStatusSink()));
90+
DebuggerContext.initClassNameFilter(classNameFilter);
9091
DefaultExceptionDebugger defaultExceptionDebugger = null;
9192
if (config.isDebuggerExceptionEnabled()) {
9293
LOGGER.info("Starting Exception Replay");
9394
defaultExceptionDebugger =
9495
new DefaultExceptionDebugger(
9596
configurationUpdater,
96-
classNameFiltering,
97+
classNameFilter,
9798
Duration.ofSeconds(config.getDebuggerExceptionCaptureInterval()),
9899
config.getDebuggerMaxExceptionPerSecond());
99100
DebuggerContext.initExceptionDebugger(defaultExceptionDebugger);
100101
}
101102
if (config.isDebuggerCodeOriginEnabled()) {
102-
DebuggerContext.initCodeOrigin(
103-
new DefaultCodeOriginRecorder(
104-
new CodeOriginProbeManager(configurationUpdater, classNameFiltering)));
103+
DebuggerContext.initCodeOrigin(new DefaultCodeOriginRecorder(configurationUpdater));
105104
}
106105
if (config.isDebuggerInstrumentTheWorld()) {
107106
setupInstrumentTheWorldTransformer(
@@ -110,7 +109,7 @@ public static synchronized void run(
110109
// Dynamic Instrumentation
111110
if (config.isDebuggerEnabled()) {
112111
startDynamicInstrumentation(
113-
instrumentation, sco, config, configurationUpdater, debuggerSink, classNameFiltering);
112+
instrumentation, sco, config, configurationUpdater, debuggerSink, classNameFilter);
114113
}
115114
try {
116115
/*
@@ -135,7 +134,7 @@ private static void startDynamicInstrumentation(
135134
Config config,
136135
ConfigurationUpdater configurationUpdater,
137136
DebuggerSink debuggerSink,
138-
ClassNameFiltering classNameFiltering) {
137+
ClassNameFilter classNameFilter) {
139138
LOGGER.info("Starting Dynamic Instrumentation");
140139
String probeFileLocation = config.getDebuggerProbeFileLocation();
141140
if (probeFileLocation != null) {
@@ -152,7 +151,7 @@ private static void startDynamicInstrumentation(
152151
config,
153152
new SymbolAggregator(
154153
debuggerSink.getSymbolSink(), config.getDebuggerSymbolFlushThreshold()),
155-
classNameFiltering);
154+
classNameFilter);
156155
if (config.isDebuggerSymbolForceUpload()) {
157156
symDBEnablement.startSymbolExtraction();
158157
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ private ProbeDefinition selectReferenceDefinition(
687687
for (ProbeDefinition definition : capturedContextProbes) {
688688
if (definition instanceof LogProbe) {
689689
if (definition instanceof ForceMethodInstrumentation) {
690-
where = Where.convertLineToMethod(where, classFileLines);
690+
where = Where.convertLineToMethod(definition.getWhere(), classFileLines);
691691
}
692692
hasLogProbe = true;
693693
LogProbe logProbe = (LogProbe) definition;

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

Lines changed: 0 additions & 115 deletions
This file was deleted.

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

Lines changed: 100 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,113 @@
11
package com.datadog.debugger.codeorigin;
22

3+
import static com.datadog.debugger.agent.ConfigurationAcceptor.Source.CODE_ORIGIN;
4+
5+
import com.datadog.debugger.agent.ConfigurationUpdater;
6+
import com.datadog.debugger.exception.Fingerprinter;
7+
import com.datadog.debugger.probe.CodeOriginProbe;
8+
import com.datadog.debugger.probe.Where;
9+
import datadog.trace.bootstrap.debugger.CapturedContext;
10+
import datadog.trace.bootstrap.debugger.DebuggerContext;
311
import datadog.trace.bootstrap.debugger.DebuggerContext.CodeOriginRecorder;
12+
import datadog.trace.bootstrap.debugger.ProbeId;
13+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
14+
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
15+
import datadog.trace.util.AgentTaskScheduler;
16+
import datadog.trace.util.stacktrace.StackWalkerFactory;
17+
import java.util.Collection;
18+
import java.util.Collections;
19+
import java.util.HashMap;
20+
import java.util.Map;
21+
import java.util.UUID;
22+
import java.util.concurrent.ConcurrentHashMap;
23+
import org.slf4j.Logger;
24+
import org.slf4j.LoggerFactory;
425

526
public class DefaultCodeOriginRecorder implements CodeOriginRecorder {
6-
private final CodeOriginProbeManager probeManager;
27+
private static final Logger LOG = LoggerFactory.getLogger(DefaultCodeOriginRecorder.class);
728

8-
public DefaultCodeOriginRecorder(CodeOriginProbeManager probeManager) {
9-
this.probeManager = probeManager;
10-
}
29+
private final ConfigurationUpdater configurationUpdater;
30+
31+
private final Map<String, CodeOriginProbe> fingerprints = new HashMap<>();
1132

12-
public CodeOriginProbeManager probeManager() {
13-
return probeManager;
33+
private final Map<String, CodeOriginProbe> probes = new ConcurrentHashMap<>();
34+
35+
private final AgentTaskScheduler taskScheduler = AgentTaskScheduler.INSTANCE;
36+
37+
public DefaultCodeOriginRecorder(ConfigurationUpdater configurationUpdater) {
38+
this.configurationUpdater = configurationUpdater;
1439
}
1540

1641
@Override
1742
public String captureCodeOrigin(String signature) {
18-
return probeManager.createProbeForFrame(signature);
43+
StackTraceElement element = findPlaceInStack();
44+
String fingerprint = Fingerprinter.fingerprint(element);
45+
if (fingerprint == null) {
46+
LOG.debug("Unable to fingerprint stack trace");
47+
return null;
48+
}
49+
CodeOriginProbe probe;
50+
51+
AgentSpan span = AgentTracer.activeSpan();
52+
if (!isAlreadyInstrumented(fingerprint)) {
53+
Where where =
54+
Where.of(
55+
element.getClassName(),
56+
element.getMethodName(),
57+
signature,
58+
String.valueOf(element.getLineNumber()));
59+
60+
probe =
61+
new CodeOriginProbe(
62+
new ProbeId(UUID.randomUUID().toString(), 0), where.getSignature(), where);
63+
addFingerprint(fingerprint, probe);
64+
65+
installProbe(probe);
66+
if (span != null) {
67+
// committing here manually so that first run probe encounters decorate the span until the
68+
// instrumentation gets installed
69+
probe.commit(
70+
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
71+
}
72+
73+
} else {
74+
probe = fingerprints.get(fingerprint);
75+
}
76+
77+
return probe.getId();
78+
}
79+
80+
private StackTraceElement findPlaceInStack() {
81+
return StackWalkerFactory.INSTANCE.walk(
82+
stream ->
83+
stream
84+
.filter(element -> !DebuggerContext.isClassNameExcluded(element.getClassName()))
85+
.findFirst()
86+
.orElse(null));
87+
}
88+
89+
public boolean isAlreadyInstrumented(String fingerprint) {
90+
return fingerprints.containsKey(fingerprint);
91+
}
92+
93+
void addFingerprint(String fingerprint, CodeOriginProbe probe) {
94+
fingerprints.putIfAbsent(fingerprint, probe);
95+
}
96+
97+
public String installProbe(CodeOriginProbe probe) {
98+
CodeOriginProbe installed = probes.putIfAbsent(probe.getId(), probe);
99+
if (installed == null) {
100+
taskScheduler.execute(() -> configurationUpdater.accept(CODE_ORIGIN, getProbes()));
101+
return probe.getId();
102+
}
103+
return installed.getId();
104+
}
105+
106+
public CodeOriginProbe getProbe(String probeId) {
107+
return probes.get(probeId);
108+
}
109+
110+
public Collection<CodeOriginProbe> getProbes() {
111+
return probes.values();
19112
}
20113
}

0 commit comments

Comments
 (0)