Fix doc-level monitor failure on source indices with custom analysis or malformed mappings

[thecodingshrimp]

Problem

Doc-level monitors fail at creation time when the source index has field mappings that contain attributes incompatible with the query index. Two distinct failure categories have been observed.

Category 1 — analysis resource references

Fields such as text or keyword can carry analyzer, normalizer, search_analyzer, search_quote_analyzer, or similarity attributes that reference custom analysis objects defined in the source index's settings.analysis.* block. The doc-level query index is created from a fixed settings resource with no custom analysis block. When DocLevelMonitorQueries copies field mappings verbatim via props.toMutableMap() and submits them in a PutMappingRequest, OpenSearch rejects the request:

IllegalArgumentException: analyzer [my_analyzer] has not been configured in mappings

Category 2 — properties on a scalar field type

Dynamic mapping collisions can leave a field in cluster state with both "type": "text" (or any scalar type) and a properties sub-block, which is only valid on object/nested mappers. OpenSearch stores this silently at ingestion time but rejects it when submitted explicitly via the PUT mapping API:

MapperParsingException: unknown parameter [properties] on mapper [field] of type [text]

Both failures block monitor creation on any real-world index that uses custom analyzers, normalizers, or has experienced dynamic mapping collisions — a common production pattern. The issue is actively tracked in alerting#961 and affects Security Analytics detector creation as reported in security-analytics#697 and security-analytics#1798.

Root cause

leafNodeProcessor in DocLevelMonitorQueries.kt performs an unconditional props.toMutableMap() copy of every source field attribute before building the PutMappingRequest. No filtering is applied to remove attributes that are invalid on the query index.

Fix

Add sanitizeFieldMappingAttributes(fieldType, mapping) to the DocLevelMonitorQueries companion object and call it on newProps immediately after the toMutableMap() copy inside leafNodeProcessor.

The function:

• Strips all five analysis-reference attributes (analyzer, search_analyzer, search_quote_analyzer, normalizer, similarity) — Category 1
• Strips properties from any field whose type is explicitly set to something other than object or nested — Category 2
• Recurses into multi-fields via the fields sub-map

No index close/open cycle is required. The change is confined to the in-memory PutMappingRequest payload. The query index stores Percolator query documents for doc-level matching only, not user-facing search data, so dropping these attributes is safe and lossless for its purpose.

This approach resolves the architectural concern raised in alerting#961: the original "won't fix" closure described Strategy B (copying settings.analysis.* from the source index into the query index), which is genuinely blocked because the shared query index cannot safely merge conflicting analysis configurations from multiple source indices. This fix implements Strategy A (strip the incompatible attributes before the PUT), which is independent of that constraint.

Changes

• alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt: add ANALYSIS_ATTRIBUTES constant and sanitizeFieldMappingAttributes() to companion object; wire into leafNodeProcessor
• alerting/src/test/kotlin/org/opensearch/alerting/util/AlertingUtilsTests.kt: 7 unit tests covering both failure categories, edge cases (object/nested/absent type), multi-field recursion, and clean pass-through

Related issues

• Fixes [BUG] [alerting_exception] analyzer [analyzer_keyword] has not been configured in mappings #961
• Relates to [BUG] Error Creating Detector [security_analytics_exception] normalizer [lowercase_keyword] not found for field security-analytics#697
• Relates to Detector creation fails with IllegalArgumentException when source index uses index-level analysis resources (custom analyzers, normalizers, similarity) security-analytics#1798

Testing

Unit tests added in AlertingUtilsTests. All existing tests in that file continue to pass unchanged.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

