Skip to content

Commit 8baf39a

Browse files
committed
fix the probe name/fingerprint
update tests to assert tags actually present
1 parent 8cd1292 commit 8baf39a

File tree

7 files changed

+413
-208
lines changed

7 files changed

+413
-208
lines changed

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ public interface ExceptionDebugger {
7575
public interface CodeOriginRecorder {
7676
String captureCodeOrigin(String signature);
7777

78-
String captureCodeOrigin(AgentSpan span, boolean entry);
79-
8078
String captureCodeOrigin(
8179
String name, Class<?> target, String method, Class<?>[] types, boolean entry);
8280
}
@@ -381,18 +379,6 @@ public static String captureCodeOrigin(Function<CodeOriginRecorder, String> func
381379
return null;
382380
}
383381

384-
public static String captureCodeOrigin(AgentSpan span, boolean entry) {
385-
try {
386-
CodeOriginRecorder recorder = codeOriginRecorder;
387-
if (recorder != null) {
388-
return recorder.captureCodeOrigin(span, entry);
389-
}
390-
} catch (Exception ex) {
391-
LOGGER.debug("Error in captureCodeOrigin: ", ex);
392-
}
393-
return null;
394-
}
395-
396382
public static String captureCodeOrigin(
397383
String name, Class<?> target, String method, Class<?>[] types, boolean entry) {
398384
try {

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ public static void entry(Class<?> type, Method method) {
2929
}
3030
}
3131

32-
public static void entry(AgentSpan span) {
33-
if (InstrumenterConfig.get().isCodeOriginEnabled()) {
34-
captureCodeOrigin(span, true);
35-
}
36-
}
37-
3832
public static void entry(String name, Class<?> target, String method, Class<?>[] types) {
3933
if (InstrumenterConfig.get().isCodeOriginEnabled()) {
4034
captureCodeOrigin(name, target, method, types, true);

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

Lines changed: 3 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import com.datadog.debugger.exception.Fingerprinter;
99
import com.datadog.debugger.probe.CodeOriginProbe;
1010
import com.datadog.debugger.probe.Where;
11-
import com.datadog.debugger.util.ClassFileLines;
1211
import com.datadog.debugger.util.ClassNameFiltering;
1312
import datadog.trace.api.Config;
1413
import datadog.trace.bootstrap.debugger.CapturedContext;
@@ -26,14 +25,12 @@
2625
import java.util.Collections;
2726
import java.util.HashMap;
2827
import java.util.HashSet;
29-
import java.util.List;
3028
import java.util.Map;
3129
import java.util.UUID;
3230
import java.util.concurrent.ConcurrentHashMap;
3331
import java.util.stream.Collectors;
3432
import org.objectweb.asm.ClassReader;
3533
import org.objectweb.asm.tree.ClassNode;
36-
import org.objectweb.asm.tree.MethodNode;
3734
import org.slf4j.Logger;
3835
import org.slf4j.LoggerFactory;
3936

@@ -140,15 +137,10 @@ public String captureCodeOrigin(String signature) {
140137
@Override
141138
public String captureCodeOrigin(
142139
String name, Class<?> target, String method, Class<?>[] types, boolean entry) {
143-
String fingerprint = Fingerprinter.fingerprint(name);
144-
if (fingerprint == null) {
145-
LOG.debug("Unable to fingerprint stack trace");
146-
return null;
147-
}
148140
CodeOriginProbe probe;
149141

150-
if (isAlreadyInstrumented(fingerprint)) {
151-
probe = fingerprints.get(fingerprint);
142+
if (isAlreadyInstrumented(name)) {
143+
probe = fingerprints.get(name);
152144
} else {
153145
Where where =
154146
Where.of(
@@ -162,48 +154,7 @@ public String captureCodeOrigin(
162154
where.getSignature(),
163155
where,
164156
maxUserFrames);
165-
addFingerprint(fingerprint, probe);
166-
167-
installProbe(probe);
168-
// committing here manually so that first run probe encounters decorate the span until the
169-
// instrumentation gets installed
170-
probe.commit(
171-
CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList());
172-
}
173-
174-
return probe.getId();
175-
}
176-
177-
@Override
178-
public String captureCodeOrigin(AgentSpan span, boolean entry) {
179-
String fingerprint = Fingerprinter.fingerprint(span.getResourceName());
180-
if (fingerprint == null) {
181-
LOG.debug("Unable to fingerprint stack trace");
182-
return null;
183-
}
184-
CodeOriginProbe probe;
185-
186-
if (isAlreadyInstrumented(fingerprint)) {
187-
probe = fingerprints.get(fingerprint);
188-
} else {
189-
StackTraceElement element = findPlaceInStack();
190-
ClassNode classNode = parseClassFile(element.getClassName());
191-
ClassFileLines lines = new ClassFileLines(classNode);
192-
List<MethodNode> methodsByLine = lines.getMethodsByLine(element.getLineNumber());
193-
Where where =
194-
Where.of(
195-
element.getClassName(),
196-
element.getMethodName(),
197-
"FIX ME",
198-
String.valueOf(element.getLineNumber()));
199-
200-
probe =
201-
new CodeOriginProbe(
202-
new ProbeId(UUID.randomUUID().toString(), 0),
203-
where.getSignature(),
204-
where,
205-
maxUserFrames);
206-
addFingerprint(fingerprint, probe);
157+
addFingerprint(name, probe);
207158

208159
installProbe(probe);
209160
// committing here manually so that first run probe encounters decorate the span until the

dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import datadog.trace.agent.tooling.Instrumenter.ForTypeHierarchy;
99
import datadog.trace.agent.tooling.InstrumenterModule;
1010
import datadog.trace.bootstrap.debugger.spanorigin.CodeOriginInfo;
11+
import java.lang.reflect.Field;
1112
import java.lang.reflect.Method;
1213
import net.bytebuddy.asm.Advice;
1314
import net.bytebuddy.description.type.TypeDescription;
@@ -17,7 +18,7 @@
1718
public class MethodHandlersInstrumentation extends InstrumenterModule.Tracing
1819
implements ForTypeHierarchy {
1920
private static final ElementMatcher<TypeDescription> METHOD_HANDLERS =
20-
nameEndsWith("MethodHandlers");
21+
nameEndsWith("$MethodHandlers");
2122

2223
public MethodHandlersInstrumentation() {
2324
super("grpc-server-code-origin");
@@ -44,20 +45,28 @@ public static class BuildAdvice {
4445

4546
@Advice.OnMethodEnter(suppress = Throwable.class)
4647
public static void onEnter(@Advice.Argument(0) Object serviceImpl) {
47-
Class<?> serviceClass = serviceImpl.getClass();
48-
Method[] methods = serviceClass.getSuperclass().getDeclaredMethods();
49-
for (Method method : methods) {
50-
try {
51-
Method declaredMethod =
52-
serviceClass.getDeclaredMethod(method.getName(), method.getParameterTypes());
53-
CodeOriginInfo.entry(
54-
"testing",
55-
serviceClass,
56-
declaredMethod.getName(),
57-
declaredMethod.getParameterTypes());
58-
} catch (NoSuchMethodException e) {
59-
// service method not override on the impl. skipping instrumentation
48+
try {
49+
Class<?> serviceClass = serviceImpl.getClass();
50+
Class<?> superclass = serviceClass.getSuperclass();
51+
Class<?> enclosingClass = superclass.getEnclosingClass();
52+
Field serviceNameField = enclosingClass.getDeclaredField("SERVICE_NAME");
53+
String serviceName = (String) serviceNameField.get(enclosingClass);
54+
for (Method method : superclass.getDeclaredMethods()) {
55+
try {
56+
Method declaredMethod =
57+
serviceClass.getDeclaredMethod(method.getName(), method.getParameterTypes());
58+
CodeOriginInfo.entry(
59+
String.format("%s/%s", serviceName, method.getName()),
60+
serviceClass,
61+
declaredMethod.getName(),
62+
declaredMethod.getParameterTypes());
63+
} catch (NoSuchMethodException e) {
64+
// service method not override on the impl. skipping instrumentation
65+
}
6066
}
67+
} catch (ReflectiveOperationException e) {
68+
// need to find a way to log this
69+
// LOG.debug("Member not found. Not instrumenting {}", serviceClass.getName(), e);
6170
}
6271
}
6372
}

0 commit comments

Comments
 (0)