PPL Alerting: Get and Search Monitors #1966

toepkerd · 2025-10-30T16:15:32Z

Description

Get and Search Monitor functionality for PPL Alerting

Related Issues

Check List

[Y] New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
[Y] Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

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: Dennis Toepker <[email protected]>

lezzago · 2025-10-30T16:55:09Z

alerting/src/main/kotlin/org/opensearch/alerting/resthandlerv2/RestGetMonitorV2Action.kt

+        if (request.method() == HEAD) {
+            srcContext = FetchSourceContext.DO_NOT_FETCH_SOURCE
+        }


What is the purpose for head here?

If this is copying v1 monitors for UI setup, does the new V2 UI even use this?

This case handling is ported over from Alerting V1, where it helped with regenerating the visual basic mode of monitors. This use case doesn't immediately apply to Alerting V2, but the logic will be retained so Alerting V2 can catch this potential curve ball in the future.

lezzago · 2025-10-30T18:23:36Z

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorV2RestApiIT.kt

+ * ./gradlew :alerting:integTest -Dhttps=true -Dsecurity=true -Duser=admin -Dpassword=admin \
+ * --tests "org.opensearch.alerting.resthandler.SecureMonitorV2RestApiIT"
+ */
+class SecureMonitorV2RestApiIT : AlertingRestTestCase() {


Can we also test update monitor on just cluster permissions and a user trying to update a monitor (change a trigger name, but not the index) that monitors an index that they do not have permissions to

Signed-off-by: Dennis Toepker <[email protected]>

AWSHurneyt · 2025-10-30T23:03:39Z

alerting/src/main/kotlin/org/opensearch/alerting/actionv2/GetMonitorV2Response.kt

+            .field(_SEQ_NO, seqNo)
+            .field(_PRIMARY_TERM, primaryTerm)
+        if (monitorV2 != null) {
+            builder.field("monitorV2", monitorV2)


Nitpick:
The monitorV2 field name seems a little awkward in this context. It seems like we could have probably just reused the [GetMonitorResponse] object.

Not blocking if this is the decision we're going for; but seems like the v1 and v2 monitors are being separated to an arbitrary extent.

This is indeed more of an engineering decision than a product decision. I chose this approach in the spirit of committing to separating the v1 and v2 APIs. It may be a duplicate of the v1 response object now, but it gives us room to add things to v2 that may not be applicable to v1.

lezzago · 2025-10-30T23:39:54Z

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorV2RestApiIT.kt

+        }
+
+        // RBAC is out of scope for this test, so give all users and requests the same one
+        val backendRole = "backend_role_a"


[Not blocking] We can disable filter by backend role setting so this is not needed to be added.

lezzago · 2025-10-30T23:52:48Z

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorV2RestApiIT.kt

+            )
+            fail("Expected update monitor to fail as user does not have permissions to index that monitor queries")
+        } catch (e: ResponseException) {
+            assertEquals("Unexpected error status", RestStatus.BAD_REQUEST.status, e.response.statusLine.statusCode)


We should have handle this error gracefully by calling out the user does not have permissions on the index set in the monitor. This would actually be more of a comment on the update monitor api

To validate the PPL Query, PPL Alerting submits the raw query to PPL plugin. If there's an error from the PPL plugin, it gets relayed to the user here.

The try catch that throws the error this test expects is pretty one size fits all: an invalid PPL query throws the same error as a valid query on an index users don't have permissions to. Increasing the granularity of the error would require either 1) parsing the string error response of the PPL query API, or 2) doing a separate Alerting level check of the indices in the PPL query before validating the PPL query against PPL Plugin itself.

Will raise a Github issue calling out the one size fits all nature of the validation check against PPL query.

Github issue: #1967

opensearch-trigger-bot · 2025-10-31T18:35:14Z

The backport to 3.3 failed:

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

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/alerting/backport-3.3 3.3
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-3.3
# Create a new branch
git switch --create backport-1966-to-3.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 819599f3431be0691d8540746a7a9c132e41dfa3
# Push it to GitHub
git push --set-upstream origin backport-1966-to-3.3
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-3.3

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

opensearch-trigger-bot · 2025-10-31T18:35:15Z

The backport to 2.19 failed:

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

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/alerting/backport-2.19 2.19
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-2.19
# Create a new branch
git switch --create backport-1966-to-2.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 819599f3431be0691d8540746a7a9c132e41dfa3
# Push it to GitHub
git push --set-upstream origin backport-1966-to-2.19
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-2.19

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

* PPL Alerting: Get and Search Monitors Signed-off-by: Dennis Toepker <[email protected]> * adding requested ID Signed-off-by: Dennis Toepker <[email protected]> --------- Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]> (cherry picked from commit 819599f)

* PPL Alerting: Get and Search Monitors * adding requested ID --------- (cherry picked from commit 819599f) Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]>

* PPL Alerting: Get and Search Monitors Signed-off-by: Dennis Toepker <[email protected]> * adding requested ID Signed-off-by: Dennis Toepker <[email protected]> --------- Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]>

* PPL Alerting: Get and Search Monitors * adding requested ID --------- Signed-off-by: Dennis Toepker <[email protected]> Co-authored-by: Dennis Toepker <[email protected]>

PPL Alerting: Get and Search Monitors

2402186

Signed-off-by: Dennis Toepker <[email protected]>

toepkerd marked this pull request as ready for review October 30, 2025 16:18

toepkerd requested review from AWSHurneyt, amsiglan, bowenlan-amzn, eirsep, engechas, getsaurabh02, goyamegh, jowg-amazon, lezzago, praveensameneni, rishabhmaurya, riysaxen-amzn and sbcd90 as code owners October 30, 2025 16:18

lezzago reviewed Oct 30, 2025

View reviewed changes

adding requested ID

f8e84d4

Signed-off-by: Dennis Toepker <[email protected]>

AWSHurneyt reviewed Oct 30, 2025

View reviewed changes

lezzago reviewed Oct 30, 2025

View reviewed changes

lezzago approved these changes Oct 31, 2025

View reviewed changes

AWSHurneyt approved these changes Oct 31, 2025

View reviewed changes

toepkerd merged commit 819599f into opensearch-project:main Oct 31, 2025
18 checks passed

toepkerd deleted the ppl-review branch October 31, 2025 16:55

toepkerd added backport 2.19 backport 3.3 labels Oct 31, 2025

opensearch-trigger-bot bot added the failed backport label Oct 31, 2025

toepkerd mentioned this pull request Nov 8, 2025

[Backport 2.19] PPL Alerting: Get and Search Monitors (#1966) #1983

Merged

PPL Alerting: Get and Search Monitors #1966

PPL Alerting: Get and Search Monitors #1966

Uh oh!

Conversation

toepkerd commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lezzago Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lezzago Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 31, 2025

Uh oh!

opensearch-trigger-bot bot commented Oct 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

toepkerd commented Oct 30, 2025 •

edited

Loading

lezzago Oct 30, 2025 •

edited

Loading

lezzago Oct 30, 2025 •

edited

Loading