Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the CodeOriginProbe fingerprint to not rely on a stack walk #8016

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Update the CodeOriginProbe fingerprint to not rely on a stack walk #8016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions ...er/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import datadog.trace.api.Config;
import datadog.trace.bootstrap.debugger.util.TimeoutChecker;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import java.lang.reflect.Method;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
Expand Down Expand Up @@ -72,7 +73,9 @@ public interface ExceptionDebugger {
}

public interface CodeOriginRecorder {
String captureCodeOrigin(String signature);
String captureCodeOrigin(boolean entry);

String captureCodeOrigin(Method method, boolean entry);
}

private static volatile ProbeResolver probeResolver;
Expand Down Expand Up @@ -351,11 +354,23 @@ public static void commit(
}
}

public static String captureCodeOrigin(String signature) {
public static String captureCodeOrigin(boolean entry) {
try {
CodeOriginRecorder recorder = codeOriginRecorder;
if (recorder != null) {
return recorder.captureCodeOrigin(entry);
}
} catch (Exception ex) {
LOGGER.debug("Error in captureCodeOrigin: ", ex);
}
return null;
}

public static String captureCodeOrigin(Method method, boolean entry) {
try {
CodeOriginRecorder recorder = codeOriginRecorder;
if (recorder != null) {
return recorder.captureCodeOrigin(signature);
return recorder.captureCodeOrigin(method, entry);
}
} catch (Exception ex) {
LOGGER.debug("Error in captureCodeOrigin: ", ex);
Expand Down
12 changes: 3 additions & 9 deletions ...r-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/spanorigin/CodeOriginInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,23 @@
package datadog.trace.bootstrap.debugger.spanorigin;

import static datadog.trace.bootstrap.debugger.DebuggerContext.captureCodeOrigin;
import static java.util.Arrays.stream;

import datadog.trace.api.InstrumenterConfig;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import java.lang.reflect.Method;
import java.util.stream.Collectors;

public class CodeOriginInfo {
public static void entry(Method method) {
if (InstrumenterConfig.get().isCodeOriginEnabled()) {
String signature =
stream(method.getParameterTypes())
.map(Class::getTypeName)
.collect(Collectors.joining(", ", "(", ")"));
captureCodeOrigin(signature);
captureCodeOrigin(method, true);
}
}

public static void exit(AgentSpan span) {
if (InstrumenterConfig.get().isCodeOriginEnabled()) {
String probeId = captureCodeOrigin(null);
String probeId = captureCodeOrigin(false);
if (span != null) {
span.getLocalRootSpan().setTag(probeId, span);
span.getLocalRootSpan().setTag(probeId, span.getSpanId());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes introduces a bug because here we are expecting a span while we have only an span id

}
}
}
Expand Down
82 changes: 48 additions & 34 deletions ...ent-debugger/src/main/java/com/datadog/debugger/codeorigin/DefaultCodeOriginRecorder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
import datadog.trace.util.AgentTaskScheduler;
import datadog.trace.util.stacktrace.StackWalkerFactory;
import java.lang.reflect.Method;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -27,63 +28,75 @@
public class DefaultCodeOriginRecorder implements CodeOriginRecorder {
private static final Logger LOG = LoggerFactory.getLogger(DefaultCodeOriginRecorder.class);

private final Config config;

private final ConfigurationUpdater configurationUpdater;

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

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

private final AgentTaskScheduler taskScheduler = AgentTaskScheduler.INSTANCE;
private final int maxUserFrames;

public DefaultCodeOriginRecorder(Config config, ConfigurationUpdater configurationUpdater) {
this.config = config;
this.configurationUpdater = configurationUpdater;
maxUserFrames = config.getDebuggerCodeOriginMaxUserFrames();
}

@Override
public String captureCodeOrigin(String signature) {
public String captureCodeOrigin(boolean entry) {
StackTraceElement element = findPlaceInStack();
String fingerprint = Fingerprinter.fingerprint(element);
if (fingerprint == null) {
LOG.debug("Unable to fingerprint stack trace");
return null;
}
CodeOriginProbe probe;

AgentSpan span = AgentTracer.activeSpan();
if (!isAlreadyInstrumented(fingerprint)) {
Where where =
Where.of(
element.getClassName(),
element.getMethodName(),
signature,
String.valueOf(element.getLineNumber()));

if (isAlreadyInstrumented(fingerprint)) {
probe = fingerprints.get(fingerprint);
} else {
probe =
new CodeOriginProbe(
new ProbeId(UUID.randomUUID().toString(), 0),
where.getSignature(),
where,
config.getDebuggerCodeOriginMaxUserFrames());
addFingerprint(fingerprint, probe);

installProbe(probe);
if (span != null) {
// committing here manually so that first run probe encounters decorate the span until the
// instrumentation gets installed
probe.commit(
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
}
createProbe(
fingerprint,
entry,
Where.of(
element.getClassName(),
element.getMethodName(),
null,
String.valueOf(element.getLineNumber())));
}

return probe.getId();
}

@Override
public String captureCodeOrigin(Method method, boolean entry) {
CodeOriginProbe probe;

String fingerPrint = method.toString();
if (isAlreadyInstrumented(fingerPrint)) {
probe = fingerprints.get(fingerPrint);
} else {
probe = fingerprints.get(fingerprint);
probe = createProbe(fingerPrint, entry, Where.of(method));
}

return probe.getId();
}

private CodeOriginProbe createProbe(String fingerPrint, boolean entry, Where where) {
CodeOriginProbe probe;
AgentSpan span = AgentTracer.activeSpan();

probe =
new CodeOriginProbe(
new ProbeId(UUID.randomUUID().toString(), 0), entry, where, maxUserFrames);
addFingerprint(fingerPrint, probe);

installProbe(probe);
// committing here manually so that first run probe encounters decorate the span until the
// instrumentation gets installed
if (span != null) {
probe.commit(
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
}
return probe;
}

private StackTraceElement findPlaceInStack() {
return StackWalkerFactory.INSTANCE.walk(
stream ->
Expand All @@ -104,7 +117,8 @@ void addFingerprint(String fingerprint, CodeOriginProbe probe) {
public String installProbe(CodeOriginProbe probe) {
CodeOriginProbe installed = probes.putIfAbsent(probe.getId(), probe);
if (installed == null) {
taskScheduler.execute(() -> configurationUpdater.accept(CODE_ORIGIN, getProbes()));
AgentTaskScheduler.INSTANCE.execute(
() -> configurationUpdater.accept(CODE_ORIGIN, getProbes()));
return probe.getId();
}
return installed.getId();
Expand Down
24 changes: 14 additions & 10 deletions dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/CodeOriginProbe.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import com.datadog.debugger.agent.DebuggerAgent;
import com.datadog.debugger.instrumentation.InstrumentationResult;
import com.datadog.debugger.sink.DebuggerSink;
import com.datadog.debugger.sink.Snapshot;
import datadog.trace.bootstrap.debugger.CapturedContext;
import datadog.trace.bootstrap.debugger.DebuggerContext;
Expand All @@ -28,14 +29,11 @@ public class CodeOriginProbe extends LogProbe implements ForceMethodInstrumentat

public final int maxFrames;

private final String signature;

private final boolean entrySpanProbe;

public CodeOriginProbe(ProbeId probeId, String signature, Where where, int maxFrames) {
public CodeOriginProbe(ProbeId probeId, boolean entry, Where where, int maxFrames) {
super(LANGUAGE, probeId, null, where, MethodLocation.EXIT, null, null, true, null, null, null);
this.signature = signature;
this.entrySpanProbe = signature != null;
this.entrySpanProbe = entry;
this.maxFrames = maxFrames;
}

Expand Down Expand Up @@ -68,16 +66,19 @@ public void commit(
return;
}
String snapshotId = null;
if (isDebuggerEnabled(span)) {
DebuggerSink sink = DebuggerAgent.getSink();
if (isDebuggerEnabled(span) && sink != null) {
Snapshot snapshot = createSnapshot();
if (fillSnapshot(entryContext, exitContext, caughtExceptions, snapshot)) {
snapshotId = snapshot.getId();
LOGGER.debug("committing code origin probe id={}, snapshot id={}", id, snapshotId);
commitSnapshot(snapshot, DebuggerAgent.getSink());
commitSnapshot(snapshot, sink);
}
}
applySpanOriginTags(span, snapshotId);
DebuggerAgent.getSink().getProbeStatusSink().addEmitting(probeId);
if (sink != null) {
sink.getProbeStatusSink().addEmitting(probeId);
}
span.getLocalRootSpan().setTag(getId(), (String) null); // clear possible span reference
}

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

/** look "back" to find exit spans that may have already come and gone */
private AgentSpan findSpan(AgentSpan candidate) {
if (candidate == null) {
return null;
}
AgentSpan span = candidate;
AgentSpan localRootSpan = candidate.getLocalRootSpan();
if (localRootSpan.getTag(getId()) != null) {
Expand Down
17 changes: 15 additions & 2 deletions dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/Where.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
package com.datadog.debugger.probe;

import static java.util.Arrays.stream;

import com.datadog.debugger.agent.Generated;
import com.datadog.debugger.instrumentation.Types;
import com.datadog.debugger.util.ClassFileLines;
import com.squareup.moshi.JsonAdapter;
import com.squareup.moshi.JsonReader;
import com.squareup.moshi.JsonWriter;
import java.io.IOException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.tree.MethodNode;

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

public static Where of(Method method) {
return of(
method.getDeclaringClass().getName(),
method.getName(),
stream(method.getParameterTypes())
.map(Class::getTypeName)
.collect(Collectors.joining(", ", "(", ")")));
}

protected static SourceLine[] sourceLines(String[] defs) {
if (defs == null) {
if (defs == null || defs.length == 0) {
return null;
}
SourceLine[] lines = new SourceLine[defs.length];
Expand All @@ -72,7 +85,7 @@ public static Where convertLineToMethod(Where lineWhere, ClassFileLines classFil
null);
}
}
throw new IllegalArgumentException("Invalid where to convert from line to method " + lineWhere);
return lineWhere;
}

public String getTypeName() {
Expand Down
41 changes: 34 additions & 7 deletions dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/origin/CodeOriginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void stackDepth() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
installProbes(
new CodeOriginProbe(
CODE_ORIGIN_ID1, null, Where.of(CLASS_NAME, "exit", "()", "39"), MAX_FRAMES));
CODE_ORIGIN_ID1, true, Where.of(CLASS_NAME, "exit", "()", "39"), MAX_FRAMES));

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

@Test
public void testCaptureCodeOriginWithSignature() {
public void testCaptureCodeOriginEntry() {
installProbes();
CodeOriginProbe probe = codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin("()"));
CodeOriginProbe probe = codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin(true));
assertNotNull(probe);
assertTrue(probe.entrySpanProbe());
}

@Test
public void testCaptureCodeOriginWithNullSignature() {
public void testCaptureCodeOriginExit() {
installProbes();
CodeOriginProbe probe = codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin(null));
CodeOriginProbe probe =
codeOriginRecorder.getProbe(codeOriginRecorder.captureCodeOrigin(false));
assertNotNull(probe);
assertFalse(probe.entrySpanProbe());
}

@Test
public void testCaptureCodeOriginWithExplicitInfo()
throws IOException, URISyntaxException, NoSuchMethodException {
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
final Class<?> testClass = compileAndLoadClass(CLASS_NAME);
installProbes();
CodeOriginProbe probe =
codeOriginRecorder.getProbe(
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true));
assertNotNull(probe, "The probe should have been created.");
assertTrue(probe.entrySpanProbe(), "Should be an entry probe.");
}

@Test
public void testDuplicateInstrumentations()
throws IOException, URISyntaxException, NoSuchMethodException {
final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04";
final Class<?> testClass = compileAndLoadClass(CLASS_NAME);
installProbes();
String probe1 =
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true);
String probe2 =
codeOriginRecorder.captureCodeOrigin(testClass.getMethod("main", int.class), true);
assertEquals(probe1, probe2);
}

@NotNull
private List<LogProbe> codeOriginProbes(String type) {
CodeOriginProbe entry =
new CodeOriginProbe(CODE_ORIGIN_ID1, "()", Where.of(type, "entry", "()", "53"), MAX_FRAMES);
new CodeOriginProbe(CODE_ORIGIN_ID1, true, Where.of(type, "entry", "()", "53"), MAX_FRAMES);
CodeOriginProbe exit =
new CodeOriginProbe(CODE_ORIGIN_ID2, null, Where.of(type, "exit", "()", "60"), MAX_FRAMES);
new CodeOriginProbe(CODE_ORIGIN_ID2, false, Where.of(type, "exit", "()", "60"), MAX_FRAMES);
return new ArrayList<>(asList(entry, exit));
}

Expand Down
Loading