Add support for parent event field access during iterate_on processing in add_entries processor#6713
Add support for parent event field access during iterate_on processing in add_entries processor#6713yavmanis wants to merge 3 commits intoopensearch-project:mainfrom
Conversation
…ries processor Signed-off-by: Manisha Yadav <yavmanis@amazon.com>
1390a5d to
0ba2de5
Compare
| return !evaluateWhenOnElement || (iterateOn != null && addToElementWhen != null); | ||
| } | ||
|
|
||
| public Entry(final String key, |
There was a problem hiding this comment.
issue (non-blocking): Massive constructor duplication
- Problem: The new 13-parameter constructor duplicates all the validation logic from the existing 11-parameter constructor. There are now four constructors in Entry, three of which contain nearly identical validation blocks (null checks, mutualexclusion checks, etc.).
- Impact: Any future validation change must be replicated across all constructors. This is a maintenance burden.
- Fix: Have the shorter constructors delegate to the longest one with default values for the new parameters:
public Entry(String key, String metadataKey, Object value, String format,
String valueExpression, boolean overwriteIfKeyExists, boolean appendIfKeyExists,
String addWhen, String iterateOn, boolean flattenKey, String addToElementWhen) {
this(key, metadataKey, value, format, valueExpression, overwriteIfKeyExists,
appendIfKeyExists, addWhen, iterateOn, flattenKey, true, false, addToElementWhen);
}This eliminates all duplicated validation.
There was a problem hiding this comment.
Removed the duplication in the constructor as suggested.
| @AlsoRequired(values = { | ||
| @AlsoRequired.Required(name="iterate_on") | ||
| }) | ||
| private boolean disableRootKeys = true; |
There was a problem hiding this comment.
suggestion: Confusing double-negative naming for disable_root_keys
- Problem: To enable root-level field access, users must set disable_root_keys: false. The double-negative (!disableRootKeys in the processor code) is hard to reason about. The issue discussion originally proposed use_root_context: true which reads more
naturally. - Impact: User confusion in configuration. However, @dlvenable explicitly requested this name in this comment ([FEATURE] Support parent event field access during iterate_on processing #6609 (comment)), so this is intentional.
- Fix: No change needed — this follows the maintainer's direction. Noting for awareness only.
There was a problem hiding this comment.
I would suggest using a positive field name as well, like resolve_from_root, use_root_context, etc. so that: 1) the default value would be false; 2) it's easier for users to comprehend and use
Signed-off-by: Manisha Yadav <yavmanis@amazon.com>
✅ License Header Check PassedAll newly added files have proper license headers. Great work! 🎉 |
Signed-off-by: Manisha Yadav <yavmanis@amazon.com>
Fixed License header violations by adding the headers in the yaml files |
|
As per the discussion with the David during the call, he will revisit the issue details for the add_entries processor enhancement and analyze Problem 2 and its corresponding Fix 2. He will then advise on how he would like to proceed with this enhancement, after which we will move forward with the PR. |
| @AlsoRequired(values = { | ||
| @AlsoRequired.Required(name="iterate_on") | ||
| }) | ||
| private boolean disableRootKeys = true; |
There was a problem hiding this comment.
I would suggest using a positive field name as well, like resolve_from_root, use_root_context, etc. so that: 1) the default value would be false; 2) it's easier for users to comprehend and use
| import org.opensearch.dataprepper.test.plugins.PluginConfigurationFile; | ||
| import org.opensearch.dataprepper.test.plugins.junit.BaseDataPrepperPluginStandardTestSuite; | ||
|
|
||
| import java.util.*; |
There was a problem hiding this comment.
Star import may result in checkstyle failures. I triggered the checks, please check that gradle builds are OK.
Description
This PR addresses two limitations in the
add_entriesprocessor when usingiterate_on:Problem 1: No root-level field access during array iteration
When
iterate_onis configured,value_expressionandformatresolve against the individual array element context. There is no way to reference root-level fields from the event, so expressions like/alert_titlereturn null ifalert_titleexists only at the root.Problem 2:
add_to_element_whenevaluates against root eventThe
add_to_element_whencondition is evaluated against the root event, not the individual array element. This makes per-element conditional logic impossible — e.g., you cannot add a key only to elements whereseverity == "critical".Solution
Fix 1:
disable_root_keys(default:true)When set to
falsewithiterate_on, resolvesvalue_expressionandformatagainst the root event instead of the array element. This allows injecting root-level fields into each element during iteration.Fix 2:
evaluate_when_on_element(default:false)When set to
truewithiterate_onandadd_to_element_when, evaluates theadd_to_element_whencondition against each individual array element instead of the root event.Example
Input:
Config:
Output:
Backward Compatibility
Both flags default to values that preserve existing behavior. No existing constructors, tests, or functionality are modified.
Default values for the flags:
Issues Resolved
Resolves #6609
Check List
--signoffBy 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.