snehashisp commented on PR #17741: URL: https://github.com/apache/kafka/pull/17741#issuecomment-2533473455
Hi @gharris1727. Please take another pass. I have made the requested changes, lmk if I have missed anything. Some tests are failing, will look into those soon. There is another unrelated issue I came across while testing this PR. There might be a regression in #16604, which seems to break reflections based isolation for plugins that are already defined in the classpath. Looks like the new classgraph scanner is always loading the classpath plugins even if there is a another version in an isolated plugin.path. This is causing the runtime to ignore the plugin due to [this](https://github.com/apache/kafka/blob/520681c38dbefe497181c4fd5dfc793d54233408/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/ReflectionScanner.java#L134) check. In my setup, I have built the following plugin hierarchy for testing, and I found that the different versions of json converter are never identified due to the one being present in the classpath. ``` plugins3 ├── connect-file-1.2.1-T-0.9.0-P-3.1 │ └── lib │ └── connect-file-1.2.1-T-0.9.0-P-3.1.jar ├── connect-file-2.2.1-T-1.0.0-P-3.1-extra │ └── lib │ ├── connect-file-2.2.1-T-1.0.0-P-3.1-extra.jar │ └── connect-json-11.jar ├── connect-file-2.2.1-T-1.1.0-P-3.1 │ └── lib │ └── connect-file-2.2.1-T-1.1.0-P-3.1.jar ├── connect-file-2.2.11-T-2.1.0-P-3.5 │ └── lib │ └── connect-file-2.2.11-T-2.1.0-P-3.5.jar ├── json-11 │ └── lib │ └── connect-json-11.jar └── json-17 └── libs └── connect-json-17.jar ``` I changed the debug logs to error in the class and observed the following errors during scanning. ``` [2024-12-11 07:29:28,968] ERROR class org.apache.kafka.connect.json.JsonConverter from other classloader jdk.internal.loader.ClassLoaders$AppClassLoader@c387f44 is visible from C:\Users\user\Desktop\confluent\testing\plugins3\connect-file-1.2.1-T-0.9.0-P-3.1, excluding to prevent isolated loading (org.apache.kafka.connect.runtime.isolation.ReflectionScanner:135) [2024-12-11 07:29:28,969] ERROR class org.apache.kafka.connect.json.JsonConverter from other classloader jdk.internal.loader.ClassLoaders$AppClassLoader@c387f44 is visible from C:\Users\user\Desktop\confluent\testing\plugins3\connect-file-1.2.1-T-0.9.0-P-3.1, excluding to prevent isolated loading (org.apache.kafka.connect.runtime.isolation.ReflectionScanner:135) ``` The fix I made was to use `overrideClassLoaders(source.loaders()` instead of `addClassLoader` [here](https://github.com/apache/kafka/blob/520681c38dbefe497181c4fd5dfc793d54233408/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/ReflectionScanner.java#L80). Based on this code, `addClassLoader` by default includes the jdk classloaders first and only then scans the provided loader. I think overriding the loader here is the correct approach, but have not dug too deep here. This is probably not the correct place but wanted to bring it to your attention and see if I am missing something. LMK if I should create a bug ticket for this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
