Skip to content
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

Added documentation for printing associated queries, and sample documents in notification message. #6866

Merged

Conversation

AWSHurneyt
Copy link
Contributor

Description

  1. Changed the examples to access items in ctx arrays from using brackets (e.g., itemList[0]) as mustache template uses dot notation (e.g., itemList.0).
  2. Added documentation for associated_queries, and sample_documents variables added to the ctx object.

Implementation PR for reference - opensearch-project/alerting#1450

Issues Resolved

opensearch-project/alerting#1300
opensearch-project/alerting#1396
opensearch-project/alerting#1401

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…ents in notification message.

Signed-off-by: AWSHurneyt <[email protected]>
@AWSHurneyt AWSHurneyt force-pushed the 3.0-contextual-alerting branch from b40c4bf to 98767e6 Compare April 3, 2024 05:05
@AWSHurneyt
Copy link
Contributor Author

@hdhalter
Copy link
Contributor

Thanks so much for adding this content, @AWSHurneyt! Is it ready for a doc review?

@AWSHurneyt
Copy link
Contributor Author

Thanks so much for adding this content, @AWSHurneyt! Is it ready for a doc review?

@hdhalter Yes, this can be reviewed. I actually have a few other updates I was considering adding to this PR as well. We're still waiting for final approval though, so I'll just create a separate PR for them. Those updates would have be unrelated to the feature described in these doc changes; so it's probably for the best to keep them separate.

@hdhalter hdhalter added 4 - Doc review PR: Doc review in progress backport 2.13 PR: Backport label for 2.13 labels Apr 16, 2024

```groovy
Alerts
{{#ctx.alerts}}
Copy link
Contributor Author

@AWSHurneyt AWSHurneyt Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question for documentation reviewers:
The following sentence on line 146, calls out that the variable names for the "alerts" list is newAlerts for per bucket monitors, and alerts for per document monitors.

Each new alert that's generated during a monitor execution will be added to a list (i.e., newAlerts for per bucket monitors, and alerts for per document monitors) within the ctx variable.

The example in this code block is for a per document monitor as, on line 188, the "alerts" list is accessed via {{#ctx.alerts}} instead of {{#ctx.newAlerts}}. Does that seem clear enough, or would it be helpful to update the note on line 184 to call out that the example is for a per document monitor?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update line 184 and call it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revised.

@Naarcha-AWS Naarcha-AWS self-assigned this Apr 16, 2024
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved

```groovy
Alerts
{{#ctx.alerts}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update line 184 and call it out.

_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
AWSHurneyt and others added 2 commits May 8, 2024 09:50
@Naarcha-AWS Naarcha-AWS added 5 - Editorial review PR: Editorial review in progress and removed 4 - Doc review PR: Doc review in progress labels May 14, 2024
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AWSHurneyt @Naarcha-AWS Please see my comments and changes and let me know if you have any questions. Thanks!

_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
`bucket_keys` | String | Comma-separated list of the monitor's bucket key values. Available only for `ctx.dedupedAlerts`, `ctx.newAlerts`, and `ctx.completedAlerts`. Accessed through `ctx.dedupedAlerts[0].bucket_keys`.
`parent_bucket_path` | String | The parent bucket path of the bucket that triggered the alert. Accessed through `ctx.dedupedAlerts[0].parent_bucket_path`.
`ctx.alerts` | Array | Newly created alerts. Includes the `ctx.alerts.0.finding_ids` that triggered the alert and the `ctx.alerts.0.related_doc_ids` associated with the findings. Only available with document-level monitors.
`ctx.dedupedAlerts` | Array | Alerts that have been triggered. OpenSearch keeps the existing alert to prevent the plugin from creating endless numbers of the same alert. Only available with bucket-level monitors.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Alerts that have been triggered" => "Triggered alerts"? "creating endless numbers of the same alert" => "perpetually creating the same alert"?

_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
`bucket_keys` | String | A comma-separated list of the monitor's bucket key values. Available only for `ctx.dedupedAlerts`, `ctx.newAlerts`, and `ctx.completedAlerts`. Accessed through the `ctx.dedupedAlerts.0.bucket_keys` variable.
`parent_bucket_path` | String | The parent bucket path of the bucket that triggered the alert. Accessed through `ctx.dedupedAlerts.0.parent_bucket_path`.
`associated_queries` | Array | An array of document-level monitor queries that triggered the creation of the finding associated with the alert. Only available with document-level monitors. Accessed through the `ctx.alerts.0.associated_queries` variable.
`sample_documents` | Array | An array of sample documents that matched the monitor query. Only available with bucket-level, and document-level monitors. Accessed through `ctx.newAlerts.0.sample_documents` and `ctx.alerts.0.sample_documents` variables, respectively.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second sentence: "Available with both bucket- and document-level monitors"?

_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
_observing-your-data/alerting/triggers.md Outdated Show resolved Hide resolved
Naarcha-AWS and others added 3 commits May 14, 2024 09:48
@Naarcha-AWS Naarcha-AWS merged commit a4fb638 into opensearch-project:main May 14, 2024
5 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 14, 2024
…ents in notification message. (#6866)

* Added documentation for printing associated queries, and sample documents in notification message.

Signed-off-by: AWSHurneyt <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

* Apply suggestions from code review

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AWSHurneyt <[email protected]>

* Revised draft.

* Update _observing-your-data/alerting/triggers.md

Signed-off-by: Naarcha-AWS <[email protected]>

* Update _observing-your-data/alerting/triggers.md

Signed-off-by: Naarcha-AWS <[email protected]>

* Update _observing-your-data/alerting/triggers.md

Signed-off-by: Naarcha-AWS <[email protected]>

* Update _observing-your-data/alerting/triggers.md

Signed-off-by: Naarcha-AWS <[email protected]>

* Apply suggestions from code review

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Naarcha-AWS <[email protected]>

* Update _observing-your-data/alerting/triggers.md

Signed-off-by: Naarcha-AWS <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

* Apply suggestions from code review

Signed-off-by: Naarcha-AWS <[email protected]>

---------

Signed-off-by: AWSHurneyt <[email protected]>
Signed-off-by: Naarcha-AWS <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
(cherry picked from commit a4fb638)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
github-actions bot pushed a commit that referenced this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Editorial review PR: Editorial review in progress backport 2.13 PR: Backport label for 2.13
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants