Skip to content

Commit 036fe91

Browse files
authored
Fix error logged for Bridge methods (#9504)
We avoid to instrument Bridge method but we don't want to log error for method not found. So introduce a skip status for method matching to differentiate if we skip instrumentation but still we have found the method
1 parent 6d4f080 commit 036fe91

File tree

3 files changed

+58
-26
lines changed

3 files changed

+58
-26
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -535,10 +535,18 @@ private boolean performInstrumentation(
535535
for (MethodNode methodNode : classNode.methods) {
536536
List<ProbeDefinition> matchingDefs = new ArrayList<>();
537537
for (ProbeDefinition definition : definitions) {
538-
if (definition.getWhere().isMethodMatching(methodNode, classFileLines)
539-
&& remainingDefinitions.contains(definition)) {
540-
matchingDefs.add(definition);
541-
remainingDefinitions.remove(definition);
538+
Where.MethodMatching methodMatching =
539+
definition.getWhere().isMethodMatching(methodNode, classFileLines);
540+
if (remainingDefinitions.contains(definition)) {
541+
if (methodMatching == Where.MethodMatching.MATCH) {
542+
// method matches, add into collection of definitions to instrument
543+
matchingDefs.add(definition);
544+
}
545+
if (methodMatching == Where.MethodMatching.MATCH
546+
|| methodMatching == Where.MethodMatching.SKIP)
547+
// match or need to skip instrumentation (because bridge) remove from remaining
548+
// definitions to avoid reporting error
549+
remainingDefinitions.remove(definition);
542550
}
543551
}
544552
if (matchingDefs.isEmpty()) {
@@ -850,7 +858,7 @@ private List<MethodNode> matchMethodDescription(
850858
List<MethodNode> result = new ArrayList<>();
851859
try {
852860
for (MethodNode methodNode : classNode.methods) {
853-
if (where.isMethodMatching(methodNode, classFileLines)) {
861+
if (where.isMethodMatching(methodNode, classFileLines) == Where.MethodMatching.MATCH) {
854862
result.add(methodNode);
855863
}
856864
}

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -127,27 +127,38 @@ public boolean isMethodNameMatching(String targetMethod) {
127127
return methodName == null || methodName.equals("*") || methodName.equals(targetMethod);
128128
}
129129

130-
public boolean isMethodMatching(MethodNode methodNode, ClassFileLines classFileLines) {
130+
public enum MethodMatching {
131+
MATCH,
132+
SKIP,
133+
FAIL
134+
}
135+
136+
public MethodMatching isMethodMatching(MethodNode methodNode, ClassFileLines classFileLines) {
131137
String targetName = methodNode.name;
132138
String targetMethodDescriptor = methodNode.desc;
133139
// try exact matching: name + FQN signature
134-
if (!isMethodNameMatching(targetName)
135-
|| ((methodNode.access & Opcodes.ACC_BRIDGE) == Opcodes.ACC_BRIDGE)) {
136-
return false;
140+
if (!isMethodNameMatching(targetName)) {
141+
return MethodMatching.FAIL;
142+
}
143+
if ((methodNode.access & Opcodes.ACC_BRIDGE) == Opcodes.ACC_BRIDGE) {
144+
// name is matching but method is a bridge method
145+
return MethodMatching.SKIP;
137146
}
138147
if (signature == null) {
139148
if (lines == null || lines.length == 0) {
140-
return true;
149+
return MethodMatching.MATCH;
141150
}
142151
// try matching by line
143152
List<MethodNode> methodsByLine = classFileLines.getMethodsByLine(lines[0].getFrom());
144153
if (methodsByLine == null || methodsByLine.isEmpty()) {
145-
return false;
154+
return MethodMatching.FAIL;
146155
}
147-
return methodsByLine.stream().anyMatch(m -> m == methodNode);
156+
return methodsByLine.stream().anyMatch(m -> m == methodNode)
157+
? MethodMatching.MATCH
158+
: MethodMatching.FAIL;
148159
}
149160
if (signature.equals("*") || signature.equals(targetMethodDescriptor)) {
150-
return true;
161+
return MethodMatching.MATCH;
151162
}
152163
// try full JVM signature: "(Ljava.lang.String;Ljava.util.Map;I)V"
153164
if (probeMethodDescriptor == null) {
@@ -160,20 +171,22 @@ public boolean isMethodMatching(MethodNode methodNode, ClassFileLines classFileL
160171
}
161172
}
162173
if (probeMethodDescriptor.isEmpty()) {
163-
return true;
174+
return MethodMatching.MATCH;
164175
}
165176
if (probeMethodDescriptor.equals(targetMethodDescriptor)) {
166-
return true;
177+
return MethodMatching.MATCH;
167178
}
168179
// fallback to signature without return type: "Ljava.lang.String;Ljava.util.Map;I"
169180
String noRetTypeDescriptor = removeReturnType(probeMethodDescriptor);
170181
targetMethodDescriptor = removeReturnType(targetMethodDescriptor);
171182
if (noRetTypeDescriptor.equals(targetMethodDescriptor)) {
172-
return true;
183+
return MethodMatching.MATCH;
173184
}
174185
// Fallback to signature without Fully Qualified Name: "LString;LMap;I"
175186
String simplifiedSignature = removeFQN(targetMethodDescriptor);
176-
return noRetTypeDescriptor.equals(simplifiedSignature);
187+
return noRetTypeDescriptor.equals(simplifiedSignature)
188+
? MethodMatching.MATCH
189+
: MethodMatching.FAIL;
177190
}
178191

179192
private static boolean isMissingReturnType(String probeMethodDescriptor) {

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/WhereTest.java

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,43 +101,54 @@ public void convertLineToMethod() {
101101
@Test
102102
public void methodMatching() {
103103
Where where = new Where("String", "substring", "(int,int)", new String[0], null);
104-
assertTrue(
104+
assertEquals(
105+
Where.MethodMatching.MATCH,
105106
where.isMethodMatching(createMethodNode("substring", "(II)Ljava/lang/String;"), null));
106107
where = new Where("String", "replaceAll", "(String,String)", new String[0], null);
107-
assertTrue(
108+
assertEquals(
109+
Where.MethodMatching.MATCH,
108110
where.isMethodMatching(
109111
createMethodNode(
110112
"replaceAll", "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"),
111113
null));
112114
where = new Where("HashMap", "<init>", "(Map)", new String[0], null);
113-
assertTrue(where.isMethodMatching(createMethodNode("<init>", "(Ljava/util/Map;)V"), null));
115+
assertEquals(
116+
Where.MethodMatching.MATCH,
117+
where.isMethodMatching(createMethodNode("<init>", "(Ljava/util/Map;)V"), null));
114118
where = new Where("ArrayList", "removeIf", "(Predicate)", new String[0], null);
115-
assertTrue(
119+
assertEquals(
120+
Where.MethodMatching.MATCH,
116121
where.isMethodMatching(
117122
createMethodNode("removeIf", "(Ljava/util/function/Predicate;)Z"), null));
118123
where = new Where("String", "concat", "", new String[0], null);
119-
assertTrue(where.isMethodMatching(createMethodNode("concat", "String (String)"), null));
124+
assertEquals(
125+
Where.MethodMatching.MATCH,
126+
where.isMethodMatching(createMethodNode("concat", "String (String)"), null));
120127
where = new Where("String", "concat", " \t", new String[0], null);
121-
assertTrue(where.isMethodMatching(createMethodNode("concat", "String (String)"), null));
128+
assertEquals(
129+
Where.MethodMatching.MATCH,
130+
where.isMethodMatching(createMethodNode("concat", "String (String)"), null));
122131
where =
123132
new Where(
124133
"Inner",
125134
"innerMethod",
126135
"(com.datadog.debugger.probe.Outer$Inner)",
127136
new String[0],
128137
null);
129-
assertTrue(
138+
assertEquals(
139+
Where.MethodMatching.MATCH,
130140
where.isMethodMatching(
131141
createMethodNode("innerMethod", "(Lcom/datadog/debugger/probe/Outer$Inner;)V"), null));
132142
where = new Where("Inner", "innerMethod", "(Outer$Inner)", new String[0], null);
133-
assertTrue(
143+
assertEquals(
144+
Where.MethodMatching.MATCH,
134145
where.isMethodMatching(
135146
createMethodNode("innerMethod", "(Lcom/datadog/debugger/probe/Outer$Inner;)V"), null));
136147
where = new Where("MyClass", "myMethod", null, new String[] {"42"}, null);
137148
ClassFileLines classFileLines = mock(ClassFileLines.class);
138149
MethodNode myMethodNode = createMethodNode("myMethod", "()V");
139150
when(classFileLines.getMethodsByLine(42)).thenReturn(Arrays.asList(myMethodNode));
140-
assertTrue(where.isMethodMatching(myMethodNode, classFileLines));
151+
assertEquals(Where.MethodMatching.MATCH, where.isMethodMatching(myMethodNode, classFileLines));
141152
}
142153

143154
private MethodNode createMethodNode(String name, String desc) {

0 commit comments

Comments
 (0)