Fix entitlement test failures on Java 17

[mark-vieira]

Fixes a number of integration test failures when running on Java 17 as a side effect of #125226. We get some strange behvavior in our instrumentation when also doing bytecode verification since the verification requires loading referenced classes itself. This introduces awkward ordering problems which we have to explicitly account for. This fix here was to add some of our instrumented class to the list of classes we eagerly load early.

Closes #125271
Closes #125270
Closes #125269
Closes #125268
Closes #125267

[elasticsearchmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[prdoyle]

Given that this SimpleVerifier has such side effects, we might want to reconsider whether we want to use it at all here.

Perhaps it's simpler and safer to stick a checkcast in the generated bytecode and call it a day.

[mark-vieira]

Perhaps it's simpler and safer to stick a checkcast in the generated bytecode and call it a day.

I guess the concern is there may be other categories of mistakes in our instrumented bytecode that may be difficult to detect without explicit verification.

Another option is to do this validation in an explicit set of tests that aren't susceptible to these weird side effects.

[rjernst]

I wonder if these circularity errors come up if we call retransform on each class individually? Re-reading the docs it seems the point of retransform taking multiple classes is for when there are interdependencies within the transformation, but we don't have such interdependencies since our instrumentation is isolated to each method.

[mark-vieira]

I wonder if these circularity errors come up if we call retransform on each class individually? Re-reading the docs it seems the point of retransform taking multiple classes is for when there are interdependencies within the transformation, but we don't have such interdependencies since our instrumentation is isolated to each method.

I gave that a try and you still get ClassCircularityError.

[mark-vieira]

It's worth nothing that we're only enabling this verification in our entitlements integration tests (i.e. those using EntitlementsTestRule) so the maintenance burden of this list is probably minimal since these tests use the integ-test distribution and have few dependencies that might change and affect this ordering.

[ldematte]

I guess the concern is there may be other categories of mistakes in our instrumented bytecode that may be difficult to detect without explicit verification.

++

I gave that a try and you still get ClassCircularityError.

Yes, this is very likely due to when we verify (see slack thread). I also wrote down an idea on how we can "fix" this by taking a different approach, but that is a bigger change and we should discuss it first.

It's worth nothing that we're only enabling this verification in our entitlements integration tests

++
That was on purpose because I found verification at this stage does have side effects (reinforced by this issue)

[ldematte]

LGTM2

Can we backport this to 8.18 too please?
(Verification backport to 8.18 is here #125224, it was just not merged as auto-merge did not work... it fails a lot recently)

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	8.18