[FEATURE] Create abstract Base classes for Extension and ExtensionRestHandler

[dbwiddis]

Is your feature request related to a problem?

Extension developers need a stable API (interface) to implement extensions.

SDK developers need a way to add additional functionality without forcing extension developers to change their code.

What solution would you like?

I propose to create a 3-layer inheritance for Extension and ExtensionRestHandler implementations.

• The Extension interface will include all the methods required by ExtensionsRunner including
  • settings
  • rest handlers
  • settings (to be added)
  • components (to be added)
• A new BaseExtension abstract class will provide default methods so users don't need to implement everything in the interface but have the ability to override
  • can provide an empty settings list so extension authors with no custom settings don't need to do anything
  • can be used for default components but allow for the extension implementation to override
  • can move the read settings from yaml convenience code out of interface and simplify the awkward user implementation with all the exception handling
• Similar layering can be done for the ExtensionRestHandler with a BaseExtensionRestHandler
  • This will create a place for an abstract class to provide optional syntax for handling REST requests as envisioned by [FEATURE] Provide a higher level implementation for REST handlers  #128

Also we should create a new ExtensionRestRequest class to be the sole argument to ExtensionRestHandler.handleRequest().

• This will keep the API stable as we add new functionality such as params ([FEATURE] Pass REST params to extensions #111) and security principal information
• Goal should be to at least match RestRequest method names exactly to ease porting extension code

What alternatives have you considered?

An interface-only model requires using default methods and exposes a lot of implementation details that aren't really needed.

An abstract-class-only model would work but from an "API" or "SDK" perspective I prefer the cleanliness of an interface.

Do you have any additional context?

@owaiskazi19 please provide some input on what the API method for creating components should look like (what arguments, etc.)
@cwperks this relates to this comment on your draft PR.
@mloufra the create components piece may touch your code after refactoring ExtensionsRunner in #116 so will try to coordinate my implementation with yours in #116. If you can let me know which branch on your local fork you're pushing code to it will be helpful.

[mloufra]

@mloufra the create components piece may touch your code after refactoring ExtensionsRunner in #116 so will try to coordinate my implementation with yours in #116. If you can let me know which branch on your local fork you're pushing code to it will be helpful.

This is my branch which I am pushing code for #116 .

[ryanbogan]

Design for components section of this issue, as discussed with @dbwiddis:

• Extension interface: get method for each param in createComponents()
• BaseExtension abstract class:
  • Private instance variables for each param
  • Public get methods for each instance variable
  • Protected set methods for each instance variable
  • Contructor calls private createComponents() method that sets defaults for each instance variable
• HelloWorldExtension will extend BaseExtension

[dbwiddis]

Design for components section of this issue, as discussed with @dbwiddis

A good example of this pattern in the OpenSearch codebase is the RestHandler interface which has several methods, many of them with default implementations. The BaseRestHandler implements others, and adds a few abstract methods.

plugin "action" to handle REST request which extendsBaseRestHandler needs only to implement a few methods (getName(), routes(), and prepareRequest()). If the implementing class wants to override things in the superclass it is easy to do so.

[owaiskazi19]

There should be a similar method like getExtensionSettings named as createComponent the only difference would be to create initial objects of anomalyDetectionIndices, searchFeatureDao, memoryTracker, adTaskManager and the remaining objects present here. These objects will help us in the create detector workflow
Also, these objects or the params should be the APIs @joshpalis created in his PR. We should avoid using it from OpenSearch directly.

[dbwiddis]

There should be a similar method like getExtensionSettings named as createComponent

I'm not sure that is the best approach here.

In the plugin ecosystem, OpenSearch initializes the plugins (from Node) and passes all these parameters (e.g., client is the NodeClient, the cluster service has been initialized, environment is set, etc. This method is primarily a means to let the Extension know a bunch of OpenSearch settings.

We are instantiating extensions from a different Node instance and many of these don't apply; those that are relevant should be present in the ExtensionsRunner (such as the SDKClient). This method would only be called by the ExtensionsRunner so it makes little sense for the runner to ask the extension for what it already has.

[dbwiddis]

Continuing the above example. The fundamental difference is that in the Plugin case, every plugin needs access to the same Java object(s) (e.g., the NodeClient). However, in the Extensions case there is a 1 to 1 correspondence between an Extension and its ExtensionsRunner instance.

So to get an SDKClient in an extension, we have a few options:

• We could force the extension to declare a local client variable (similar to how Plugins work) and define a createComponents() method to save it
  • Not horrible for one component but a lot of boilerplate code for things that may never be used.
• The Extension is already instantiating the ExtensionsRunner in its main() method. It can easily save that instance and query any of the runner's methods.... a getClient() method could return the SDKClient, for example.
  • A reasonable approach but still requires the extension to do Client client = runner.getClient();.
• We could create an ExtensionsComponents object that has a createComponents method built in that the ExtensionsRunner passes to the Extension.
  • So the extension could just call components.getClient() when it needed one. Not much different than the previous approach of saving the ExtensionsRunner, but at least groups things together.
• Create an abstract base class that an Extension extends, that already has the objects within it. An extension that inherits from it instantly has access to client if it's declared `protected'.
  • No work at all for the Extension. It's just there. And the extension can overwrite it. All we need to do is have an internal createComponents object in the Base Extension class that populates these variables.
    • We could require createComponents at the interface level and then implement it in the base class, so the end user Extension still would have no work to do. This seems reasonable, but it also seems unnecessary as all of these things happen at startup and the method can be called from the constructor.

I obviously favor the last one, but open to discussion on this.

[dbwiddis]

An option to the proposed approach (private instance, public getters, protected setters) would just be to have protected instance variables.

[dbwiddis]

Hey @ryanbogan ... after reviewing @owaiskazi19 comments here and elsewhere I think we're all in agreement on the following approach. Yes this is the opposite of what I've said before but I've come to see the light. :-)

In SDK:

1. Add the createComponents() method (as in Plugin class) to the Extension interface. The ExtensionsRunner will use this to send information to the extension.
2. Where you currently have a private method in the BaseExtension, make it public, add @Override annotation and just save the arguments passed to it from ExtensionsRunner as instance variables.
3. Just change the instance variables to protected and remove the getters and setters. But add javadocs for each one. Then an extension can just use client.send(...) directly without having to do any other work.
4. In ExtensionsRunner initialization (same method where getExtensionRestHandlers() and getSettings() are called) call the extension's createComponents with the list of components you currently have....
   • Except use specific components from ExtensionsRunner, not from OpenSearch itself. SDKClient, for example. The Settings retrieved from Opensearch. And so on.

In Extension:

5. Add any additional classes you need, so in the AD extension (which extends BaseExtension) you will need the components listed in this comment

[owaiskazi19]

Hey @ryanbogan ... after reviewing @owaiskazi19 comments here and elsewhere I think we're all in agreement on the following approach. Yes this is the opposite of what I've said before but I've come to see the light. :-)

Thanks @dbwiddis for outlining the clear steps for createComponent interface.

[owaiskazi19]

@ryanbogan the 5th point in above comment can be address in #160.