-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow updation/deletion of custom log type if custom rule index is missing #767
Allow updation/deletion of custom log type if custom rule index is missing #767
Conversation
Signed-off-by: Megha Goyal <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #767 +/- ##
============================================
+ Coverage 24.73% 24.74% +0.01%
- Complexity 1021 1023 +2
============================================
Files 275 275
Lines 12662 12689 +27
Branches 1389 1400 +11
============================================
+ Hits 3132 3140 +8
- Misses 9264 9284 +20
+ Partials 266 265 -1 ☔ View full report in Codecov by Sentry. |
} | ||
|
||
@Override | ||
public void onFailure(Exception e) { | ||
onFailures(e); | ||
if (e instanceof IndexNotFoundException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we do. an index exists
check instead of letting this fail and handling exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. Added the check but still keeping the exception check to account for edge cases.
} | ||
|
||
@Override | ||
public void onFailure(Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log exception with custom message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is calling the onFailures()
call which is logging this exception. So, skipping it here.
@@ -292,7 +284,7 @@ private void onOperation(DeleteResponse response) { | |||
} | |||
|
|||
private void onFailures(Exception t) { | |||
log.error(String.format(Locale.ROOT, "Failed to delete detector")); | |||
log.error(String.format(Locale.ROOT, "Failed to delete log type")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also log the name of the log type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception being passed would ideally have the log type. Added the stack trace to be logged here as well.
…ssing Signed-off-by: Megha Goyal <[email protected]>
Looks like delete log type is wrongly wired. If detector index does NOT exist, we seem to directly delete the log type instead of checking if there is a rule index. Line 176 in 8be2159
Lines 227 to 229 in 8be2159
Plz add check for rule index here also. extract into a method if redundant. |
@@ -547,6 +605,61 @@ public void testDeleteCustomLogTypeFailsAsDetectorExist() throws IOException { | |||
}); | |||
} | |||
|
|||
@SuppressWarnings("unchecked") | |||
public void testDeleteCustomLogTypeWithRuleIndexMissing() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add two scenarios:
deleting log type when -
- custom rules index exists, detector index exists.
- custom rules index exists, detector index does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
Signed-off-by: Megha Goyal <[email protected]>
Signed-off-by: Megha Goyal <[email protected]>
The backport to
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-767-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-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 |
The backport to
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.11 2.11
# Navigate to the new working tree
cd .worktrees/backport-2.11
# Create a new branch
git switch --create backport/backport-767-to-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-to-2.11
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.11 Then, create a pull request where the |
The backport to
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.11-with-features 2.11-with-features
# Navigate to the new working tree
cd .worktrees/backport-2.11-with-features
# Create a new branch
git switch --create backport/backport-767-to-2.11-with-features
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-to-2.11-with-features
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.11-with-features Then, create a pull request where the |
…ssing (opensearch-project#767) Signed-off-by: Megha Goyal <[email protected]>
…ensearch-project#767) * Allow deletion of custom log type if custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Allow custom log type name to be updated when custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Adding changes for detector index missing Signed-off-by: Megha Goyal <[email protected]> * Update log type when detector index is missing Signed-off-by: Megha Goyal <[email protected]> --------- Signed-off-by: Megha Goyal <[email protected]>
…ensearch-project#767) * Allow deletion of custom log type if custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Allow custom log type name to be updated when custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Adding changes for detector index missing Signed-off-by: Megha Goyal <[email protected]> * Update log type when detector index is missing Signed-off-by: Megha Goyal <[email protected]> --------- Signed-off-by: Megha Goyal <[email protected]>
…) (#780) * Allow deletion of custom log type if custom rule index is missing * Allow custom log type name to be updated when custom rule index is missing * Adding changes for detector index missing * Update log type when detector index is missing --------- Signed-off-by: Megha Goyal <[email protected]>
…ssing (#767) (#779) Signed-off-by: Megha Goyal <[email protected]>
The backport to
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.9 2.9
# Navigate to the new working tree
cd .worktrees/backport-2.9
# Create a new branch
git switch --create backport/backport-767-to-2.9
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-to-2.9
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.9 Then, create a pull request where the |
The backport to
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.9-with-features 2.9-with-features
# Navigate to the new working tree
cd .worktrees/backport-2.9-with-features
# Create a new branch
git switch --create backport/backport-767-to-2.9-with-features
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-to-2.9-with-features
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.9-with-features Then, create a pull request where the |
…ensearch-project#767) * Allow deletion of custom log type if custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Allow custom log type name to be updated when custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Adding changes for detector index missing Signed-off-by: Megha Goyal <[email protected]> * Update log type when detector index is missing Signed-off-by: Megha Goyal <[email protected]> --------- Signed-off-by: Megha Goyal <[email protected]>
…) (#814) * Allow deletion of custom log type if custom rule index is missing * Allow custom log type name to be updated when custom rule index is missing * Adding changes for detector index missing * Update log type when detector index is missing --------- Signed-off-by: Megha Goyal <[email protected]>
The backport to
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.7 2.7
# Navigate to the new working tree
cd .worktrees/backport-2.7
# Create a new branch
git switch --create backport/backport-767-to-2.7
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-to-2.7
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.7 Then, create a pull request where the |
The backport to
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-767-to-2.5
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c373343e1981af0bf2795aa2d0ac45f301d8ac3e
# Push it to GitHub
git push --set-upstream origin backport/backport-767-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 |
* Allow deletion of custom log type if custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Allow custom log type name to be updated when custom rule index is missing Signed-off-by: Megha Goyal <[email protected]> * Adding changes for detector index missing Signed-off-by: Megha Goyal <[email protected]> * Update log type when detector index is missing Signed-off-by: Megha Goyal <[email protected]> --------- Signed-off-by: Megha Goyal <[email protected]>
Description
In the case when the custom rule index is missing, we want to allow the update/deletion of custom log types to be successful instead of throwing an error. The detector index may not be present, and the custom log types will still get updated/deleted respectively.
Issues Resolved
#700
Check List
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.