cudos: claude

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 99dd424)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Sanitized copy unused newProps is populated from a sanitized copy of props, but the subsequent code in leafNodeProcessor (not shown in the diff but referenced via props.containsKey("type") on the next line) continues to operate on the original props. Verify that downstream logic reads from newProps (the sanitized map) rather than props before returning, otherwise the analyzer/properties attributes will still be sent to the query index and the fix will not take effect for the leaf mapping path. val newProps = props.toMutableMap().also { sanitizeFieldMappingAttributes(it[TYPE] as? String, it) }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 99dd424

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Deep-copy before in-place mutation props.toMutableMap() creates only a shallow copy; the nested fields and properties maps still reference the originals. sanitizeFieldMappingAttributes mutates those nested maps in place (via mapping.keys.removeAll and mapping.remove(PROPERTIES)), which will also mutate the caller's original props and the underlying indexMetadata mapping tree. Deep-copy the map before sanitizing to avoid corrupting shared state. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [352-354] -val newProps = props.toMutableMap().also { +@Suppress("UNCHECKED_CAST") +val newProps = (deepCopy(props) as MutableMap<String, Any>).also { sanitizeFieldMappingAttributes(it[TYPE] as? String, it) } Suggestion importance[1-10]: 6 __ Why: Valid observation that toMutableMap() is a shallow copy, meaning nested fields/properties maps are still shared with the original indexMetadata mapping tree. In-place mutation could corrupt shared state, though the actual impact depends on whether the original mapping is used elsewhere after this processing.	Low
General	Guard against immutable sub-maps during recursion *Casting the inner map with as? MutableMap<String, Any> is an unchecked cast that will succeed for any MutableMap<*, > even if its keys/values are not String/Any. More importantly, if the sub-mapping was deserialized as an immutable Map (e.g., from JSON parsers producing LinkedHashMap is fine, but some paths produce Collections.unmodifiableMap), the mapping.keys.removeAll(...) and mapping.remove(PROPERTIES) calls inside the recursion will throw UnsupportedOperationException. Guard against non-mutable maps or convert to mutable before recursing. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [92-97] // Recurse into multi-fields @Suppress("UNCHECKED_CAST") (mapping["fields"] as? Map<*, *>)?.forEach { (_, subMapping) -> - (subMapping as? MutableMap<String, Any>)?.let { + (subMapping as? MutableMap<String, Any>)?.takeIf { runCatching { it.remove("__probe__") }.isSuccess }?.let { sanitizeFieldMappingAttributes(it[TYPE] as? String, it) } } Suggestion importance[1-10]: 2 __ Why: The concern about immutable maps is speculative, and the proposed fix using a __probe__ remove trick is hacky and could have side effects. The improvement is marginal and the suggested code is not a clean solution.	Low

---

Previous suggestions

Suggestions up to commit 0e80b45

Category	Suggestion	Impact
Possible issue	Avoid unintended mutation via shallow copy props.toMutableMap() creates only a shallow copy, so nested maps (e.g. fields, properties) are still shared references with the original props. When sanitizeFieldMappingAttributes recursively mutates those nested maps (removing analyzer, normalizer, etc.), it mutates the caller's original mapping tree as a side effect. Deep-copy the map before sanitizing, or accept and document the in-place mutation intentionally. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [362-364] val leafNodeProcessor = fun(fieldName: String, fullPath: String, props: MutableMap<String, Any>): Triple<String, String, MutableMap<String, Any>> { - val newProps = props.toMutableMap().also { - sanitizeFieldMappingAttributes(it[TYPE] as? String, it) - } + val newProps = deepCopyMapping(props) + sanitizeFieldMappingAttributes(newProps[TYPE] as? String, newProps) Suggestion importance[1-10]: 7 __ Why: The observation is correct: toMutableMap() is a shallow copy, so recursive mutation of nested maps like fields and properties will affect the original props. This could lead to subtle bugs where the source mapping tree is unintentionally modified, though the practical impact depends on whether callers reuse props afterward.	Medium

Suggestions up to commit 5a796b4

Category	Suggestion	Impact
Possible issue	Handle immutable sub-map entries during recursion The cast subMapping as? MutableMap<String, Any> silently skips entries that are immutable maps (e.g., when the source mapping was deserialized as LinkedHashMap values containing plain Map sub-entries). If any sub-mapping is not a MutableMap, its analysis attributes will not be stripped, and the PutMappingRequest will still fail. Consider wrapping non-mutable sub-maps into a mutable copy and writing them back, or cast to MutableMap<String, Any?> matching the actual runtime type. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [102-107] // Recurse into multi-fields @Suppress("UNCHECKED_CAST") -(mapping["fields"] as? Map<*, *>)?.forEach { (_, subMapping) -> - (subMapping as? MutableMap<String, Any>)?.let { - sanitizeFieldMappingAttributes(it[TYPE] as? String, it) +(mapping["fields"] as? MutableMap<String, Any>)?.let { fieldsMap -> + fieldsMap.entries.forEach { entry -> + val sub = entry.value + if (sub is MutableMap<*, *>) { + @Suppress("UNCHECKED_CAST") + val mSub = sub as MutableMap<String, Any> + sanitizeFieldMappingAttributes(mSub[TYPE] as? String, mSub) + } else if (sub is Map<*, *>) { + @Suppress("UNCHECKED_CAST") + val mSub = (sub as Map<String, Any>).toMutableMap() + sanitizeFieldMappingAttributes(mSub[TYPE] as? String, mSub) + entry.setValue(mSub) + } } } Suggestion importance[1-10]: 4 __ Why: The concern about immutable sub-maps is theoretically valid, but in practice OpenSearch mapping source maps are typically deserialized as HashMap/LinkedHashMap which are mutable. The suggestion adds significant complexity for an edge case that may not occur in this code path.	Low

