Skip to content

Simplify instrumenter and tests #118493

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

Merged
merged 9 commits into from
Dec 13, 2024
Merged

Simplify instrumenter and tests #118493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0a2c86e
Simplify instrumenter and tests
rjernst Dec 11, 2024
1e97836
fix oops
rjernst Dec 11, 2024
d5ed4e3
spotless
rjernst Dec 11, 2024
68ace09
fix type
rjernst Dec 12, 2024
7c973a7
Merge branch 'main' into entitlements/test_simplfiication
rjernst Dec 12, 2024
9423423
spotless
rjernst Dec 12, 2024
9195c50
Merge branch 'main' into entitlements/test_simplfiication
rjernst Dec 13, 2024
84a8ee3
fix
rjernst Dec 13, 2024
51aeb74
Merge branch 'main' into entitlements/test_simplfiication
rjernst Dec 13, 2024
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
8 changes: 3 additions & 5 deletions ...n/java/org/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,13 @@
public class InstrumentationServiceImpl implements InstrumentationService {

@Override
public Instrumenter newInstrumenter(Map<MethodKey, CheckMethod> checkMethods) {
return InstrumenterImpl.create(checkMethods);
public Instrumenter newInstrumenter(Class<?> clazz, Map<MethodKey, CheckMethod> methods) {
return InstrumenterImpl.create(clazz, methods);
}

@Override
public Map<MethodKey, CheckMethod> lookupMethodsToInstrument(String entitlementCheckerClassName) throws ClassNotFoundException,
IOException {
public Map<MethodKey, CheckMethod> lookupMethods(Class<?> checkerClass) throws IOException {
var methodsToInstrument = new HashMap<MethodKey, CheckMethod>();
var checkerClass = Class.forName(entitlementCheckerClassName);
var classFileInfo = InstrumenterImpl.getClassFileInfo(checkerClass);
ClassReader reader = new ClassReader(classFileInfo.bytecodes());
ClassVisitor visitor = new ClassVisitor(Opcodes.ASM9) {
Expand Down
28 changes: 6 additions & 22 deletions ...er/src/main/java/org/elasticsearch/entitlement/instrumentation/impl/InstrumenterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,30 +58,14 @@ public class InstrumenterImpl implements Instrumenter {
this.checkMethods = checkMethods;
}

static String getCheckerClassName() {
int javaVersion = Runtime.version().feature();
final String classNamePrefix;
if (javaVersion >= 23) {
classNamePrefix = "Java23";
} else {
classNamePrefix = "";
}
return "org/elasticsearch/entitlement/bridge/" + classNamePrefix + "EntitlementChecker";
}

public static InstrumenterImpl create(Map<MethodKey, CheckMethod> checkMethods) {
String checkerClass = getCheckerClassName();
String handleClass = checkerClass + "Handle";
String getCheckerClassMethodDescriptor = Type.getMethodDescriptor(Type.getObjectType(checkerClass));
public static InstrumenterImpl create(Class<?> checkerClass, Map<MethodKey, CheckMethod> checkMethods) {
Type checkerClassType = Type.getType(checkerClass);
String handleClass = checkerClassType.getInternalName() + "Handle";
String getCheckerClassMethodDescriptor = Type.getMethodDescriptor(checkerClassType);
return new InstrumenterImpl(handleClass, getCheckerClassMethodDescriptor, "", checkMethods);
}

public ClassFileInfo instrumentClassFile(Class<?> clazz) throws IOException {
ClassFileInfo initial = getClassFileInfo(clazz);
return new ClassFileInfo(initial.fileName(), instrumentClass(Type.getInternalName(clazz), initial.bytecodes()));
}

public static ClassFileInfo getClassFileInfo(Class<?> clazz) throws IOException {
static ClassFileInfo getClassFileInfo(Class<?> clazz) throws IOException {
String internalName = Type.getInternalName(clazz);
String fileName = "/" + internalName + ".class";
byte[] originalBytecodes;
Expand Down Expand Up @@ -306,5 +290,5 @@ protected void pushEntitlementChecker(MethodVisitor mv) {
mv.visitMethodInsn(INVOKESTATIC, handleClass, "instance", getCheckerClassMethodDescriptor, false);
}

public record ClassFileInfo(String fileName, byte[] bytecodes) {}
record ClassFileInfo(String fileName, byte[] bytecodes) {}
}
12 changes: 6 additions & 6 deletions ...a/org/elasticsearch/entitlement/instrumentation/impl/InstrumentationServiceImplTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ interface TestCheckerCtors {
void check$org_example_TestTargetClass$(Class<?> clazz, int x, String y);
}

public void testInstrumentationTargetLookup() throws IOException, ClassNotFoundException {
Map<MethodKey, CheckMethod> checkMethods = instrumentationService.lookupMethodsToInstrument(TestChecker.class.getName());
public void testInstrumentationTargetLookup() throws IOException {
Map<MethodKey, CheckMethod> checkMethods = instrumentationService.lookupMethods(TestChecker.class);

assertThat(checkMethods, aMapWithSize(3));
assertThat(
Expand Down Expand Up @@ -116,8 +116,8 @@ public void testInstrumentationTargetLookup() throws IOException, ClassNotFoundE
);
}

public void testInstrumentationTargetLookupWithOverloads() throws IOException, ClassNotFoundException {
Map<MethodKey, CheckMethod> checkMethods = instrumentationService.lookupMethodsToInstrument(TestCheckerOverloads.class.getName());
public void testInstrumentationTargetLookupWithOverloads() throws IOException {
Map<MethodKey, CheckMethod> checkMethods = instrumentationService.lookupMethods(TestCheckerOverloads.class);

assertThat(checkMethods, aMapWithSize(2));
assertThat(
Expand Down Expand Up @@ -148,8 +148,8 @@ public void testInstrumentationTargetLookupWithOverloads() throws IOException, C
);
}

public void testInstrumentationTargetLookupWithCtors() throws IOException, ClassNotFoundException {
Map<MethodKey, CheckMethod> checkMethods = instrumentationService.lookupMethodsToInstrument(TestCheckerCtors.class.getName());
public void testInstrumentationTargetLookupWithCtors() throws IOException {
Map<MethodKey, CheckMethod> checkMethods = instrumentationService.lookupMethods(TestCheckerCtors.class);

assertThat(checkMethods, aMapWithSize(2));
assertThat(
Expand Down
Loading