Skip to content

Commit

Permalink
Fix Where signature (#7735)
Browse files Browse the repository at this point in the history
in Where::convertLineToMethod method we are converting a Where with a
line to a new Where as a method + signature matching the line
However the signature was the JVM descriptor and therefore does not
match with the way we are using it for matching instrumentation
that expect a java signature
  • Loading branch information
jpbempel authored Oct 4, 2024
1 parent 78e523d commit fd84e8c
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public String createProbeForFrame(String signature) {
AgentSpan span = AgentTracer.activeSpan();
if (!isAlreadyInstrumented(fingerprint)) {
Where where =
Where.convertLineToMethod(
Where.of(
element.getClassName(),
element.getMethodName(),
signature,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public boolean createProbesForException(StackTraceElement[] stackTraceElements)
continue;
}
Where where =
Where.convertLineToMethod(
Where.of(
stackTraceElement.getClassName(),
stackTraceElement.getMethodName(),
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import datadog.trace.bootstrap.debugger.ProbeId;
import datadog.trace.bootstrap.debugger.el.ReflectiveFieldValueResolver;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -34,6 +35,7 @@
import java.util.Map;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.stream.Collectors;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;

Expand Down Expand Up @@ -339,6 +341,13 @@ public static String descriptorFromSignature(String javaSignature) {
return buf.toString();
}

public static String descriptorToSignature(String desc) {
Type[] argumentTypes = Type.getArgumentTypes(desc);
return Arrays.stream(argumentTypes)
.map(Type::getClassName)
.collect(Collectors.joining(", ", "(", ")"));
}

/**
* Turn a type specified in Java syntax (eg. {@literal int}, {@literal java.lang.String} or
* {@literal java.lang.String[]}) into the corresponding JVM type descriptor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ public class Where {
this(typeName, methodName, signature, sourceLines(lines), sourceFile);
}

public static Where convertLineToMethod(
String typeName, String methodName, String signature, String... lines) {
public static Where of(String typeName, String methodName, String signature, String... lines) {
return new Where(typeName, methodName, signature, lines, null);
}

Expand All @@ -65,7 +64,8 @@ public static Where convertLineToMethod(Where lineWhere, ClassFileLines classFil
if (methodsByLine != null && !methodsByLine.isEmpty()) {
// pick the first method, as we can have multiple methods (lambdas) on the same line
MethodNode method = methodsByLine.get(0);
return new Where(lineWhere.typeName, method.name, method.desc, (SourceLine[]) null, null);
String javaSignature = Types.descriptorToSignature(method.desc);
return new Where(lineWhere.typeName, method.name, javaSignature, (SourceLine[]) null, null);
}
}
throw new IllegalArgumentException("Invalid where to convert from line to method " + lineWhere);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2359,7 +2359,7 @@ public void lineRecord() throws IOException, URISyntaxException {
public void allProbesSameMethod() throws IOException, URISyntaxException {
final String CLASS_NAME = "CapturedSnapshot01";
final String METRIC_NAME = "count";
Where where = Where.convertLineToMethod(CLASS_NAME, "main", null);
Where where = Where.of(CLASS_NAME, "main", null);
Configuration configuration =
Configuration.builder()
.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ public void testProbeInstallation() throws IOException, URISyntaxException {
public void testWithCodeOrigin() throws IOException, URISyntaxException {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20";

Where where =
Where.convertLineToMethod(CLASS_NAME, "process", "int (java.lang.String)", (String[]) null);
Where where = Where.of(CLASS_NAME, "process", "int (java.lang.String)", (String[]) null);
installProbes(
DebuggerProbe.builder()
.probeId(DEBUG_PROBE_ID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ <T extends ProbeDefinition> T createMock(
.when(mock)
.instrument(any(), anyList(), anyList());
when(mock.getProbeId()).thenReturn(new ProbeId(id, 0));
Where where = Where.convertLineToMethod(ArrayList.class.getName(), "add", "(Object)");
Where where = Where.of(ArrayList.class.getName(), "add", "(Object)");
when(mock.getWhere()).thenReturn(where);
return (T) mock;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,15 @@ void testDescriptorFromSignatureInvalid() {
IllegalArgumentException.class, () -> Types.descriptorFromSignature("int (a, ,b)"));
}

@Test
void descriptorToSignature() {
String desc = "(I[JLjava/util/Map;Ljava/util/Map$Entry;[[Ljava/lang/String;)Ljava/lang/String;";
String expectedSignature =
"(int, long[], java.util.Map, java.util.Map$Entry, java.lang.String[][])";
String signature = Types.descriptorToSignature(desc);
assertEquals(expectedSignature, signature);
}

@ParameterizedTest
@MethodSource("provideGetArrayType")
void testGetArrayType(Class<?> clazz, int opcode) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.datadog.debugger.probe;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -9,6 +11,7 @@
import com.squareup.moshi.JsonAdapter;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.objectweb.asm.tree.MethodNode;
Expand Down Expand Up @@ -82,6 +85,19 @@ public void linesRoundTrip() throws IOException {
Assertions.assertEquals(linesJson, adapter.toJson(lines));
}

@Test
public void convertLineToMethod() {
Where wherePut = Where.of("java.util.Map", "put", "(Object, Object)", "42");
ClassFileLines classFileLines = mock(ClassFileLines.class);
MethodNode methodNode = createMethodNode("put", "(Ljava/lang/Object;Ljava/lang/Object;)V");
when(classFileLines.getMethodsByLine(42)).thenReturn(Collections.singletonList(methodNode));
Where whereMapPut = Where.convertLineToMethod(wherePut, classFileLines);
assertEquals("java.util.Map", whereMapPut.getTypeName());
assertEquals("put", whereMapPut.getMethodName());
assertEquals("(java.lang.Object, java.lang.Object)", whereMapPut.getSignature());
assertNull(whereMapPut.getLines());
}

@Test
public void methodMatching() {
Where where = new Where("String", "substring", "(int,int)", new String[0], null);
Expand Down

0 comments on commit fd84e8c

Please sign in to comment.