Update config index schema if needed at the start of each monitor execution #849

lezzago · 2023-04-04T16:58:20Z

Issue #, if available:
#841
Description of changes:
Update config index schema if needed at the start of each monitor execution. This fixes issues when upgrading the cluster where the index mapping changed and the user has not indexed or updated a new monitor yet.
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.

…cution Signed-off-by: Ashish Agrawal <[email protected]>

codecov-commenter · 2023-04-04T17:11:41Z

Codecov Report

Merging #849 (cea9b9e) into main (3f2e651) will increase coverage by 0.01%.
The diff coverage is 0.00%.

📣 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     #849      +/-   ##
============================================
+ Coverage     75.46%   75.47%   +0.01%     
  Complexity      111      111              
============================================
  Files           125      125              
  Lines          6885     6892       +7     
  Branches       1034     1037       +3     
============================================
+ Hits           5196     5202       +6     
- Misses         1158     1160       +2     
+ Partials        531      530       -1

Impacted Files	Coverage Δ
...in/org/opensearch/alerting/MonitorRunnerService.kt	`75.47% <0.00%> (-6.53%)`	⬇️

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

getsaurabh02 · 2023-04-05T02:42:18Z

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

+
+        // Updating the scheduled job index at the start of monitor execution runs for when there is an upgrade the the schema mapping
+        // has not been updated.
+        if (!IndexUtils.scheduledJobIndexUpdated && monitorCtx.clusterService != null && monitorCtx.client != null) {


Do we need to add tests to validate this execution flow?

It would have to be a part of this issue: #848.

We have no framework for this setup

eirsep · 2023-04-12T18:38:12Z

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

+
+        // Updating the scheduled job index at the start of monitor execution runs for when there is an upgrade the the schema mapping
+        // has not been updated.
+        if (!IndexUtils.scheduledJobIndexUpdated && monitorCtx.clusterService != null && monitorCtx.client != null) {


Why is this not being done at node start up?

scheduled jobs index schema shouldnt be handled inside a scheduled job. Tomorrow when we have workflows this might become a miss. Let's handle it at a holistic level

The only place if we do it at node startup would be in the Alerting plugin class, but it would not work there since to make some of the calls to check the index would not work as the node is still setting up services. I felt it was too much to startup another service from the Alerting plugin class to update the config schema if needed. I believe we can just have the same logic in the workflow runner service to handle this. Additionally the workflow runner service and monitor runner service have very similar logic so maybe parts of it can be refactored to reduce redundant code.

eirsep

Minor comment

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

Signed-off-by: Ashish Agrawal <[email protected]>

getsaurabh02 · 2023-04-17T16:48:17Z

Looks like bwc test is failing

It has been addressedd

…cution (#849) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <[email protected]> (cherry picked from commit 21aeb3c)

…cution (#849) (#866) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <[email protected]> (cherry picked from commit 21aeb3c) Co-authored-by: Ashish Agrawal <[email protected]>

…cution (#849) (#869) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <[email protected]> (cherry picked from commit 21aeb3c) Co-authored-by: Ashish Agrawal <[email protected]>

…cution (#849) (#867) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <[email protected]> (cherry picked from commit 21aeb3c) Co-authored-by: Ashish Agrawal <[email protected]>

…cution (opensearch-project#849) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <[email protected]>

* 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 <[email protected]> * Formating of the file fixed Signed-off-by: Stevan Buzejic <[email protected]> Signed-off-by: Stevan Buzejic <[email protected]> * refactored DeleteMonitor Action to be synchronious (#628) (#630) * refactored DeleteMonitor Action to be synchronious Signed-off-by: Petar Dzepina <[email protected]> Signed-off-by: Petar Dzepina <[email protected]> Co-authored-by: Petar Dzepina <[email protected]> * [Backport 2.x] QueryIndex rollover when field mapping limit is reached (#729) Signed-off-by: Petar Dzepina <[email protected]> * Mappings fix backport 2.x (#730) * Added support for "nested" mappings (#645) * example Signed-off-by: Petar Dzepina <[email protected]> * fixed updating mappings for queryIndex Signed-off-by: Petar Dzepina <[email protected]> Signed-off-by: Petar Dzepina <[email protected]> * mappings traversal bug fix (#669) Signed-off-by: Petar Dzepina <[email protected]> Signed-off-by: Petar Dzepina <[email protected]> * Added unwrapping exception from core; added more debug logs (#728) Signed-off-by: Petar Dzepina <[email protected]> * Add DataSources test for future backports Signed-off-by: Ashish Agrawal <[email protected]> * fix percolator mapping error when having field name 'type' (#726) Signed-off-by: Raj Chakravarthi <[email protected]> * [BUG] ExecuteMonitor inserting metadata doc during dry run (#758) * execute monitor bugfix Signed-off-by: Petar Dzepina <[email protected]> * added IT Signed-off-by: Petar Dzepina <[email protected]> * fixed created retval when skipIndex=true Signed-off-by: Petar Dzepina <[email protected]> --------- Signed-off-by: Petar Dzepina <[email protected]> * fix for ERROR alert state generation in doc-level monitors (#768) Signed-off-by: Subhobrata Dey <[email protected]> * 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 <[email protected]> * Multiple indices support in DocLevelMonitorInput (#784) Signed-off-by: Petar Dzepina <[email protected]> * Doc transform 2.x backport (#853) * conflict resovle - backport from main to 2.x Signed-off-by: Petar Dzepina <[email protected]> * fixed module class names Signed-off-by: Petar Dzepina <[email protected]> --------- Signed-off-by: Petar Dzepina <[email protected]> * 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 <[email protected]> * Mappings parsing fix (#851) * fixed mappings parsing when field name named "properties" exists in mappings Signed-off-by: Petar Dzepina <[email protected]> * message typo fix Signed-off-by: Petar Dzepina <[email protected]> --------- Signed-off-by: Petar Dzepina <[email protected]> * [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 <[email protected]> * compile fix Signed-off-by: Petar Dzepina <[email protected]> * refactored user_info injection to use InjectSecurity Signed-off-by: Petar Dzepina <[email protected]> * ktlint fix Signed-off-by: Petar Dzepina <[email protected]> --------- Signed-off-by: Petar Dzepina <[email protected]> (cherry picked from commit e0b7a5a) * remove unneeded import Signed-off-by: Ashish Agrawal <[email protected]> --------- Signed-off-by: Ashish Agrawal <[email protected]> Co-authored-by: Petar Dzepina <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]> * 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 <[email protected]> * fixed security tests (#484) (#794) * fixed security tests Signed-off-by: Raj Chakravarthi <[email protected]> (cherry picked from commit c51940f) --------- Signed-off-by: Stevan Buzejic <[email protected]> Signed-off-by: Petar Dzepina <[email protected]> Signed-off-by: Ashish Agrawal <[email protected]> Signed-off-by: Raj Chakravarthi <[email protected]> Signed-off-by: Subhobrata Dey <[email protected]> Signed-off-by: AWSHurneyt <[email protected]> Co-authored-by: Stevan Buzejic <[email protected]> Co-authored-by: Surya Sashank Nistala <[email protected]> Co-authored-by: Petar Dzepina <[email protected]> Co-authored-by: RAJ CHAKRAVARTHI <[email protected]> Co-authored-by: Subhobrata Dey <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: AWSHurneyt <[email protected]> Co-authored-by: RAJ CHAKRAVARTHI <[email protected]>

…cution (#849) (#868) * Update config index schema if needed at the start of each monitor execution Signed-off-by: Ashish Agrawal <[email protected]> (cherry picked from commit 21aeb3c) Co-authored-by: Ashish Agrawal <[email protected]>

Update config index schema if needed at the start of each monitor exe…

d3b88ce

…cution Signed-off-by: Ashish Agrawal <[email protected]>

lezzago added backport 2.x backport 2.5 backport 2.6 labels Apr 4, 2023

lezzago requested a review from a team April 4, 2023 16:58

getsaurabh02 reviewed Apr 5, 2023

View reviewed changes

eirsep reviewed Apr 12, 2023

View reviewed changes

eirsep previously requested changes Apr 12, 2023

View reviewed changes

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

lezzago added 4 commits April 13, 2023 15:07

move logic at node startup

ee99f5a

Signed-off-by: Ashish Agrawal <[email protected]>

fix ktlint

73c4f83

Signed-off-by: Ashish Agrawal <[email protected]>

undo other changes

d9ae868

Signed-off-by: Ashish Agrawal <[email protected]>

remove added logger

cea9b9e

Signed-off-by: Ashish Agrawal <[email protected]>

lezzago requested review from eirsep and getsaurabh02 April 17, 2023 16:16

getsaurabh02 approved these changes Apr 18, 2023

View reviewed changes

AWSHurneyt approved these changes Apr 18, 2023

View reviewed changes

AWSHurneyt added the backport 2.7 label Apr 18, 2023

lezzago merged commit 21aeb3c into opensearch-project:main Apr 18, 2023

opensearch-trigger-bot bot mentioned this pull request Apr 18, 2023

[Backport 2.x] Update config index schema if needed at the start of each monitor execution #866

Merged

opensearch-trigger-bot bot mentioned this pull request Apr 18, 2023

[Backport 2.5] Update config index schema if needed at the start of each monitor execution #867

Merged

opensearch-trigger-bot bot mentioned this pull request Apr 18, 2023

[Backport 2.6] Update config index schema if needed at the start of each monitor execution #868

Merged

opensearch-trigger-bot bot mentioned this pull request Apr 18, 2023

[Backport 2.7] Update config index schema if needed at the start of each monitor execution #869

Merged

AWSHurneyt mentioned this pull request Jun 6, 2023

[Backport 2.5] Manual backports of several PRs #957

Merged

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

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 #849

lezzago commented Apr 4, 2023

codecov-commenter commented Apr 4, 2023 •

edited

Loading

getsaurabh02 Apr 5, 2023

lezzago Apr 5, 2023

eirsep Apr 12, 2023

eirsep Apr 12, 2023

lezzago Apr 14, 2023

eirsep left a comment

getsaurabh02 commented Apr 17, 2023

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 #849

Conversation

lezzago commented Apr 4, 2023

codecov-commenter commented Apr 4, 2023 • edited Loading

Codecov Report

getsaurabh02 Apr 5, 2023

Choose a reason for hiding this comment

lezzago Apr 5, 2023

Choose a reason for hiding this comment

eirsep Apr 12, 2023

Choose a reason for hiding this comment

eirsep Apr 12, 2023

Choose a reason for hiding this comment

lezzago Apr 14, 2023

Choose a reason for hiding this comment

eirsep left a comment

Choose a reason for hiding this comment

getsaurabh02 commented Apr 17, 2023

codecov-commenter commented Apr 4, 2023 •

edited

Loading