[Feature/multi_tenancy] Add SearchDataObject interface, Client, and Connector Implementations

[dbwiddis]

Description

• Adds search method to SdkClient interface
• Implements search method on Local and Remote clients
• Migrates Connector Search action to use SdkClient for the searching
• Updates Delete and Update connector actions which search model index to use new API

Issues Resolved

Continuation of PR #2459

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[dhrubo-os]

How this will work? After searching in multiple indices are we going to combine the results and then send it back through SearchDataObjectResponse?

[dbwiddis]

In practice in ML commons, there's a single index (the system index for connectors, models, etc.).

In general it can be an empty list (search all indices) or multiple indices. The search request really just packages up the entire JSON of the search query and the full response as well, parsing it back out from the XContent.

So the search response will be identical (some metadata/stats and then a list of hits).

[dhrubo-os]

I think we should also add tenant id in the sourceBuilder?

[dbwiddis]

Yes, have you already added it to all the others? I lost track with the multiple PRs at the same time.

[dhrubo-os]

No, I haven't made any changes with search functionalities yet.

[dbwiddis]

I think @arjunkumargiri is adding tenant id in https://github.com/opensearch-project/ml-commons/pull/2553/files

It's not currently in of the other requests; probably needs to be its own standalone field.

[dhrubo-os]

yeah, I'll address that accordingly after that PR is being merged.

[dhrubo-os]

May be we should have a recursive method to identify the root cause as we know the exception can be wrapped with other exception. What do you think about this method:

private Throwable getRootCause(Throwable throwable) {
    Throwable cause = throwable;
    while (cause.getCause() != null && cause != cause.getCause()) {
        cause = cause.getCause();
    }
    return cause;
}

[dbwiddis]

See unwrapCause() here. This seems to do what you're asking for. https://github.com/opensearch-project/OpenSearch/blob/802f2e6e4b21f27ddc6c01e7fc6f6cdcd69138d3/libs/core/src/main/java/org/opensearch/ExceptionsHelper.java#L127-L145

[dhrubo-os]

Yes! Should we refactor the code accordingly then?

[dbwiddis]

Possibly? But possibly not. I intentionally only unwrapped one level. Tracing the code, we know we're using actionGet so looking at the code we either throw the original exception (if it's a RTE) or it's a wrapped UncategorizedExecutionException containing the cause, which is what we're extracting here. So that built in recursive method is a bit too deep.

While a util method is helpful here, a ternary one-liner is also pretty brief so I'm struggling to see the huge benefit of a refactor.

So I'm open to it, but not thinking it's a high priority.