Prevent IllegalAccessError in OpenJDK 17.0.4 by excluding javax.management.* & javax.swing.*

[ygree]

What Does This Do

Prevent IllegalAccessError in OpenJDK 17.0.4 by excluding javax.management.* & javax.swing.*

Motivation

Customer case APMS-8366

java.lang.IllegalAccessError: superinterface check failed: class javax.swing.RepaintManager$ProcessingRunnable (in module java.desktop) cannot access class datadog.trace.bootstrap.FieldBackedContextAccessor (in unnamed module @0x3ed242a4) because module java.desktop does not read unnamed module @0x3ed242a4

Additional Notes

[bantonsson]

I think these need to be 1 system level ignore for this to work.

[mcculls]

Both 1 or 2 will work in production because we treat both as ignored.

The difference is when running tests - additional ignores (2) are not ignored during testing so we can detect when an instrumentation was applied during a unit test, but won't be applied in production. This helps us find ignores that are too wide and might unknowingly disable an existing instrumentation.

Typically though we mark JDK packages as 1 and application/library packages as 2 - so this should really be 1

[mcculls]

They should also be moved up to the system section, to clarify these are JavaSE javax packages

[ygree]

I noticed that some classes in the javax.management and javax.swing packages are matched by some instrumentations thus I added them to additional ignores to be able to catch if these are too wide to ignore. Also javax.el and javax.xml are put into additional ignores. Should we also move them to system level ignores then?

[mcculls]

javax.el.* and javax.xml.* were in the original AdditionalLibraryIgnoresMatcher class - they were probably put there because they cover JavaEE packages which typically exist in additional spec jars, not JavaSE (although since then JavaSE has included content under those packages)

I don't mind whether they get moved alongside the new JavaSE javax. ignores or if they're left where they are.

Regarding instrumentations matching classes under javax.management and javax.swing packages - that's likely because we try to cover threading for context-propagation. However, not every thread or runnable is of interest and the threads under javax.management and javax.swing are unlikely to have active traces needing propagation.

[ygree]

Thanks for providing context about javax.el.* and javax.xml.*!

As far as instrumentations matching classes those mostly RunnableInstrumentation, RunnableFutureInstrumentation, and some of JavaExecutorInstrumentation, NonStandardExecutorInstrumentation, ClassloadingInstrumentation, but I agree that they are unlikely to have active traces needing propagation. So, I'll move them to system level ignores, as @bantonsson proposed.