opensearch-project / alerting

📟 Get notified when your data meets certain conditions by setting up monitors, alerts, and notifications
https://opensearch.org/docs/latest/monitoring-plugins/alerting/index/
Apache License 2.0
60 stars 102 forks source link

Implemented cross-cluster monitor support #1404

Closed AWSHurneyt closed 8 months ago

AWSHurneyt commented 8 months ago

Issue #, if available: https://github.com/opensearch-project/alerting-dashboards-plugin/issues/796

Description of changes:

  1. Implemented support for cross-cluster cluster metrics monitors.
  2. Implemented GetRemoteIndexes API to populate the frontend UI with details regarding the remote clusters, and indexes.

These changes are intended to enhance query, bucket, and cluster metrics monitors. As mentioned in the documentation issue (link), query and bucket monitors already support querying remote indexes; but configuring such monitors required using devtools/API commands. The enhancements in this PR allow users to configure remote using the frontend UI. In addition, these changes allow cluster metrics monitors to execute against remote clusters, which was not previously possible.

When this feature is enabled, the frontend will display a cluster selection dropdown for query, bucket, and cluster metrics monitors. The index selection dropdown will then display the indexes on the selected clusters. Alternatively for cluster metrics monitors, users can include a list of cluster name strings in the monitor input to create remote cluster metrics monitors via devtools/API commands.

Documentation PR for reference https://github.com/opensearch-project/documentation-website/pull/6350

CheckList:

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.

AWSHurneyt commented 8 months ago

Successfully building this PR will depend on merging the common utils changes, and publishing maven. https://github.com/opensearch-project/common-utils/pull/584

bowenlan-amzn commented 8 months ago

@AWSHurneyt can you fix the workflow

For the description, can you add how would user use this feature? and a quick walkthrough of the implementation idea.

AWSHurneyt commented 8 months ago

@AWSHurneyt can you fix the workflow

For the description, can you add how would user use this feature? and a quick walkthrough of the implementation idea.

@bowenlan-amzn I added some more details to the description.

eirsep commented 8 months ago

why are we adding support to fetch remote indices in alerting plugin? isn't that a core feature?

AWSHurneyt commented 8 months ago

why are we adding support to fetch remote indices in alerting plugin? isn't that a core feature?

@eirsep The GetRemoteIndexes API is used to gather details about the clusters and their indexes to display in the frontend. The frontend plugins do not have access to remoteClients to make most API calls against the remote clusters.

eirsep commented 8 months ago

have we made changes in security plugin?

eirsep commented 8 months ago

no integ tests added to verify end to end behaviour?

AWSHurneyt commented 8 months ago

have we made changes in security plugin?

No, we didn't need to make any changes in the security plugin.

AWSHurneyt commented 8 months ago

no integ tests added to verify end to end behaviour?

This is not ideal, but so far testing has all been manual. I have implementing feature-specific tests as a fast follow-up item. For some additional context, query and bucket monitors already supported remote index querying. The major change in this PR is supporting remote calls for cluster metrics monitors.

AWSHurneyt commented 8 months ago

CVE PR https://github.com/opensearch-project/alerting/pull/1405 adjusted the ktlint version, which is causing style failures upon rerunning the tests now that the PR https://github.com/opensearch-project/common-utils/pull/584/changes changes have been published to maven. Cleaning up those errors.

AWSHurneyt commented 8 months ago

Lets follow up on test failures via opening separate issues

All tests are passing now. Merging in these changes.

opensearch-trigger-bot[bot] commented 8 months ago

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

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport-1404-to-2.x.