support list of monitor ids in Chained Monitor Findings #514
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
eirsep
requested review from
lezzago,
qreshi,
bowenlan-amzn,
rishabhmaurya,
tlfeng,
getsaurabh02,
sbcd90 and
AWSHurneyt
as code owners
Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #514 +/- ##
============================================
+ Coverage 74.60% 74.63% +0.02%
- Complexity 868 872 +4
============================================
Files 130 130
Lines 5632 5662 +30
Branches 689 697 +8
============================================
+ Hits 4202 4226 +24
- Misses 1124 1130 +6
Partials 306 306
|
lezzago
previously approved these changes
Aug 30, 2023
sbcd90
reviewed
Aug 31, 2023
| @@ -30,7 +30,7 @@ class IndexWorkflowRequest : ActionRequest { | |||
| refreshPolicy: WriteRequest.RefreshPolicy, | |||
| method: RestRequest.Method, | |||
| workflow: Workflow, | |||
| rbacRoles: List<String>? = null | |||
| rbacRoles: List<String>? = null, | |||
sbcd90
reviewed
Aug 31, 2023
| } | ||
|
|
||
| @Throws(IOException::class) | ||
| constructor(sin: StreamInput) : this( | ||
| sin.readString(), // monitorId | ||
| sin.readOptionalString(), // monitorId | ||
| Collections.unmodifiableList(sin.readStringList()) |
There was a problem hiding this comment.
can we use kotlin toList here?
There was a problem hiding this comment.
Any specific reason to use toList?
Collections.unmodifiableList has been used across the code base to wrap string lists read from stream input
sbcd90
reviewed
Aug 31, 2023
| fun asTemplateArg(): Map<String, Any> { | ||
| return mapOf( | ||
| MONITOR_ID_FIELD to monitorId, | ||
| ) | ||
| MONITOR_IDS_FIELD to monitorIds |
There was a problem hiding this comment.
is it possible we merge these 2 fields to 1? it looks little complex that we have both
MONITOR_ID_FIELD to monitorId,
MONITOR_IDS_FIELD to monitorIds```
There was a problem hiding this comment.
we need to have both for backward compatibility.
Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
sbcd90
approved these changes
Aug 31, 2023
lezzago
approved these changes
Aug 31, 2023
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/common-utils/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/common-utils/backport-2.x
# Create a new branch
git switch --create backport-514-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 7736e080f9b0a2f6f2d9ad3b4272090b281ea8fa
# Push it to GitHub
git push --set-upstream origin backport-514-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/common-utils/backport-2.xThen, create a pull request where the |
eirsep
added a commit
to eirsep/common-utils
that referenced
this pull request
Sep 1, 2023
…roject#514) support list of monitor ids in Chained Monitor Findings Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
eirsep
added a commit
that referenced
this pull request
Sep 1, 2023
AWSHurneyt
pushed a commit
to AWSHurneyt/common-utils
that referenced
this pull request
Apr 12, 2024
…roject#514) (opensearch-project#515) support list of monitor ids in Chained Monitor Findings Signed-off-by: Surya Sashank Nistala <snistala@amazon.com> Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for passing a list monitor ids in chained monitor findings. This helps filter data of a monitor in workflow based on findings of multiple monitors instead of a single monitor.
If workflow sequence looks like:
[m1] -> [m2] -> [m3 (chained findings of m1, m2)]
this means m3's data input would be the related doc ids from findings of m1 and m2.
for backward compatibility reasons, the monitorId field would be given preference over the new list type field, monitorIds, if both are provided.