Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Design/Discuss what goes into extensions.yml #65

Closed
saratvemulapalli opened this issue Jul 28, 2022 · 7 comments · Fixed by opensearch-project/OpenSearch#6003 or #365
Closed
Assignees
Labels
discuss enhancement New feature or request

Comments

@saratvemulapalli
Copy link
Member

Is your feature request related to a problem?

Opening this as a follow up of #60 (comment)

The code reads all of this configuration today. We should get rid of the unnecessary parts of it which were inherited from plugins.

extensions:
  - name: opensearch-sdk
    uniqueId: opensearch-sdk-1
    hostName: 'sdk_host'
    hostAddress: '127.0.0.1'
    port: '4532'
    version: '1.0'
    description: Extension for the Opensearch SDK Repo
    opensearchVersion: '3.0.0'
    javaVersion: '14'
    className: ExtensionsRunner
    customFolderName: opensearch-sdk
    hasNativeController: false	
@peternied
Copy link
Member

I'd prefer this configuration be minimal with the heavy lifting done by the extension registration:

extensions:
  - name: Hello World
    description: Adds a header to all responses from the service hello:world
    registrationUrl: 'https://127.0.0.1:4532/hello/register'

During the extension registration needed data such as the minimum supported version(s) of OpenSearch, what extension points should be routed to this extension, extension versioning information, etc.

@dbwiddis
Copy link
Member

name: opensearch-sdk

It's good to have a name, but we should be aware there will probably be name collisions and not rely on this other than for friendly display/logging.

uniqueId: opensearch-sdk-1

We do need a unique extension ID. We should document how to generate these. Will there be a registry somewhere where people can request a UUID from it?

hostName: 'sdk_host'
hostAddress: '127.0.0.1'

Not sure we need both hostName and hostAddress. What purpose does it serve having both? If we have a hostName we can lookup the address. Do we want to allow both but specify one as primary? Or just have a hostName field in which an IP address is acceptable?

port: '4532'

I do like separating the host and port in the config, so we don't need to deal with parsing them out of a URI later.

version: '1.0'

We shouldn't specify the version on the OS side, the extension knows its own version and should report that as part of the handshake.

description: Extension for the Opensearch SDK Repo

Sure.

opensearchVersion: '3.0.0'

Doesn't OpenSearch already know its version and can send it as part of the handshake?

javaVersion: '14'

Not needed to know what version of Java (or any other language) is on the other end.

className: ExtensionsRunner

Not needed. Should be a standard className.

customFolderName: opensearch-sdk

Where is this used? Is it part of the REST configuration? I'm thinking this or something needs to be here as part of #64.

hasNativeController: false

Don't even know what this does.

We need to add some other configurations such as:

  • reconnectOnFailure (auto/manual): if the extension becomes unreachable should we keep pinging it to see if it's there or gracefully fail to no-extension state?
  • priority: should requests from this extension be prioritized over other extensions
  • secure: should API connections from this extension be encrypted?

@peternied
Copy link
Member

priority: should requests from this extension be prioritized over other extensions

Are the calls going to be processed in series? For your consideration 5x extensions with 200ms roundtrip each time means 1 sec for response time on the OpenSearch request, installing extensions linearly impacts your OpenSearch clusters performance, seems like a scaling bottleneck.

@minalsha minalsha added discuss and removed untriaged labels Aug 2, 2022
@dblock
Copy link
Member

dblock commented Aug 10, 2022

uniqueId: opensearch-sdk-1

We do need a unique extension ID. We should document how to generate these. Will there be a registry somewhere where people can request a UUID from it?

Note that UUIDs don't have to come from a registry, they can be generated client-side (e.g. with uuidgen). The public catalog can serve as a registry for looking up information based on a uuid.

@dbwiddis
Copy link
Member

Note that UUIDs don't have to come from a registry, they can be generated client-side (e.g. with uuidgen). The public catalog can serve as a registry for looking up information based on a uuid.

In #95 I have proposed using reverse DNS names for these. Seems to work in the Java ecosystem for unique artifacts from the central repository, (most) package names (unenforced), JPMS module names (enforced), etc.

@minalsha
Copy link
Collaborator

@saratvemulapalli, @dbwiddis what is pending to finalize the decision on this issue and the next steps?

@saratvemulapalli
Copy link
Member Author

saratvemulapalli commented Jan 25, 2023

As we have inherited lots of properties from plugins, I'll take this on as part of #346 and clean it up.
Here is the proposed version of the config file.

extensions:
  - name: opensearch-sdk
    uniqueId: opensearch-sdk-1
    hostName: 'sdk_host'
    hostAddress: '127.0.0.1'
    port: '4532'
    version: '1.0'
    description: Extension for the Opensearch SDK Repo
    opensearchVersion: '3.0.0'	

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment