[Discuss] Consider changing how classloading works for extensible and extending plugins

[cwperks]

tldr;

Plugins should control their deps and not have to wonder about whether plugins they extend provide a dep at runtime

Longer version:

ExtensiblePlugin is an interesting sort of extension point for OpenSearch. It allows us to define additional ways of extending core functionality, but writing another extension point outside of the core. Two such common use-cases are lang-painless and the opensearch-job-scheduler. With ExtensiblePlugins, plugins define relationships to one another.

For instance, the reporting plugin extends the opensearch-job-scheduler in order to get job scheduling functionality to run reports on a schedule.

There is one not so obvious thing about these plugins relations and that is how their classloaders relate.

When the reporting plugin extends the job scheduler, a side-effect is that job-scheduler's dependencies are available to the reporting plugin at runtime. However, the same dependencies are not available to reporting at compile time.

As a result, you find extending plugins setting compileOnly declarations on dependencies that they share with their extended plugin.

This causes some headache to plugin developers when extending other plugins because they are at the whim of the extensible plugin deciding to add dependencies that they also rely upon.

I'm opening this issue to discuss if we can change how classloading works for extensible and extending plugins.

In particular, I'd like to explore removing extendedLoaders from here: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/plugins/PluginsService.java#L789

Related component

No response

To Reproduce

Prevent issues such as opensearch-project/ml-commons#4202

Expected behavior

No jar hell if extensible and extending plugin have the same implementation level dep.

Additional Details

Extensible plugins have been being used much more than from the earlier days of OpenSearch.

[dbwiddis]

As a result, you find extending plugins setting compileOnly declarations on dependencies that they share with their extended plugin.

This causes some headache to plugin developers when extending other plugins because they are at the whim of the extensible plugin deciding to add dependencies that they also rely upon.

Example: opensearch-project/ml-commons#3860

[andrross]

In particular, I'd like to explore removing extendedLoaders from here: main/server/src/main/java/org/opensearch/plugins/PluginsService.java#L789

You'll need to explain like I'm five what this will do :)

I agree that transitive dependencies being available at runtime but not at compile time is weird and not how dependency resolution usually works. Do you know what it was done that way in the first place? How would your suggestion change this behavior?

[cwperks]

With the current setup, the ExtensiblePlugin provides a SPI (Service Provider Interface) which is a minimal library that contains the interface to be implemented by the extending plugin.

For extending plugins, they must add a compileOnly dependency on this SPI. (See below reference in reporting plugin)

i.e.

compileOnly "org.opensearch:opensearch-job-scheduler-spi:3.3.0.0-SNAPSHOT"

In this way, the classes from the JobScheduler SPI are available at compile time so that you can import the classes and compile.

At runtime, Job-Scheduler brings this jar with it and its available in plugins/opensearch-job-scheduler/ along with all of the other job scheduler dependencies.

All of those jars in plugins/opensearch-job-scheduler/ are available to the reporting plugin at runtime. The way that works is because the parent classloader of reporting plugin is the classloader of job-scheduler plugin because of that extensions loading logic linked above. As an effect, all of job-schedulers dependencies are available to the reporting plugin at runtime.

One such recent example that comes to mind is that Job-Scheduler had a guava dependency and extending plugins needed to define their guava dependencies as compileOnly, relying on the fact that it would be provided at runtime by the job scheduler.

Looks like that issue linked to another issue in the core with more info on the problem.

Its just not obvious when extending another plugin that their dependencies will be available at runtime. I think plugins should be better isolated and declare all of the dependencies they need directly in their build.gradle file without the coupling.

tbh, I haven't fully specced out what would be the ramifications of changing classloading. It has caused many problems so I'd like to better understanding why the classloading logic is the way it is or if its something that could/should be changed.

[cwperks]

Need to confirm, but GPT-5 says this:

• Single-definition rule for SPI types. Your extender must implement/instantiate interfaces and classes that live in the extensible plugin (Job Scheduler’s SPI, etc.). If those SPI classes were loaded from a different loader than the one the host plugin uses, you’d hit ClassCastException: X loaded by L1 cannot be cast to X loaded by L2. The clean way to avoid that is to make the extensible plugin’s classloader a parent (or otherwise in the delegation path) of the extending plugin’s loader. Then there’s one canonical definition of the SPI types.

So the interface to be extended (i.e. JobSchedulerExtension) needs to be loaded from the same place. Makes sense, but it does necessarily mean that the other dependencies are available at runtime.

[cwperks]

hmm, one idea could be to make some of this SPI libs into mini plugins instead. i.e. convert https://github.com/opensearch-project/job-scheduler/tree/main/spi to a plugin that has a zip and installed like other plugins, but simply is one jar with the code in the SPI.

Then both JobScheduler and its extensions can declare extendedPlugins = ['opensearch-job-scheduler-spi'] and have the same instance of the SPI classes, but not the other JobScheduler dependencies.

[cwperks]

2 thoughts that come to mind:

• Can we following similar model to src/main/bin and src/main/config where these are copied during install to a plugin specific directory under bin/ and config/, but additionally extend this to lib/
• If the 2 plugin approach is taken (slim plugin for SPI) and main plugin then can we also install both plugins during ./bin/opensearch-plugin install... so that 2 separate commands don't need to be run.

[DarshitChanpura]

one way to get around this problem, that I've seen in plugins is usage of repackaging via shadow jars to avoid jarHells during runtime.

[cwperks]

I like the idea of the micro plugin solution for the SPI. With the micro plugin solution, both the extensible and the extending plugin will declare a relationship to the SPI in their respective build.gradle files.

i.e. extendedPlugins = ['opensearch-job-scheduler-spi']

With that approach, both job-scheduler and extending plugins like the reporting plugin will have access to the extension point (i.e. JobSchedulerExtension), but otherwise keep their dependencies completely separate.

What would be nice, is to automatically install the JS SPI micro plugin when JS is installed to prevent having to use ./bin/opensearch-plugin install ... twice.

---

The other solution could be to install libs under the libs/ folder at installation time similar to how config and bin work.

Plugins that have files in src/main/config get these files copied into config/<plugin_name> at installation time.

i.e. when installation notifications you end up with a folder config/opensearch-notifications/ that has a notifications.yml file.

I'm wondering if we can similarly install the spi to libs/opensearch-job-scheduler/ at installation time and if this would be available to all plugins. Would need to make sure its removed when a plugin is uninstalled as well.