fix for ERROR alert state generation in doc-level monitors #768

sbcd90 · 2023-01-27T03:56:16Z

Signed-off-by: Subhobrata Dey sbcd90@gmail.com

Issue #, if available:

Description of changes:
fix for ERROR alert state generation in doc-level monitors

CheckList:
[X] 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.

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

codecov-commenter · 2023-01-27T04:11:05Z

Codecov Report

Merging #768 (2ecde35) into main (780744f) will increase coverage by 0.08%.
The diff coverage is 83.33%.

📣 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     #768      +/-   ##
============================================
+ Coverage     75.43%   75.52%   +0.08%     
  Complexity      116      116              
============================================
  Files           125      125              
  Lines          6828     6835       +7     
  Branches       1024     1026       +2     
============================================
+ Hits           5151     5162      +11     
- Misses         1143     1146       +3     
+ Partials        534      527       -7

Impacted Files	Coverage Δ
.../opensearch/alerting/DocumentLevelMonitorRunner.kt	`79.51% <82.35%> (+1.93%)`	⬆️
...rch/alerting/transport/TransportGetAlertsAction.kt	`75.51% <100.00%> (ø)`
.../alerting/transport/TransportIndexMonitorAction.kt	`54.93% <0.00%> (-2.97%)`	⬇️
.../kotlin/org/opensearch/alerting/util/IndexUtils.kt	`70.83% <0.00%> (-2.09%)`	⬇️
...ain/kotlin/org/opensearch/alerting/AlertService.kt	`80.00% <0.00%> (+0.93%)`	⬆️
.../org/opensearch/alerting/model/MonitorRunResult.kt	`87.12% <0.00%> (+2.97%)`	⬆️
...tlin/org/opensearch/alerting/util/AlertingUtils.kt	`68.75% <0.00%> (+15.62%)`	⬆️

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

eirsep · 2023-01-27T19:19:01Z

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

@@ -265,6 +264,16 @@ object DocumentLevelMonitorRunner : MonitorRunner() {
            alerts.add(alert)
        }

