Alerting V2 Monitor and Trigger models #873

toepkerd · 2025-09-03T22:33:07Z

Description

Adds PPL Monitor and PPL Trigger models for Alerting V2

Related Issues

Check List

[N] New functionality includes testing.
[N] New functionality has been documented.
[N/A] API changes companion pull request created.
[Y] Commits are signed per the DCO using --signoff.
[N/A] 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]>

eirsep · 2025-09-08T19:34:28Z

src/main/kotlin/org/opensearch/commons/alerting/model/AlertV2.kt

+    val schemaVersion: Int = NO_SCHEMA_VERSION,
+    val monitorId: String,
+    val monitorName: String,
+    val monitorVersion: Long,


what is monitorVersion?

This would refer to the version of the monitor this alert was generated from since sometimes the monitor is updated, so the current query might not be applicable anymore to the alert.

eirsep

tests?

eirsep · 2025-09-08T19:35:52Z

src/main/kotlin/org/opensearch/commons/alerting/model/AlertV2.kt

+import java.io.IOException
+import java.time.Instant
+
+data class AlertV2(


does Alert not have a state?

For tests: tests will be added post internal demo and customer playground, as the goal currently is to see if RBAC and other AuthZ can be implemented before internal and customer demo.

For Alert States: no, Alerts in AlertingV2 will not have states. The general flow of alerts is that a PPLMonitor will fire and forget them. Alerts will then get deleted by a separate Alert cleanup mechanism based on the configured Alert expiration time (AlertExpirer.kt) is the upcoming class that does this.

FYI we should discuss: opensearch-project/security#4500.

We should move away from storing user copies...esp for new use cases

For Roles Injection..used for Alerting monitor runtime..that would still require storing a user copy, however, for visibility and sharing purposes security will be pushing for the new model.

Roles Injection will also need replaced eventually. That is used in cases where the creator of an alerting monitor has DLS restrictions.

That being said, there is a known issue that a contributor has graciously been helping us with but its stuck in review in the alerting repo: opensearch-project/alerting#1917

eirsep · 2025-09-08T19:36:38Z

src/main/kotlin/org/opensearch/commons/alerting/model/AlertV2.kt

+        monitorId = sin.readString(),
+        monitorName = sin.readString(),
+        monitorVersion = sin.readLong(),
+//        monitorUser = if (sin.readBoolean()) {


why is this commented out?

see: #873 (comment)

See please: #873 (comment)

eirsep · 2025-09-08T19:39:05Z

src/main/kotlin/org/opensearch/commons/alerting/model/AlertV2.kt

+//    val monitorUser: User?,
+    val triggerId: String,
+    val triggerName: String,
+    val queryResults: Map<String, Any>,


will this be the same structure for PPL, SQL, DSL?

For PPL and SQL yes. DSL responses can technically be stored as a Map<String, Any>, but the PPL Alerting logic expects this to follow the format of SQL/PPL Plugin Execute API Response format: https://docs.opensearch.org/latest/search-plugins/sql/sql-ppl-api/#example-request

eirsep · 2025-09-08T19:40:21Z

src/main/kotlin/org/opensearch/commons/alerting/model/PPLTrigger.kt

+    override val name: String,
+    override val severity: Severity,
+    override val suppressDuration: TimeValue?,
+    override val expireDuration: TimeValue?,


what is this?
add code comments and java docs

lezzago · 2025-09-08T19:34:37Z

src/main/kotlin/org/opensearch/commons/alerting/model/AlertV2.kt

+    val monitorId: String,
+    val monitorName: String,
+    val monitorVersion: Long,
+//    val monitorUser: User?,


nitpick: cleanup commented code unless intentional. This applies to everywhere else as well.

Also I noticed the older alert contained this. Why do we want to remove it now? I believe this helped with rbac support.

The comments are intentional. In all coming PRs, RBAC related code is commented out as markers for what RBAC data needs to be stored, and where RBAC checks need to be made. There are no plans to remove monitorUser from Alerts

FYI we should discuss: opensearch-project/security#4500.

We should move away from storing user copies...esp for new use cases

lezzago · 2025-09-08T20:07:49Z

src/main/kotlin/org/opensearch/commons/alerting/model/AlertV2.kt

+    val schemaVersion: Int = NO_SCHEMA_VERSION,
+    val monitorId: String,
+    val monitorName: String,
+    val monitorVersion: Long,


This would refer to the version of the monitor this alert was generated from since sometimes the monitor is updated, so the current query might not be applicable anymore to the alert.

lezzago · 2025-09-08T20:27:01Z

src/main/kotlin/org/opensearch/commons/alerting/model/MonitorV2.kt

+import java.io.IOException
+import java.time.Instant
+
+interface MonitorV2 : ScheduledJob {


The RFC's model of the v2 monitor is different from this. Can we get them in sync?

MonitorV2 is meant to serve as a more upstream interface. Please see PPLMonitor.kt for a model that matches the RFC.

lezzago · 2025-09-08T20:35:08Z

tests?

+1 we need tests.

Also can we update the RFC with all the models we are planning on creating? Additionally is there any plugin requiring to use these alerting v2 monitors? If not, I would recommend keeping this in the Alerting plugin.
Also the Alerting plugin should follow what ML-Commons has done and expose a client that has a dependency on the Alerting's commons sub project and have that sub project contain all the models. This prevents having to make updates to the common-utils package to just update the Alerting plugin.

eirsep · 2025-09-09T21:15:07Z

src/main/kotlin/org/opensearch/commons/alerting/model/PPLTrigger.kt

+    override val suppressDuration: TimeValue?,
+    override val expireDuration: TimeValue?,
+    override var lastTriggeredTime: Instant?,
+    override val actions: List<Action>,


we need Action V2?

toepkerd force-pushed the ppl-review branch 3 times, most recently from 62cfcdf to 7997234 Compare September 4, 2025 17:25

toepkerd marked this pull request as ready for review September 4, 2025 17:36

toepkerd requested review from AWSHurneyt, bowenlan-amzn, cwperks, eirsep, engechas, getsaurabh02, jowg-amazon, lezzago, rishabhmaurya, riysaxen-amzn and sbcd90 as code owners September 4, 2025 17:36

toepkerd force-pushed the ppl-review branch 2 times, most recently from 11a7fed to ff223d2 Compare September 4, 2025 20:06

toepkerd-zz added 5 commits September 8, 2025 09:00

PPL Monitor and Trigger Models

ff22251

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

adding schemaVersion

0a6ca7d

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

linter issues

beb860f

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

initial lookback window implementation

6aa5218

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

adding AlertV2 model

4287231

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

toepkerd force-pushed the ppl-review branch from 77b92fb to 4287231 Compare September 8, 2025 16:00

updating models

e641fa9

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

eirsep reviewed Sep 8, 2025

View reviewed changes

lezzago reviewed Sep 8, 2025

View reviewed changes

eirsep reviewed Sep 9, 2025

View reviewed changes

Alerting V2 Monitor and Trigger models #873

Are you sure you want to change the base?

Alerting V2 Monitor and Trigger models #873

Uh oh!

Conversation

toepkerd commented Sep 3, 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!

eirsep left a comment

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!

cwperks Sep 12, 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!

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!

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 commented Sep 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

toepkerd commented Sep 3, 2025 •

edited

Loading

cwperks Sep 12, 2025 •

edited

Loading