Simplify instrumenter and tests #118493
Simplify instrumenter and tests #118493
Conversation
This commit simplifies the entitlements instrumentation service and instrumenter a bit. It especially removes some repetition in the instrumenter tests.
rjernst
added
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
| var classesToTransform = checkMethods.keySet().stream().map(MethodKey::className).collect(Collectors.toSet()); | ||
|
|
||
| inst.addTransformer(new Transformer(INSTRUMENTER_FACTORY.newInstrumenter(checkMethods), classesToTransform), true); | ||
| Instrumenter instrumenter = INSTRUMENTER_FACTORY.newInstrumenter(EntitlementChecker.class, checkMethods); |
There was a problem hiding this comment.
Is there no need to reference Java23EntitlementChecker anymore?
There was a problem hiding this comment.
Oof, looks like we already lost this. I'll fix in a followup, but I think we can still use the Class object here, we will want to do Class.forName anyways
There was a problem hiding this comment.
Note that we were already doing Class.forName inside the instrumenter. We are just making it more explicit here.
There was a problem hiding this comment.
OK, but still, I do not see a way to load Java23EntitlementChecker in the new code. And I wonder if referencing a class directly (instead by name) could be an issue here (for JDK 22, for example).
I agree, using the Class object is nicer, but does it work?
There was a problem hiding this comment.
In a followup I will change this back to using a helper method. In the case of Java 23, it will use Class.forName to get the Class object, from a string. In the default case it will use the .class as it does here. In either case, we will pass in the Class object. That's what InstrumentationService was doing internally anyways.
There was a problem hiding this comment.
Sounds like a good plan, thank you for clarifying.
ldematte
left a comment
ldematte
left a comment
There was a problem hiding this comment.
Changes looks good, thanks for the cleanup!
💚 Backport successful
|
This commit simplifies the entitlements instrumentation service and instrumenter a bit. It especially removes some repetition in the instrumenter tests.
3 participants
This commit simplifies the entitlements instrumentation service and instrumenter a bit. It especially removes some repetition in the instrumenter tests.