[Feature/extensions] Implement createDetector request in AD Extension

[ryanbogan]

Create a Transport API for creating Index for AD extension

• Initial discussion of the deliverables
• Figure out if an API already exist in clients/OpenSearch
• Create the API if not
• Register custom POST route for create detector
• Integrate the createIndex API with AD plugin to test if SDKClient is working as expected
• Integrate the createDetector workflow

[owaiskazi19]

As we are looking into creating a SDKClient and calling the REST APIs associated with it to create the AD index
Listing out the occurrences of client for creating a detector:

1. https://github.com/opensearch-project/anomaly-detection/blob/main/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java#L95
2. https://github.com/opensearch-project/anomaly-detection/blob/main/src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java#L190
3. https://github.com/opensearch-project/anomaly-detection/blob/main/src/main/java/org/opensearch/ad/rest/handler/AbstractAnomalyDetectorActionHandler.java#L279
4. https://github.com/opensearch-project/anomaly-detection/blob/main/src/main/java/org/opensearch/ad/indices/AnomalyDetectionIndices.java#L538

[dbwiddis]

1 and 3 will be handled as part of #22 (or are already handled in #23).

2 looks like a dependency on the "user" stored with the detector which in our case will already be "null" so probably requires no action but might need some documentation.

4 is the AdminClient that I think we get for free with the AbstractClient inheritance.

[dbwiddis]

Opened up #63 to track item 2 above. The rest should be no problem.

[owaiskazi19]

Keeping this on hold until #24 is finished.

[owaiskazi19]

Hi @dbwiddis! To replicate prepareRequest for creating a detector. This issue requires seqNo, primaryTerm, refreshPolicy, detectorId which in turns comes from paramAsLong, hasParam, param and contentParser. I see we have issue already for the same #111.

[dbwiddis]

]This issue requires seqNo, primaryTerm, refreshPolicy, detectorId which in turns comes from paramAsLong, hasParam, param and contentParser. I see we have issue already for the same #111.

#111 (and my PR #163) has implemented the param parts, but content parser is a bigger issue. It's the XContent parser in the existing RestRequest class and the parser itself is a functional interface which is ugly to send over transport.

I see #91 in draft but I don't think that directly solves the problem anyway.

One possible solution is to entirely recreate the (JSON, at least) XContent parser on the SDK side of things. We can probably get smart by inheriting classes from OpenSearch but these should be instantiated on the SDK side so they are not passed as part of a RestRequest.

I think this should be part of @ryanbogan CreateComponents piece, or at least a follow-on to that piece.

[joshpalis]

One possible solution is to entirely recreate the (JSON, at least) XContent parser on the SDK side of things. We can probably get smart by inheriting classes from OpenSearch but these should be instantiated on the SDK side so they are not passed as part of a RestRequest.

This solution would require a few things. Digging into RestRequest::contentParser() here, in order to generate this Xcontent parser on the SDK side, we would first need to transport the consolidated namedXcontentRegistry from Node.java to the SDK. Or at least the namedXcontents from all the required modules so that we could also include the AnomalyDetection namedXcontent entries and generate the NamedXContentRegistry on the extension, since the underlying data structure within the registry is an unmodifiable map.

[dbwiddis]

n order to generate this Xcontent parser on the SDK side, we would first need to transport the consolidated namedXcontentRegistry ...

This is what I'm saying is ugly.

I'm suggesting we generate the whole registry fresh on the SDK side.

There are a fixed number of registries, see here:
https://github.com/opensearch-project/OpenSearch/tree/main/libs/x-content/src/main/java/org/opensearch/common/xcontent

The registry is initialized in Node.java by iterating known getters:

NamedXContentRegistry xContentRegistry = new NamedXContentRegistry(
    Stream.of(
        NetworkModule.getNamedXContents().stream(),
        IndicesModule.getNamedXContents().stream(),
        searchModule.getNamedXContents().stream(),
        pluginsService.filterPlugins(Plugin.class).stream().flatMap(p -> p.getNamedXContent().stream()),
        ClusterModule.getNamedXWriteables().stream()
    ).flatMap(Function.identity()).collect(toList())
);

The ones starting from uppercase class names (Network, Indices, Cluster) are static and can be called from the SDK side exactly as they are here.

[dbwiddis]

OK, I'm overcomplicating this with the registries. Let's get back to the parsers. I think I can completely handle this in #163.

The REST request will have an enum XContentType with only 4 possible values: JSON, SMILE, YAML, and CBOR.

This enum can just be sent over transport, and then the content parser can be instantiated on the receiving side.

So there is no crisis, I just need to add the params, content type enum, and content, and handle it.

You may now return to your regular Friday Eve programming.

[dbwiddis]

Closing the loop here: #163 will send the XContentType to the extension. Because the ExtensionRestRequest class is in OpenSearch side of the API, it doesn't make sense to put the contentParser() getter on it. So I'll go ahead and put #163 back in review.

