[Backport 2.x] Adds preset contentRegistry for IngestProcessors (#3281)…

… (#3317) (#3406) * Adds preset contentRegistry for IngestProcessors (#3281) * add preset xContentRegistry to ingestProcessors for custom parametized local models Curently local models that use the parameters map within the payload to create a request can not create objects to be used for local model prediction. This requires a opensearch core change because it needs the contentRegistry,however given there is not much dependency on the registry (currently) we can give it the preset registry given in the MachineLearningPlugin class vai the getNamedXContent() class Signed-off-by: Brian Flores <iflorbri@amazon.com> * Adds UT for proving models depend on xContentRegistry for prediction Signed-off-by: Brian Flores <iflorbri@amazon.com> * apply spotless Signed-off-by: Brian Flores <iflorbri@amazon.com> * Adds IT for Asymmetric Embedding scenario with MLInferenceIngestProcessor We needed to make sure that a IT existed so that the preset content registry on the processor could work with parametized local models. By providing an IT that uses the asymetric embedding model its proven that the content registry is needed to create the embeddings. In this specific test case I used a ingest pipeline to convert passage embeddings, by simulating the pipeline to save test time. Signed-off-by: Brian Flores <iflorbri@amazon.com> --------- Signed-off-by: Brian Flores <iflorbri@amazon.com> (cherry picked from commit df1b1ef) * apply spotless Signed-off-by: Brian Flores <iflorbri@amazon.com> * Adds check for Java 11 The test passes on JAVA 21 but for some reason Java 11 has trouble passing in a message so it returns null when getMessage occurs so when Mockito tries to invoke getMessage it will get get a second NPE. and thus is why the exception is caught. This code change is to get around the gap that Java 11 has. Signed-off-by: Brian Flores <iflorbri@amazon.com> * spotless Signed-off-by: Brian Flores <iflorbri@amazon.com> --------- Signed-off-by: Brian Flores <iflorbri@amazon.com> (cherry picked from commit ca0f0da) Co-authored-by: Brian Flores <iflorbri@amazon.com>
opensearch-project · Jan 17, 2025 · 4587c21 · 4587c21
1 parent ad7fbec
commit 4587c21
Show file tree

Hide file tree

Showing 4 changed files with 185 additions and 3 deletions.
diff --git a/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java b/plugin/src/main/java/org/opensearch/ml/plugin/MachineLearningPlugin.java
@@ -1063,10 +1063,12 @@ public void loadExtensions(ExtensionLoader loader) {
     @Override
     public Map<String, org.opensearch.ingest.Processor.Factory> getProcessors(org.opensearch.ingest.Processor.Parameters parameters) {
         Map<String, org.opensearch.ingest.Processor.Factory> processors = new HashMap<>();
+        NamedXContentRegistry contentRegistry = new NamedXContentRegistry(getNamedXContent());
+
         processors
             .put(
                 MLInferenceIngestProcessor.TYPE,
-                new MLInferenceIngestProcessor.Factory(parameters.scriptService, parameters.client, xContentRegistry)
+                new MLInferenceIngestProcessor.Factory(parameters.scriptService, parameters.client, contentRegistry)
             );
         return Collections.unmodifiableMap(processors);
     }

diff --git a/plugin/src/test/java/org/opensearch/ml/processor/MLInferenceIngestProcessorTests.java b/plugin/src/test/java/org/opensearch/ml/processor/MLInferenceIngestProcessorTests.java
@@ -6,7 +6,14 @@
 package org.opensearch.ml.processor;
 
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.argThat;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 import static org.opensearch.ml.common.utils.StringUtils.toJson;
 import static org.opensearch.ml.processor.MLInferenceIngestProcessor.DEFAULT_OUTPUT_FIELD_NAME;
 
@@ -31,6 +38,7 @@
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.ingest.IngestDocument;
+import org.opensearch.ml.common.FunctionName;
 import org.opensearch.ml.common.dataset.remote.RemoteInferenceInputDataSet;
 import org.opensearch.ml.common.input.MLInput;
 import org.opensearch.ml.common.output.model.MLResultDataType;
@@ -138,6 +146,63 @@ public void testExecute_Exception() throws Exception {
 
     }
 
+    /**
+     * Models that use the parameters field need to have a valid NamedXContentRegistry object to create valid MLInputs. For example
+     * <pre>
+     * PUT   /_plugins/_ml/_predict/text_embedding/model_id
+     *  {
+     *     "parameters": {
+     *         "content_type" : "query"
+     *     },
+     *     "text_docs" : ["what day is it today?"],
+     *     "target_response" : ["sentence_embedding"]
+     *   }
+     * </pre>
+     * These types of models like Local Asymmetric embedding models use the parameters field.
+     * And as such we need to test that having the contentRegistry throws an exception as it can not
+     * properly create a valid MLInput to perform prediction
+     *
+     * @implNote If you check the stack trace of the test you will see it tells you that it's a direct consequence of xContentRegistry being null
+     */
+    public void testExecute_xContentRegistryNullWithLocalModel_throwsException() throws Exception {
+        // Set the registry to null and reset after exiting the test
+        xContentRegistry = null;
+
+        String localModelInput =
+            "{ \"text_docs\": [\"What day is it today?\"],\"target_response\": [\"sentence_embedding\"], \"parameters\": { \"contentType\" : \"query\"} }";
+
+        MLInferenceIngestProcessor processor = createMLInferenceProcessor(
+            "local_model_id",
+            null,
+            null,
+            null,
+            false,
+            FunctionName.TEXT_EMBEDDING.toString(),
+            false,
+            false,
+            false,
+            localModelInput
+        );
+        try {
+            String npeMessage =
+                "Cannot invoke \"org.opensearch.ml.common.input.MLInput.setAlgorithm(org.opensearch.ml.common.FunctionName)\" because \"mlInput\" is null";
+
+            processor.execute(ingestDocument, handler);
+            verify(handler)
+                .accept(
+                    isNull(),
+                    argThat(exception -> exception instanceof NullPointerException && exception.getMessage().equals(npeMessage))
+                );
+        } catch (Exception e) {
+            // Java 11 doesn't pass the message correctly resulting in an anonymous NPE thus this getMessage() results in null
+            if (e.getMessage() != null) {
+                assertEquals("this catch block should not get executed.", e.getMessage());
+            }
+        }
+        // reset to mocked object
+        xContentRegistry = mock(NamedXContentRegistry.class);
+    }
+
     /**
      * test nested object document with array of Map<String,String>
      */

diff --git a/plugin/src/test/java/org/opensearch/ml/rest/RestMLInferenceIngestProcessorIT.java b/plugin/src/test/java/org/opensearch/ml/rest/RestMLInferenceIngestProcessorIT.java
@@ -6,6 +6,7 @@
 package org.opensearch.ml.rest;
 
 import static org.opensearch.ml.common.MLTask.MODEL_ID_FIELD;
+import static org.opensearch.ml.utils.TestData.SENTENCE_TRANSFORMER_MODEL_HASH_VALUE;
 import static org.opensearch.ml.utils.TestData.SENTENCE_TRANSFORMER_MODEL_URL;
 import static org.opensearch.ml.utils.TestHelper.makeRequest;
 
@@ -25,6 +26,7 @@
 import org.opensearch.ml.common.transport.register.MLRegisterModelInput;
 import org.opensearch.ml.utils.TestHelper;
 
+import com.jayway.jsonpath.DocumentContext;
 import com.jayway.jsonpath.JsonPath;
 
 public class RestMLInferenceIngestProcessorIT extends MLCommonsRestTestCase {
@@ -431,6 +433,110 @@ public void testMLInferenceProcessorLocalModelObjectField() throws Exception {
         Assert.assertEquals(0.49191704, (Double) embedding2.get(0), 0.005);
     }
 
+    public void testMLInferenceIngestProcessor_simulatesIngestPipelineSuccessfully_withAsymmetricEmbeddingModelUsingPassageContentType()
+        throws Exception {
+        String taskId = registerModel(TestHelper.toJsonString(registerAsymmetricEmbeddingModelInput()));
+        waitForTask(taskId, MLTaskState.COMPLETED);
+        getTask(client(), taskId, response -> {
+            assertNotNull(response.get(MODEL_ID_FIELD));
+            this.localModelId = (String) response.get(MODEL_ID_FIELD);
+            try {
+                String deployTaskID = deployModel(this.localModelId);
+                waitForTask(deployTaskID, MLTaskState.COMPLETED);
+
+                getModel(client(), this.localModelId, model -> { assertEquals("DEPLOYED", model.get("model_state")); });
+            } catch (IOException | InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+        });
+
+        String asymmetricPipelineName = "asymmetric_embedding_pipeline";
+        String createPipelineRequestBody = "{\n"
+            + "  \"description\": \"ingest PASSAGE text and generate a embedding using an asymmetric model\",\n"
+            + "  \"processors\": [\n"
+            + "    {\n"
+            + "      \"ml_inference\": {\n"
+            + "\n"
+            + "        \"model_input\": \"{\\\"text_docs\\\":[\\\"${input_map.text_docs}\\\"],\\\"target_response\\\":[\\\"sentence_embedding\\\"],\\\"parameters\\\":{\\\"content_type\\\":\\\"passage\\\"}}\",\n"
+            + "        \"function_name\": \"text_embedding\",\n"
+            + "        \"model_id\": \""
+            + this.localModelId
+            + "\",\n"
+            + "        \"input_map\": [\n"
+            + "          {\n"
+            + "            \"text_docs\": \"description\"\n"
+            + "          }\n"
+            + "        ],\n"
+            + "        \"output_map\": [\n"
+            + "          {\n"
+            + "\n"
+            + "            "
+            + "            \"fact_embedding\": \"$.inference_results[0].output[0].data\"\n"
+            + "          }\n"
+            + "        ]\n"
+            + "      }\n"
+            + "    }\n"
+            + "  ]\n"
+            + "}";
+
+        createPipelineProcessor(createPipelineRequestBody, asymmetricPipelineName);
+        String sampleDocuments = "{\n"
+            + "  \"docs\": [\n"
+            + "    {\n"
+            + "      \"_index\": \"my-index\",\n"
+            + "      \"_id\": \"1\",\n"
+            + "      \"_source\": {\n"
+            + "        \"title\": \"Central Park\",\n"
+            + "        \"description\": \"A large public park in the heart of New York City, offering a wide range of recreational activities.\"\n"
+            + "      }\n"
+            + "    },\n"
+            + "    {\n"
+            + "      \"_index\": \"my-index\",\n"
+            + "      \"_id\": \"2\",\n"
+            + "      \"_source\": {\n"
+            + "        \"title\": \"Empire State Building\",\n"
+            + "        \"description\": \"An iconic skyscraper in New York City offering breathtaking views from its observation deck.\"\n"
+            + "      }\n"
+            + "    }\n"
+            + "  ]\n"
+            + "}\n";
+
+        Map simulateResponseDocuments = simulateIngestPipeline(asymmetricPipelineName, sampleDocuments);
+
+        DocumentContext documents = JsonPath.parse(simulateResponseDocuments);
+
+        List centralParkFactEmbedding = documents.read("docs.[0].*._source.fact_embedding.*");
+        assertEquals(768, centralParkFactEmbedding.size());
+        Assert.assertEquals(0.5137818, (Double) centralParkFactEmbedding.get(0), 0.005);
+
+        List empireStateBuildingFactEmbedding = documents.read("docs.[1].*._source.fact_embedding.*");
+        assertEquals(768, empireStateBuildingFactEmbedding.size());
+        Assert.assertEquals(0.4493039, (Double) empireStateBuildingFactEmbedding.get(0), 0.005);
+    }
+
+    private MLRegisterModelInput registerAsymmetricEmbeddingModelInput() {
+        MLModelConfig modelConfig = TextEmbeddingModelConfig
+            .builder()
+            .modelType("bert")
+            .frameworkType(TextEmbeddingModelConfig.FrameworkType.SENTENCE_TRANSFORMERS)
+            .embeddingDimension(768)
+            .queryPrefix("query >>")
+            .passagePrefix("passage >> ")
+            .build();
+
+        return MLRegisterModelInput
+            .builder()
+            .modelName("test_model_name")
+            .version("1.0.0")
+            .functionName(FunctionName.TEXT_EMBEDDING)
+            .modelFormat(MLModelFormat.TORCH_SCRIPT)
+            .modelConfig(modelConfig)
+            .url(SENTENCE_TRANSFORMER_MODEL_URL)
+            .deployModel(false)
+            .hashValue(SENTENCE_TRANSFORMER_MODEL_HASH_VALUE)
+            .build();
+    }
+
     // TODO: add tests for other local model types such as sparse/cross encoders
     public void testMLInferenceProcessorLocalModelNestedField() throws Exception {
 
@@ -550,6 +656,14 @@ protected void createPipelineProcessor(String requestBody, final String pipeline
 
     }
 
+    protected Map simulateIngestPipeline(String pipelineName, String sampleDocuments) throws IOException {
+        Response ingestionResponse = TestHelper
+            .makeRequest(client(), "POST", "/_ingest/pipeline/" + pipelineName + "/_simulate", null, sampleDocuments, null);
+        assertEquals(200, ingestionResponse.getStatusLine().getStatusCode());
+
+        return parseResponseToMap(ingestionResponse);
+    }
+
     protected void createIndex(String indexName, String requestBody) throws Exception {
         Response response = makeRequest(client(), "PUT", indexName, null, requestBody, null);
         assertEquals(200, response.getStatusLine().getStatusCode());
@@ -585,7 +699,7 @@ protected MLRegisterModelInput registerModelInput() throws IOException, Interrup
             .modelConfig(modelConfig)
             .url(SENTENCE_TRANSFORMER_MODEL_URL)
             .deployModel(false)
-            .hashValue("e13b74006290a9d0f58c1376f9629d4ebc05a0f9385f40db837452b167ae9021")
+            .hashValue(SENTENCE_TRANSFORMER_MODEL_HASH_VALUE)
             .build();
     }
 

diff --git a/plugin/src/test/java/org/opensearch/ml/utils/TestData.java b/plugin/src/test/java/org/opensearch/ml/utils/TestData.java
@@ -30,6 +30,7 @@ public class TestData {
         "https://github.com/opensearch-project/ml-commons/blob/2.x/ml-algorithms/src/test/resources/org/opensearch/ml/engine/algorithms/text_embedding/all-MiniLM-L6-v2_torchscript_huggingface.zip?raw=true";
     public static final String SENTENCE_TRANSFORMER_MODEL_URL =
         "https://github.com/opensearch-project/ml-commons/blob/2.x/ml-algorithms/src/test/resources/org/opensearch/ml/engine/algorithms/text_embedding/traced_small_model.zip?raw=true";
+    public static final String SENTENCE_TRANSFORMER_MODEL_HASH_VALUE = "e13b74006290a9d0f58c1376f9629d4ebc05a0f9385f40db837452b167ae9021";
     public static final String TIME_FIELD = "timestamp";
     public static final String HUGGINGFACE_TRANSFORMER_MODEL_HASH_VALUE =
         "e13b74006290a9d0f58c1376f9629d4ebc05a0f9385f40db837452b167ae9021";