Suggestions up to commit 8ff6a8e

Category	Suggestion	Impact
General	Avoid parameter/map type disagreement risk The fieldType parameter is passed by the caller but the function also reads mapping[TYPE] during recursion. If a caller passes a fieldType that disagrees with mapping[TYPE] (e.g., a stale value), the branch decisions become inconsistent. Consider deriving fieldType from mapping[TYPE] inside the function to eliminate this class of bug. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [97-99] +val effectiveType = mapping[TYPE] as? String ?: fieldType // Category 2: remove "properties" from scalar (non-object, non-nested) fields. -// A null/absent type defaults to "object" in OpenSearch, so only strip when the type is -// explicitly set to something other than "object" or "nested". -if (fieldType != null && fieldType != "object" && fieldType != NESTED) { +if (effectiveType != null && effectiveType != "object" && effectiveType != NESTED) { mapping.remove(PROPERTIES) } Suggestion importance[1-10]: 3 __ Why: The suggestion is theoretically valid but low-impact: all callers in the PR already pass mapping[TYPE] as the fieldType argument, so the disagreement scenario is unlikely in practice. It's a minor defensive coding improvement.	Low

Suggestions up to commit b282a54

Category	Suggestion	Impact
Possible issue	Ensure multi-field maps are recursively sanitized The fields map values may be immutable Map instances (e.g. from sourceAsMap), in which case the as? MutableMap cast returns null and multi-field attributes are never sanitized. Convert entries into a mutable map before recursion and write back the sanitized result to ensure attributes like normalizer/analyzer under multi-fields are actually stripped. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [102-107] // Recurse into multi-fields @Suppress("UNCHECKED_CAST") -(mapping["fields"] as? Map<*, *>)?.forEach { (_, subMapping) -> - (subMapping as? MutableMap<String, Any>)?.let { - sanitizeFieldMappingAttributes(it[TYPE] as? String, it) +(mapping["fields"] as? Map<String, Any>)?.let { fieldsMap -> + val sanitizedFields = mutableMapOf<String, Any>() + fieldsMap.forEach { (name, subMapping) -> + val subMutable = (subMapping as? Map<String, Any>)?.toMutableMap() + if (subMutable != null) { + sanitizeFieldMappingAttributes(subMutable[TYPE] as? String, subMutable) + sanitizedFields[name] = subMutable + } else { + sanitizedFields[name] = subMapping + } } + mapping["fields"] = sanitizedFields } Suggestion importance[1-10]: 7 __ Why: Valid concern: if fields sub-maps are immutable Map instances, the as? MutableMap cast returns null and sanitization is silently skipped. The fix ensures multi-field attributes like normalizer/analyzer are actually stripped, which is important for correctness given the PR's purpose.	Medium

Suggestions up to commit 7cab7b2

Category	Suggestion	Impact
Possible issue	Defensively mutate multi-field sub-maps The recursion into multi-fields relies on the "fields" sub-maps being MutableMap<String, Any>, but they may actually be immutable maps read from cluster state (e.g. LinkedHashMap passed as Map). Convert to mutable defensively, or ensure the source maps at the call site are deeply mutable, otherwise the analysis-attribute removal on multi-fields will silently no-op. alerting/src/main/kotlin/org/opensearch/alerting/util/DocLevelMonitorQueries.kt [120-125] @Suppress("UNCHECKED_CAST") -(mapping["fields"] as? Map<*, *>)?.forEach { (_, subMapping) -> - (subMapping as? MutableMap<String, Any>)?.let { - sanitizeFieldMappingAttributes(it[TYPE] as? String, it) +(mapping["fields"] as? MutableMap<String, Any>)?.let { fieldsMap -> + fieldsMap.entries.forEach { entry -> + val subMapping = (entry.value as? Map<String, Any>)?.toMutableMap() ?: return@forEach + sanitizeFieldMappingAttributes(subMapping[TYPE] as? String, subMapping) + entry.setValue(subMapping) } } Suggestion importance[1-10]: 5 __ Why: The concern is valid: if sub-maps in the fields block are immutable, the cast to MutableMap returns null and sanitization silently no-ops. However, in the call site the mapping is built via props.toMutableMap() and cluster-state maps typically are HashMap, so the risk may be limited. The suggestion improves defensive robustness but has moderate impact.	Low

[github-actions]

Persistent review updated to latest commit 7cab7b2

[github-actions]

Persistent review updated to latest commit b282a54

[github-actions]

Persistent review updated to latest commit 8ff6a8e

[github-actions]

Persistent review updated to latest commit 5a796b4

[github-actions]

Persistent review updated to latest commit 0e80b45

[github-actions]

Persistent review updated to latest commit 99dd424