Browse Source

[8.x] Returning ignored fields in the simulate ingest API (#117214) (#119246)

Keith Massey 9 months ago
parent
commit
eb0b8d69c4

+ 5 - 0
docs/changelog/117214.yaml

@@ -0,0 +1,5 @@
+pr: 117214
+summary: Returning ignored fields in the simulate ingest API
+area: Ingest Node
+type: enhancement
+issues: []

+ 56 - 0
qa/smoke-test-ingest-with-all-dependencies/src/yamlRestTest/resources/rest-api-spec/test/ingest/80_ingest_simulate.yml

@@ -1720,3 +1720,59 @@ setup:
   - match: { docs.0.doc._source.foo: 3 }
   - match: { docs.0.doc._source.bar: "some text value" }
   - not_exists: docs.0.doc.error
+
+---
+"Test ignored_fields":
+  - skip:
+      features:
+        - headers
+        - allowed_warnings
+
+  - requires:
+      cluster_features: ["simulate.ignored.fields"]
+      reason: "ingest simulate ignored fields added in 8.18"
+
+  - do:
+      headers:
+        Content-Type: application/json
+      simulate.ingest:
+        index: nonexistent
+        body: >
+          {
+            "docs": [
+              {
+                "_index": "simulate-test",
+                "_id": "y9Es_JIBiw6_GgN-U0qy",
+                "_score": 1,
+                "_source": {
+                  "abc": "sfdsfsfdsfsfdsfsfdsfsfdsfsfdsf"
+                }
+              }
+            ],
+            "index_template_substitutions": {
+              "ind_temp": {
+                "index_patterns": ["simulate-test"],
+                "composed_of": ["simulate-test"]
+              }
+            },
+            "component_template_substitutions": {
+              "simulate-test": {
+                "template": {
+                  "mappings": {
+                    "dynamic": false,
+                    "properties": {
+                      "abc": {
+                        "type": "keyword",
+                        "ignore_above": 1
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+  - length: { docs: 1 }
+  - match: { docs.0.doc._index: "simulate-test" }
+  - match: { docs.0.doc._source.abc: "sfdsfsfdsfsfdsfsfdsfsfdsfsfdsf" }
+  - match: { docs.0.doc.ignored_fields: [ {"field": "abc"} ] }
+  - not_exists: docs.0.doc.error

+ 1 - 0
server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -150,6 +150,7 @@ public class TransportVersions {
     public static final TransportVersion NODE_VERSION_INFORMATION_WITH_MIN_READ_ONLY_INDEX_VERSION = def(8_810_00_0);
     public static final TransportVersion ERROR_TRACE_IN_TRANSPORT_HEADER = def(8_811_00_0);
     public static final TransportVersion FAILURE_STORE_ENABLED_BY_CLUSTER_SETTING = def(8_812_00_0);
+    public static final TransportVersion SIMULATE_IGNORED_FIELDS = def(8_813_00_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

+ 3 - 1
server/src/main/java/org/elasticsearch/action/bulk/BulkFeatures.java

@@ -15,6 +15,7 @@ import org.elasticsearch.features.NodeFeature;
 import java.util.Set;
 
 import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_COMPONENT_TEMPLATE_SUBSTITUTIONS;
+import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_IGNORED_FIELDS;
 import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_INDEX_TEMPLATE_SUBSTITUTIONS;
 import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_MAPPING_ADDITION;
 import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_MAPPING_VALIDATION;
@@ -29,7 +30,8 @@ public class BulkFeatures implements FeatureSpecification {
             SIMULATE_COMPONENT_TEMPLATE_SUBSTITUTIONS,
             SIMULATE_INDEX_TEMPLATE_SUBSTITUTIONS,
             SIMULATE_MAPPING_ADDITION,
-            SIMULATE_SUPPORT_NON_TEMPLATE_MAPPING
+            SIMULATE_SUPPORT_NON_TEMPLATE_MAPPING,
+            SIMULATE_IGNORED_FIELDS
         );
     }
 }

+ 43 - 13
server/src/main/java/org/elasticsearch/action/bulk/TransportSimulateBulkAction.java

@@ -9,6 +9,8 @@
 
 package org.elasticsearch.action.bulk;
 
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.DocWriteRequest;
 import org.elasticsearch.action.admin.indices.template.post.TransportSimulateIndexTemplateAction;
@@ -33,6 +35,7 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.AtomicArray;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.core.Tuple;
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.index.IndexSettingProvider;
 import org.elasticsearch.index.IndexSettingProviders;
@@ -40,6 +43,8 @@ import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexingPressure;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.index.engine.Engine;
+import org.elasticsearch.index.mapper.IgnoredFieldMapper;
+import org.elasticsearch.index.mapper.LuceneDocument;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.SourceToParse;
 import org.elasticsearch.index.seqno.SequenceNumbers;
@@ -60,6 +65,7 @@ import org.elasticsearch.xcontent.XContentParserConfiguration;
 import org.elasticsearch.xcontent.XContentType;
 
 import java.io.IOException;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -85,6 +91,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
     public static final NodeFeature SIMULATE_INDEX_TEMPLATE_SUBSTITUTIONS = new NodeFeature("simulate.index.template.substitutions");
     public static final NodeFeature SIMULATE_MAPPING_ADDITION = new NodeFeature("simulate.mapping.addition");
     public static final NodeFeature SIMULATE_SUPPORT_NON_TEMPLATE_MAPPING = new NodeFeature("simulate.support.non.template.mapping");
+    public static final NodeFeature SIMULATE_IGNORED_FIELDS = new NodeFeature("simulate.ignored.fields");
     private final IndicesService indicesService;
     private final NamedXContentRegistry xContentRegistry;
     private final Set<IndexSettingProvider> indexSettingProviders;
@@ -137,12 +144,13 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
             DocWriteRequest<?> docRequest = bulkRequest.requests.get(i);
             assert docRequest instanceof IndexRequest : "TransportSimulateBulkAction should only ever be called with IndexRequests";
             IndexRequest request = (IndexRequest) docRequest;
-            Exception mappingValidationException = validateMappings(
+            Tuple<Collection<String>, Exception> validationResult = validateMappings(
                 componentTemplateSubstitutions,
                 indexTemplateSubstitutions,
                 mappingAddition,
                 request
             );
+            Exception mappingValidationException = validationResult.v2();
             responses.set(
                 i,
                 BulkItemResponse.success(
@@ -155,6 +163,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                         request.source(),
                         request.getContentType(),
                         request.getExecutedPipelines(),
+                        validationResult.v1(),
                         mappingValidationException
                     )
                 )
@@ -168,11 +177,12 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
     /**
      * This creates a temporary index with the mappings of the index in the request, and then attempts to index the source from the request
      * into it. If there is a mapping exception, that exception is returned. On success the returned exception is null.
-     * @parem componentTemplateSubstitutions The component template definitions to use in place of existing ones for validation
+     * @param componentTemplateSubstitutions The component template definitions to use in place of existing ones for validation
      * @param request The IndexRequest whose source will be validated against the mapping (if it exists) of its index
-     * @return a mapping exception if the source does not match the mappings, otherwise null
+     * @return a Tuple containing: (1) in v1 the names of any fields that would be ignored upon indexing and (2) in v2 the mapping
+     * exception if the source does not match the mappings, otherwise null
      */
-    private Exception validateMappings(
+    private Tuple<Collection<String>, Exception> validateMappings(
         Map<String, ComponentTemplate> componentTemplateSubstitutions,
         Map<String, ComposableIndexTemplate> indexTemplateSubstitutions,
         Map<String, Object> mappingAddition,
@@ -189,6 +199,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
 
         ClusterState state = clusterService.state();
         Exception mappingValidationException = null;
+        Collection<String> ignoredFields = List.of();
         IndexAbstraction indexAbstraction = state.metadata().getIndicesLookup().get(request.index());
         try {
             if (indexAbstraction != null
@@ -275,7 +286,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                     );
                     CompressedXContent mappings = template.mappings();
                     CompressedXContent mergedMappings = mergeMappings(mappings, mappingAddition);
-                    validateUpdatedMappings(mappings, mergedMappings, request, sourceToParse);
+                    ignoredFields = validateUpdatedMappings(mappings, mergedMappings, request, sourceToParse);
                 } else {
                     List<IndexTemplateMetadata> matchingTemplates = findV1Templates(simulatedState.metadata(), request.index(), false);
                     if (matchingTemplates.isEmpty() == false) {
@@ -289,7 +300,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                             xContentRegistry
                         );
                         final CompressedXContent combinedMappings = mergeMappings(new CompressedXContent(mappingsMap), mappingAddition);
-                        validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
+                        ignoredFields = validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
                     } else if (indexAbstraction != null && mappingAddition.isEmpty() == false) {
                         /*
                          * The index matched no templates of any kind, including the substitutions. But it might have a mapping. So we
@@ -298,7 +309,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                         MappingMetadata mappingFromIndex = clusterService.state().metadata().index(indexAbstraction.getName()).mapping();
                         CompressedXContent currentIndexCompressedXContent = mappingFromIndex == null ? null : mappingFromIndex.source();
                         CompressedXContent combinedMappings = mergeMappings(currentIndexCompressedXContent, mappingAddition);
-                        validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
+                        ignoredFields = validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
                     } else {
                         /*
                          * The index matched no templates and had no mapping of its own. If there were component template substitutions
@@ -306,27 +317,28 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                          * and validate.
                          */
                         final CompressedXContent combinedMappings = mergeMappings(null, mappingAddition);
-                        validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
+                        ignoredFields = validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
                     }
                 }
             }
         } catch (Exception e) {
             mappingValidationException = e;
         }
-        return mappingValidationException;
+        return Tuple.tuple(ignoredFields, mappingValidationException);
     }
 
     /*
-     * Validates that when updatedMappings are applied
+     * Validates that when updatedMappings are applied. If any fields would be ignored while indexing, then those field names are returned.
+     * Otherwise the returned Collection is empty.
      */
-    private void validateUpdatedMappings(
+    private Collection<String> validateUpdatedMappings(
         @Nullable CompressedXContent originalMappings,
         @Nullable CompressedXContent updatedMappings,
         IndexRequest request,
         SourceToParse sourceToParse
     ) throws IOException {
         if (updatedMappings == null) {
-            return; // no validation to do
+            return List.of(); // no validation to do
         }
         Settings dummySettings = Settings.builder()
             .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
@@ -343,7 +355,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
             .settings(dummySettings)
             .putMapping(new MappingMetadata(updatedMappings))
             .build();
-        indicesService.withTempIndexService(originalIndexMetadata, indexService -> {
+        Engine.Index result = indicesService.withTempIndexService(originalIndexMetadata, indexService -> {
             indexService.mapperService().merge(updatedIndexMetadata, MapperService.MergeReason.MAPPING_UPDATE);
             return IndexShard.prepareIndex(
                 indexService.mapperService(),
@@ -360,6 +372,24 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
                 0
             );
         });
+        final Collection<String> ignoredFields;
+        if (result == null) {
+            ignoredFields = List.of();
+        } else {
+            List<LuceneDocument> luceneDocuments = result.parsedDoc().docs();
+            assert luceneDocuments == null || luceneDocuments.size() == 1 : "Expected a single lucene document from index attempt";
+            if (luceneDocuments != null && luceneDocuments.size() == 1) {
+                ignoredFields = luceneDocuments.get(0)
+                    .getFields()
+                    .stream()
+                    .filter(field -> field.name().equals(IgnoredFieldMapper.NAME) && field instanceof StringField)
+                    .map(IndexableField::stringValue)
+                    .toList();
+            } else {
+                ignoredFields = List.of();
+            }
+        }
+        return ignoredFields;
     }
 
     private static CompressedXContent mergeMappings(@Nullable CompressedXContent originalMapping, Map<String, Object> mappingAddition)

+ 22 - 0
server/src/main/java/org/elasticsearch/action/ingest/SimulateIndexResponse.java

@@ -24,6 +24,7 @@ import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.xcontent.XContentType;
 
 import java.io.IOException;
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -34,6 +35,7 @@ import java.util.List;
 public class SimulateIndexResponse extends IndexResponse {
     private final BytesReference source;
     private final XContentType sourceXContentType;
+    private final Collection<String> ignoredFields;
     private final Exception exception;
 
     @SuppressWarnings("this-escape")
@@ -47,6 +49,11 @@ public class SimulateIndexResponse extends IndexResponse {
         } else {
             this.exception = null;
         }
+        if (in.getTransportVersion().onOrAfter(TransportVersions.SIMULATE_IGNORED_FIELDS)) {
+            this.ignoredFields = in.readStringCollectionAsList();
+        } else {
+            this.ignoredFields = List.of();
+        }
     }
 
     @SuppressWarnings("this-escape")
@@ -57,6 +64,7 @@ public class SimulateIndexResponse extends IndexResponse {
         BytesReference source,
         XContentType sourceXContentType,
         List<String> pipelines,
+        Collection<String> ignoredFields,
         @Nullable Exception exception
     ) {
         // We don't actually care about most of the IndexResponse fields:
@@ -73,6 +81,7 @@ public class SimulateIndexResponse extends IndexResponse {
         this.source = source;
         this.sourceXContentType = sourceXContentType;
         setShardInfo(ShardInfo.EMPTY);
+        this.ignoredFields = ignoredFields;
         this.exception = exception;
     }
 
@@ -84,6 +93,16 @@ public class SimulateIndexResponse extends IndexResponse {
         builder.field("_source", XContentHelper.convertToMap(source, false, sourceXContentType).v2());
         assert executedPipelines != null : "executedPipelines is null when it shouldn't be - we always list pipelines in simulate mode";
         builder.array("executed_pipelines", executedPipelines.toArray());
+        if (ignoredFields.isEmpty() == false) {
+            builder.startArray("ignored_fields");
+            for (String ignoredField : ignoredFields) {
+                builder.startObject();
+                builder.field("field", ignoredField);
+                builder.endObject();
+            }
+            ;
+            builder.endArray();
+        }
         if (exception != null) {
             builder.startObject("error");
             ElasticsearchException.generateThrowableXContent(builder, params, exception);
@@ -105,6 +124,9 @@ public class SimulateIndexResponse extends IndexResponse {
         if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_15_0)) {
             out.writeException(exception);
         }
+        if (out.getTransportVersion().onOrAfter(TransportVersions.SIMULATE_IGNORED_FIELDS)) {
+            out.writeStringCollection(ignoredFields);
+        }
     }
 
     public Exception getException() {

+ 36 - 0
server/src/test/java/org/elasticsearch/action/ingest/SimulateIndexResponseTests.java

@@ -48,6 +48,7 @@ public class SimulateIndexResponseTests extends ESTestCase {
             sourceBytes,
             XContentType.JSON,
             pipelines,
+            List.of(),
             null
         );
 
@@ -79,6 +80,7 @@ public class SimulateIndexResponseTests extends ESTestCase {
             sourceBytes,
             XContentType.JSON,
             pipelines,
+            List.of(),
             new ElasticsearchException("Some failure")
         );
 
@@ -103,6 +105,39 @@ public class SimulateIndexResponseTests extends ESTestCase {
             ),
             Strings.toString(indexResponseWithException)
         );
+
+        SimulateIndexResponse indexResponseWithIgnoredFields = new SimulateIndexResponse(
+            id,
+            index,
+            version,
+            sourceBytes,
+            XContentType.JSON,
+            pipelines,
+            List.of("abc", "def"),
+            null
+        );
+
+        assertEquals(
+            XContentHelper.stripWhitespace(
+                Strings.format(
+                    """
+                        {
+                          "_id": "%s",
+                          "_index": "%s",
+                          "_version": %d,
+                          "_source": %s,
+                          "executed_pipelines": [%s],
+                          "ignored_fields": [{"field": "abc"}, {"field": "def"}]
+                        }""",
+                    id,
+                    index,
+                    version,
+                    source,
+                    pipelines.stream().map(pipeline -> "\"" + pipeline + "\"").collect(Collectors.joining(","))
+                )
+            ),
+            Strings.toString(indexResponseWithIgnoredFields)
+        );
     }
 
     public void testSerialization() throws IOException {
@@ -135,6 +170,7 @@ public class SimulateIndexResponseTests extends ESTestCase {
             sourceBytes,
             xContentType,
             pipelines,
+            randomList(0, 20, () -> randomAlphaOfLength(15)),
             randomBoolean() ? null : new ElasticsearchException("failed")
         );
     }

+ 17 - 0
server/src/test/java/org/elasticsearch/rest/action/ingest/RestSimulateIngestActionTests.java

@@ -175,6 +175,14 @@ public class RestSimulateIngestActionTests extends ESTestCase {
                     "executed_pipelines" : [
                       "pipeline1",
                       "pipeline2"
+                    ],
+                    "ignored_fields" : [
+                      {
+                        "field" : "abc"
+                      },
+                      {
+                        "field" : "def"
+                      }
                     ]
                   }
                 },
@@ -199,6 +207,14 @@ public class RestSimulateIngestActionTests extends ESTestCase {
                     "executed_pipelines" : [
                       "pipeline1",
                       "pipeline2"
+                    ],
+                    "ignored_fields" : [
+                      {
+                        "field" : "abc"
+                      },
+                      {
+                        "field" : "def"
+                      }
                     ]
                   }
                 }
@@ -228,6 +244,7 @@ public class RestSimulateIngestActionTests extends ESTestCase {
                 BytesReference.fromByteBuffers(sourceByteBuffer),
                 XContentType.JSON,
                 List.of("pipeline1", "pipeline2"),
+                List.of("abc", "def"),
                 null
             )
         );