+        if (findingDocPairs.isEmpty() && monitorResult.error != null) {


why are we checking if findingDocPairs.isEmpty() ?
What happens is findings is not empty and there is monitorResult error?

if findingDocPairs is not empty, an alert will still be generated with ERROR state due to the logic here https://github.com/opensearch-project/alerting/blob/2ecde357581a968195d1a804c601361743084688/alerting/src/main/kotlin/org/opensearch/alerting/DocumentLevelMonitorRunner.kt#L257-#L265.

eirsep · 2023-01-27T19:20:04Z

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

@@ -109,6 +101,14 @@ object DocumentLevelMonitorRunner : MonitorRunner() {
        val docsToQueries = mutableMapOf<String, MutableList<String>>()

        try {
+            monitorCtx.docLevelMonitorQueries!!.initDocLevelQueryIndex(monitor.dataSources)


why did we shift this snippet?

the method monitorCtx.docLevelMonitorQueries!!.indexDocLevelQueries assumes that the customer provided log index always exist otherwise it throws an Unhandled error.

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

Line 105 in 2ecde35

monitorCtx.docLevelMonitorQueries!!.indexDocLevelQueries(

So, moved it into the try block, so that we always get an alert with ERROR state for any unhandled errors.

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> (cherry picked from commit daf8c7c)

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> (cherry picked from commit a69d762)

…) (#772) Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> (cherry picked from commit a69d762) Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>

…h-project#768) Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

* Added exception check once the .opendistro-alerting-config index is b… (#650) * Added exception check once the .opendistro-alerting-config index is being created During .opendistro-alerting-config index creation, if ResourceAlreadyExists exception is being raised, the flow will check first if the index is in yellow state and then it will re-try to index monitor Signed-off-by: Stevan Buzejic <buzejic.stevan@gmail.com> * Formating of the file fixed Signed-off-by: Stevan Buzejic <buzejic.stevan@gmail.com> Signed-off-by: Stevan Buzejic <buzejic.stevan@gmail.com> * refactored DeleteMonitor Action to be synchronious (#628) (#630) * refactored DeleteMonitor Action to be synchronious Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Co-authored-by: Petar Dzepina <petar.dzepina@gmail.com> * [Backport 2.x] QueryIndex rollover when field mapping limit is reached (#729) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * Mappings fix backport 2.x (#730) * Added support for "nested" mappings (#645) * example Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * fixed updating mappings for queryIndex Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * mappings traversal bug fix (#669) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * Added unwrapping exception from core; added more debug logs (#728) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * Add DataSources test for future backports Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> * fix percolator mapping error when having field name 'type' (#726) Signed-off-by: Raj Chakravarthi <raj@icedome.ca> * [BUG] ExecuteMonitor inserting metadata doc during dry run (#758) * execute monitor bugfix Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * added IT Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * fixed created retval when skipIndex=true Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> --------- Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * fix for ERROR alert state generation in doc-level monitors (#768) Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> * Adjusting max field index setting dynamically for query index (#776) * added adjusting max field index setting dynamicly for query index Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * Multiple indices support in DocLevelMonitorInput (#784) Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * Doc transform 2.x backport (#853) * conflict resovle - backport from main to 2.x Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * fixed module class names Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> --------- Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * Update config index schema if needed at the start of each monitor execution (#849) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> * Mappings parsing fix (#851) * fixed mappings parsing when field name named "properties" exists in mappings Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * message typo fix Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> --------- Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * [Backport 2.x] Notification security fix (#861) * Notification security fix (#852) * added injecting whole user object in threadContext before calling notification APIs so that backend roles are available to notification plugin Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * compile fix Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * refactored user_info injection to use InjectSecurity Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> * ktlint fix Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> --------- Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> (cherry picked from commit e0b7a5a) * remove unneeded import Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> --------- Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> Co-authored-by: Petar Dzepina <petar.dzepina@gmail.com> Co-authored-by: Ashish Agrawal <ashisagr@amazon.com> * Fixed a bug that prevented alerts from being generated for doc level monitors that use wildcard characters in index names. (#894) Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * fixed security tests (#484) (#794) * fixed security tests Signed-off-by: Raj Chakravarthi <raj@icedome.ca> (cherry picked from commit c51940f) --------- Signed-off-by: Stevan Buzejic <buzejic.stevan@gmail.com> Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com> Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> Signed-off-by: Raj Chakravarthi <raj@icedome.ca> Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> Signed-off-by: AWSHurneyt <hurneyt@amazon.com> Co-authored-by: Stevan Buzejic <30922513+stevanbz@users.noreply.github.com> Co-authored-by: Surya Sashank Nistala <snistala@amazon.com> Co-authored-by: Petar Dzepina <petar.dzepina@gmail.com> Co-authored-by: RAJ CHAKRAVARTHI <49325334+raj-chak@users.noreply.github.com> Co-authored-by: Subhobrata Dey <sbcd90@gmail.com> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: AWSHurneyt <hurneyt@amazon.com> Co-authored-by: RAJ CHAKRAVARTHI <raj@icedome.ca>

fix for ERROR alert state generation in doc-level monitors

2ecde35

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

sbcd90 requested a review from a team January 27, 2023 03:56

sbcd90 requested review from eirsep and lezzago January 27, 2023 06:01

sbcd90 mentioned this pull request Jan 27, 2023

[BUG] Bucket-Level monitor doesn't generate alerts in ERROR state #769

Open

eirsep reviewed Jan 27, 2023

View reviewed changes

eirsep approved these changes Jan 27, 2023

View reviewed changes

rishabhmaurya approved these changes Jan 27, 2023

View reviewed changes

sbcd90 added the backport 2.x label Jan 27, 2023

sbcd90 merged commit daf8c7c into opensearch-project:main Jan 27, 2023

opensearch-trigger-bot bot mentioned this pull request Jan 27, 2023

[Backport 2.x] fix for ERROR alert state generation in doc-level monitors #771

Merged

opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 27, 2023

fix for ERROR alert state generation in doc-level monitors (#768)

5abba1e

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> (cherry picked from commit daf8c7c)

sbcd90 pushed a commit that referenced this pull request Jan 30, 2023

fix for ERROR alert state generation in doc-level monitors (#768) (#771)

a69d762

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

opensearch-trigger-bot bot added a commit that referenced this pull request Jan 30, 2023

fix for ERROR alert state generation in doc-level monitors (#768) (#771)

06d281f

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com> (cherry picked from commit a69d762)

AWSHurneyt mentioned this pull request Jun 6, 2023

[Backport 2.5] Manual backports of several PRs #957

Merged

lezzago pushed a commit to lezzago/alerting-opensearch that referenced this pull request Jun 8, 2023

fix for ERROR alert state generation in doc-level monitors (opensearc…

60cc9dc

…h-project#768) Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

lezzago mentioned this pull request Jun 8, 2023

[Backport 2.3] Manual backports of several PRs #960

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix for ERROR alert state generation in doc-level monitors #768

fix for ERROR alert state generation in doc-level monitors #768

sbcd90 commented Jan 27, 2023

codecov-commenter commented Jan 27, 2023 •

edited

Loading

eirsep Jan 27, 2023

sbcd90 Jan 27, 2023

eirsep Jan 27, 2023

sbcd90 Jan 27, 2023

fix for ERROR alert state generation in doc-level monitors #768

fix for ERROR alert state generation in doc-level monitors #768

Conversation

sbcd90 commented Jan 27, 2023

codecov-commenter commented Jan 27, 2023 • edited Loading

Codecov Report

eirsep Jan 27, 2023

Choose a reason for hiding this comment

sbcd90 Jan 27, 2023

Choose a reason for hiding this comment

eirsep Jan 27, 2023

Choose a reason for hiding this comment

sbcd90 Jan 27, 2023

Choose a reason for hiding this comment

codecov-commenter commented Jan 27, 2023 •

edited

Loading