Fixed transformation of document field names before running percolator search #845

petardz · 2023-03-31T00:11:22Z

Issue #, if available: #844

Description of changes:

Fixed transformation of document field names before running percolator search.

Documents which are ingested with "nested fields" weren't handled properly. Example:

"field1": {
   "field2": 12345
}

CheckList:
[ ] Commits are signed per the DCO using --signoff

By 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.

…r search Signed-off-by: Petar Dzepina <[email protected]>

Signed-off-by: Petar Dzepina <[email protected]>

… of outputing flatten json Signed-off-by: Petar Dzepina <[email protected]>

Signed-off-by: Petar Dzepina <[email protected]>

alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt

eirsep · 2023-03-31T21:01:54Z

#844

eirsep · 2023-03-31T21:03:09Z

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt


            val sourceRef = BytesReference.bytes(xContentBuilder)

            Pair(hit.id, sourceRef)
        }
    }
+
+    /**
+     * Traverses document fields in leaves recursively and appends [fieldNameSuffix] to field names.


mention that we are trying to flatten the object type fields for percolate query

Actually, we can't flatten it, because of type=nested. You can't insert document with flatten fields if type is nested. For example, let's say field "rule" is type=nested, then this is invalid:

{ "rule.category": "windows" }

but this is valid:

{ "rule" : { "category": "windows" } }

i am not talking about type=nested

eirsep · 2023-03-31T21:04:11Z

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

+     * @param jsonAsMap               Input JSON (as Map)
+     * @param fieldNameSuffix         Field suffix which is appended to existing field name
+     */
+    private fun transformDocumentFieldNames(


what happens if customer has a nested type mapping for the index.

plz add a test case to verify we are handling that and fix it if required.

There is a test case with document with nested fields: "test execute monitor with custom query index and nested mappings"

eirsep · 2023-03-31T21:06:29Z

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

+            if (entry.value is Map<*, *>) {
+                transformDocumentFieldNames(entry.value as MutableMap<String, Any>, fieldNameSuffix)
+            } else if (entry.key.endsWith(fieldNameSuffix) == false) {
+                tempMap["${entry.key}_$fieldNameSuffix"] = entry.value


add debug logs with field name generated and mention percolate query stage, monitor id

we are lacking insights into what is going wrong
this might help

Added trace log here printing whole document

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

eirsep

Thanks for this important fix Petar,
added a few comments reg. testing, logging and verbose comments/documentation

Signed-off-by: Petar Dzepina <[email protected]>

codecov-commenter · 2023-04-03T16:47:37Z

Codecov Report

Merging #845 (a5aa476) into main (3f2e651) will increase coverage by 0.09%.
The diff coverage is 88.88%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main     #845      +/-   ##
============================================
+ Coverage     75.46%   75.55%   +0.09%     
+ Complexity      111      110       -1     
============================================
  Files           125      125              
  Lines          6885     6886       +1     
  Branches       1034     1036       +2     
============================================
+ Hits           5196     5203       +7     
+ Misses         1158     1154       -4     
+ Partials        531      529       -2

Impacted Files	Coverage Δ
.../opensearch/alerting/DocumentLevelMonitorRunner.kt	`83.38% <88.88%> (+3.11%)`	⬆️

... and 8 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Petar Dzepina <[email protected]>

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

lezzago · 2023-04-04T15:28:08Z

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

+     *
+     * Example for index name is my_log_index and Monitor ID is TReewWdsf2gdJFV:
+     * {                         {
+     *   "a": {                     "a": {


We do not want a to be appended with my_log_index_TReewWdsf2gdJFV?

We don't, since we transform mappings in the same way

alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt

Signed-off-by: Petar Dzepina <[email protected]>

sbcd90 · 2023-04-06T16:43:04Z

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

+        while (it.hasNext()) {
+            val entry = it.next()
+            if (entry.value is Map<*, *>) {
+                transformDocumentFieldNames(entry.value as MutableMap<String, Any>, fieldNameSuffix)


unchecked type cast happens here. please use @Suppress("UNCHECKED_CAST")

sbcd90 · 2023-04-06T16:43:28Z

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt

+            val entry = it.next()
+            if (entry.value is Map<*, *>) {
+                transformDocumentFieldNames(entry.value as MutableMap<String, Any>, fieldNameSuffix)
+            } else if (entry.key.endsWith(fieldNameSuffix) == false) {


you can also make it !entry.key.endsWith(fieldNameSuffix)

sbcd90 · 2023-04-06T16:43:57Z

the pr looks good.

opensearch-trigger-bot · 2023-04-06T18:24:57Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-845-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 702da92495481bebfc18dbee2de9544a4245759a
# Push it to GitHub
git push --set-upstream origin backport/backport-845-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-845-to-2.x.

opensearch-trigger-bot · 2023-04-06T18:24:58Z

The backport to 2.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.3 2.3
# Navigate to the new working tree
cd .worktrees/backport-2.3
# Create a new branch
git switch --create backport/backport-845-to-2.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 702da92495481bebfc18dbee2de9544a4245759a
# Push it to GitHub
git push --set-upstream origin backport/backport-845-to-2.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.3

Then, create a pull request where the base branch is 2.3 and the compare/head branch is backport/backport-845-to-2.3.

opensearch-trigger-bot · 2023-04-06T18:24:59Z

The backport to 2.4 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.4 2.4
# Navigate to the new working tree
cd .worktrees/backport-2.4
# Create a new branch
git switch --create backport/backport-845-to-2.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 702da92495481bebfc18dbee2de9544a4245759a
# Push it to GitHub
git push --set-upstream origin backport/backport-845-to-2.4
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.4

Then, create a pull request where the base branch is 2.4 and the compare/head branch is backport/backport-845-to-2.4.

opensearch-trigger-bot · 2023-04-06T18:25:00Z

The backport to 2.5 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.5 2.5
# Navigate to the new working tree
cd .worktrees/backport-2.5
# Create a new branch
git switch --create backport/backport-845-to-2.5
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 702da92495481bebfc18dbee2de9544a4245759a
# Push it to GitHub
git push --set-upstream origin backport/backport-845-to-2.5
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.5

Then, create a pull request where the base branch is 2.5 and the compare/head branch is backport/backport-845-to-2.5.

opensearch-trigger-bot · 2023-04-06T18:25:01Z

The backport to 2.6 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.6 2.6
# Navigate to the new working tree
cd .worktrees/backport-2.6
# Create a new branch
git switch --create backport/backport-845-to-2.6
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 702da92495481bebfc18dbee2de9544a4245759a
# Push it to GitHub
git push --set-upstream origin backport/backport-845-to-2.6
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.6

Then, create a pull request where the base branch is 2.6 and the compare/head branch is backport/backport-845-to-2.6.

…r search (opensearch-project#845) * Fixed transformation of document field names before running percolator search Signed-off-by: Petar Dzepina <[email protected]> (cherry picked from commit 702da92)

* Fixed transformation of document field names before running percolator search (#845) * Fixed transformation of document field names before running percolator search Signed-off-by: Petar Dzepina <[email protected]> (cherry picked from commit 702da92) * fixed module class names Signed-off-by: Petar Dzepina <[email protected]> --------- Signed-off-by: Petar Dzepina <[email protected]>

Fixed transformation of document field names before running percolato…

82278cf

…r search Signed-off-by: Petar Dzepina <[email protected]>

petardz requested a review from a team March 31, 2023 00:11

petardz added 3 commits March 31, 2023 02:13

imports fix

9898aa1

Signed-off-by: Petar Dzepina <[email protected]>

refactored transformation function to do "in-place" transform instead…

301bc75

… of outputing flatten json Signed-off-by: Petar Dzepina <[email protected]>

import lint fix

7aa57c3

Signed-off-by: Petar Dzepina <[email protected]>

eirsep reviewed Mar 31, 2023

View reviewed changes

alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt Outdated Show resolved Hide resolved

eirsep reviewed Mar 31, 2023

View reviewed changes

alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt Show resolved Hide resolved

eirsep reviewed Mar 31, 2023

View reviewed changes

alerting/src/test/kotlin/org/opensearch/alerting/MonitorDataSourcesIT.kt Show resolved Hide resolved

eirsep reviewed Mar 31, 2023

View reviewed changes

alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt Outdated Show resolved Hide resolved

eirsep reviewed Mar 31, 2023

View reviewed changes

few small fixes

a5aa476

Signed-off-by: Petar Dzepina <[email protected]>

Added more comments; added test with parent-join docs

62f8385

Signed-off-by: Petar Dzepina <[email protected]>

This was referenced Apr 4, 2023

[FEATURE] SigmaRule Parser - Remove replacing "." to "_" in field names opensearch-project/security-analytics#375

Closed

[BUG] No findings with test detectors opensearch-project/security-analytics#368

Closed

lezzago requested changes Apr 4, 2023

View reviewed changes

added test with nested mappings/doc; moved doc log to debug level

9e67c3e

Signed-off-by: Petar Dzepina <[email protected]>

sbcd90 reviewed Apr 6, 2023

View reviewed changes

sbcd90 approved these changes Apr 6, 2023

View reviewed changes

lezzago approved these changes Apr 6, 2023

View reviewed changes

lezzago added backport 2.x backport 2.3 backport 2.4 backport 2.5 labels Apr 6, 2023

lezzago added the backport 2.6 label Apr 6, 2023

lezzago merged commit 702da92 into opensearch-project:main Apr 6, 2023

petardz mentioned this pull request Apr 12, 2023

doc transform 2.5 backport #854

Merged

sbcd90 mentioned this pull request May 12, 2023

packetbeat for DNS and sigma rules opensearch-project/security-analytics#429

Closed

This was referenced May 31, 2023

[Backport 2.3] Fixed transformation of document field names before running percolator search #943

Merged

[Backport 2.5] Manual backports of several PRs #957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed transformation of document field names before running percolator search #845

Fixed transformation of document field names before running percolator search #845

petardz commented Mar 31, 2023

eirsep commented Mar 31, 2023

eirsep Mar 31, 2023 •

edited

Loading

petardz Mar 31, 2023

eirsep Apr 3, 2023

eirsep Mar 31, 2023

petardz Mar 31, 2023

eirsep Mar 31, 2023

petardz Apr 3, 2023

eirsep left a comment

codecov-commenter commented Apr 3, 2023

lezzago Apr 4, 2023

petardz Apr 4, 2023

sbcd90 Apr 6, 2023

sbcd90 Apr 6, 2023

sbcd90 commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

Fixed transformation of document field names before running percolator search #845

Fixed transformation of document field names before running percolator search #845

Conversation

petardz commented Mar 31, 2023

eirsep commented Mar 31, 2023

eirsep Mar 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eirsep left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 3, 2023

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbcd90 commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

opensearch-trigger-bot bot commented Apr 6, 2023

eirsep Mar 31, 2023 •

edited

Loading