Part of integrating things (#132) will be finding a new place (probably the BaseExtension) to create the parser where it will have access to the extension's XContentRegistry.

[dbwiddis]

#163 is now merged, so in an extension when processing an ExtensionRestRequest we will have a getXContentType() getter, returning either null, or one of the XContentType enums. The equivalent of contentParser() is:

// must null-check request.getXContentType()
request.getXContentType().xContent().createParser(xContentRegistry, LoggingDeprecationHandler.INSTANCE, content.streamInput())

To create the xContentRegistry parameter in the above, note the AD Plugin has a getNamedXContent() method already: https://github.com/opensearch-project/anomaly-detection/blob/08fdbdde4c2f97c27f63d3a4ba31f8acd6dbf570/src/main/java/org/opensearch/ad/AnomalyDetectorPlugin.java#L938-L946

Add that to an empty registry as follows

NamedXContentRegistry xContentRegistry() {
    SearchModule searchModule = new SearchModule(Settings.EMPTY, Collections.emptyList());
    List<NamedXContentRegistry.Entry> entries = searchModule.getNamedXContents();
    entries.addAll(getNamedXContent());
    return new NamedXContentRegistry(entries);
}

Creating the registry like the above will probably end up being a function of createComponents() as part of issue #160 with the ExtensionsRunner class reading the getNamedXContent() from the extension (default empty list). cc: @owaiskazi19 @joshpalis @ryanbogan

[owaiskazi19]

The current workflow of AD for creating a detector is:

1. Create a metedata index named .opendistro-anomaly-detectors with mapping associated with body of the request.
2. Add values for the keys present in the request body

Since we are using SDKClient which is based on OpenSearch java client, it requires mapping in the form of TypeMapping. To convert the json body of the request to TypeMapping, we are currently exploring the below approaches:

1. (Hacky way) Create properties objects for the json and parse it as TypeMapping - @owaiskazi19
2. Figure out a way for creating TypeMapping using XContentParser - @dbwiddis

[owaiskazi19]

Created AD Index with mapping using JsonParser and JsonPMapper provided by java client

JsonpMapper mapper = sdkClient._transport().jsonpMapper();
        JsonParser parser = null;
        try {
            parser = mapper
                .jsonProvider()
                .createParser(new ByteArrayInputStream(getAnomalyDetectorMappings().getBytes(StandardCharsets.UTF_8)));
        } catch (Exception e) {
            e.printStackTrace();
        }

        CreateIndexRequest request = null;
        try {
            request = new CreateIndexRequest.Builder()
                .index(AnomalyDetector.ANOMALY_DETECTORS_INDEX)
                .mappings(TypeMapping._DESERIALIZER.deserialize(parser, mapper))
                .build();

INFO  org.opensearch.sdk.handlers.ExtensionsIndicesModuleRequestHandler - Registering Indices Module Request received from OpenSearch
12:01:12.716 [opensearch[ad-extension][generic][T#3]] INFO  org.opensearch.sdk.handlers.ExtensionsIndicesModuleRequestHandler - Registering Indices Module Request received from OpenSearch
Oct 25, 2022 12:01:13 PM org.opensearch.client.RestClient logResponse
WARNING: request [PUT http://localhost:9200/.opendistro-anomaly-detectors] returned 1 warnings: [299 OpenSearch-3.0.0-SNAPSHOT-401c21054bae7a716e5f8394fce425d102c6ee00 "index name [.opendistro-anomaly-detectors] starts with a dot '.', in the next major version, index names starting with a dot are reserved for hidden indices and system indices"]

{
    ".opendistro-anomaly-detectors": {
        "aliases": {},
        "mappings": {
            "_meta": {
                "schema_version": 5
            },
            "properties": {
     

[owaiskazi19]

The next steps are:

1. Get the Synchronous response from the CreateIndexRequest - @owaiskazi19
2. Change NodeClient to SDKClient for making request calls - @dbwiddis
3. Integrate the complete workflow of creating the detector

[owaiskazi19]

Was able to create a detector opensearch-project/anomaly-detection#692

{
    "id": "1",
    "version": 1,
    "seqNo": 0,
    "primaryTerm": 1,
    "detector": {
        "name": "test-detector",
        "description": "Test detector",
        "time_field": "timestamp",
        "indices": [
            "server_log*"
        ],
        "filter_query": {
            "bool": {
                "filter": [
                    {
                        "range": {
                            "value": {
                                "from": 1,
                                "to": null,
                                "include_lower": false,
                                "include_upper": true,
                                "boost": 1.0
                            }
                        }
                    }
                ],
                "adjust_pure_negative": true,
                "boost": 1.0
            }
        },
        "detection_interval": {
            "period": {
                "interval": 1,
                "unit": "Minutes"
            }
        },
        "window_delay": {
            "period": {
                "interval": 1,
                "unit": "Minutes"
            }
        },
        "shingle_size": 8,
        "schema_version": 0,
        "feature_attributes": [
            {
                "feature_id": "qfrlRIQBANUbFEg-UqnN",
                "feature_name": "test",
                "feature_enabled": true,
                "aggregation_query": {
                    "test": {
                        "sum": {
                            "field": "value"
                        }
                    }
                }
            }
        ],
        "detector_type": "SINGLE_ENTITY",
        "result_index": "opensearch-ad-plugin-result-test"
    },
    "status": "CREATED"
}