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
Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. #992
Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. #992
Conversation
… cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
The test workflows are failing in this PR because they depend on the changes in PR opensearch-project/common-utils#479. The tests pass locally after manually publishing the common-utils artifact to my local maven repo. The test outputs for the two new test classes,
|
...arch/alerting/util/clusterMetricsMonitorHelpers/SupportedClusterMetricsSettingsExtensions.kt
Outdated
Show resolved
Hide resolved
…t() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
var clusterHealthRequest: ClusterHealthRequest = | ||
ClusterHealthRequest().indicesOptions(IndicesOptions.lenientExpandHidden()) | ||
var clusterStateRequest: ClusterStateRequest = | ||
ClusterStateRequest().indicesOptions(IndicesOptions.lenientExpandHidden()) | ||
var indexSettingsRequest: GetSettingsRequest = | ||
GetSettingsRequest() | ||
.indicesOptions(IndicesOptions.lenientExpandHidden()) | ||
.names(IndexSettings.INDEX_SEARCH_THROTTLED.key) | ||
var indicesStatsRequest: IndicesStatsRequest = | ||
IndicesStatsRequest().all().indicesOptions(IndicesOptions.lenientExpandHidden()) | ||
var indicesList = arrayOf<String>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment on why we are calling multiple APIs for future ref
fun `test CatIndicesRequestWrapper validate valid pathParams`() { | ||
// GIVEN | ||
val pathParams = "index1,index-name-2,index-3" | ||
|
||
// WHEN | ||
val requestWrapper = CatIndicesRequestWrapper(pathParams = pathParams) | ||
|
||
// THEN | ||
assertEquals(3, requestWrapper.clusterHealthRequest.indices().size) | ||
assertEquals(3, requestWrapper.clusterStateRequest.indices().size) | ||
assertEquals(3, requestWrapper.indexSettingsRequest.indices().size) | ||
assertEquals(3, requestWrapper.indicesStatsRequest.indices().size) | ||
} | ||
|
||
fun `test CatIndicesRequestWrapper validate without providing pathParams`() { | ||
// GIVEN & WHEN | ||
val requestWrapper = CatIndicesRequestWrapper() | ||
|
||
// THEN | ||
assertNull(requestWrapper.clusterHealthRequest.indices()) | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.clusterStateRequest.indices()) | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.indexSettingsRequest.indices()) | ||
assertNull(requestWrapper.indicesStatsRequest.indices()) | ||
} | ||
|
||
fun `test CatIndicesRequestWrapper validate blank pathParams`() { | ||
// GIVEN | ||
val pathParams = " " | ||
|
||
// WHEN | ||
val requestWrapper = CatIndicesRequestWrapper(pathParams = pathParams) | ||
|
||
// THEN | ||
assertNull(requestWrapper.clusterHealthRequest.indices()) | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.clusterStateRequest.indices()) | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.indexSettingsRequest.indices()) | ||
assertNull(requestWrapper.indicesStatsRequest.indices()) | ||
} | ||
|
||
fun `test CatIndicesRequestWrapper validate empty pathParams`() { | ||
// GIVEN | ||
val pathParams = "" | ||
|
||
// WHEN | ||
val requestWrapper = CatIndicesRequestWrapper(pathParams = pathParams) | ||
|
||
// THEN | ||
assertNull(requestWrapper.clusterHealthRequest.indices()) | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.clusterStateRequest.indices()) | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.indexSettingsRequest.indices()) | ||
assertNull(requestWrapper.indicesStatsRequest.indices()) | ||
} | ||
|
||
fun `test CatIndicesRequestWrapper validate invalid pathParams`() { | ||
// GIVEN | ||
val pathParams = "_index1,index^2" | ||
|
||
// WHEN & THEN | ||
assertThrows(IllegalArgumentException::class.java) { CatIndicesRequestWrapper(pathParams = pathParams) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to a unit test class
testIndices.forEach { (indexName, docCount) -> | ||
if (pathParamsIndices.contains(indexName)) { | ||
assertEquals( | ||
indexName, | ||
returnedIndices[indexName]?.get(CatIndicesResponseWrapper.IndexInfo.INDEX_FIELD) as String | ||
) | ||
assertEquals( | ||
docCount.toString(), | ||
returnedIndices[indexName]?.get(CatIndicesResponseWrapper.IndexInfo.DOCS_COUNT_FIELD) as String | ||
) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future follow up to have a real cat indices call and compare all the values and not some.
assertThrows(IllegalArgumentException::class.java) { CatIndicesRequestWrapper(pathParams = pathParams) } | ||
} | ||
|
||
suspend fun `test CatIndicesResponseWrapper returns with only indices in pathParams`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add an alias test too
fun `test CatShardsRequestWrapper validate valid pathParams`() { | ||
// GIVEN | ||
val pathParams = "index1,index_2,index-3" | ||
|
||
// WHEN | ||
val requestWrapper = CatShardsRequestWrapper(pathParams = pathParams) | ||
|
||
// THEN | ||
assertEquals(3, requestWrapper.clusterStateRequest.indices().size) | ||
assertEquals(3, requestWrapper.indicesStatsRequest.indices().size) | ||
} | ||
|
||
fun `test CatShardsRequestWrapper validate without providing pathParams`() { | ||
// GIVEN & WHEN | ||
val requestWrapper = CatShardsRequestWrapper() | ||
|
||
// THEN | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.clusterStateRequest.indices()) | ||
assertNull(requestWrapper.indicesStatsRequest.indices()) | ||
} | ||
|
||
fun `test CatShardsRequestWrapper validate blank pathParams`() { | ||
// GIVEN | ||
val pathParams = " " | ||
|
||
// WHEN | ||
val requestWrapper = CatShardsRequestWrapper(pathParams = pathParams) | ||
|
||
// THEN | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.clusterStateRequest.indices()) | ||
assertNull(requestWrapper.indicesStatsRequest.indices()) | ||
} | ||
|
||
fun `test CatShardsRequestWrapper validate empty pathParams`() { | ||
// GIVEN | ||
val pathParams = "" | ||
|
||
// WHEN | ||
val requestWrapper = CatShardsRequestWrapper(pathParams = pathParams) | ||
|
||
// THEN | ||
assertEquals(Strings.EMPTY_ARRAY, requestWrapper.clusterStateRequest.indices()) | ||
assertNull(requestWrapper.indicesStatsRequest.indices()) | ||
} | ||
|
||
fun `test CatShardsRequestWrapper validate invalid pathParams`() { | ||
// GIVEN | ||
val pathParams = "_index1,index^2" | ||
|
||
// WHEN & THEN | ||
assertThrows(IllegalArgumentException::class.java) { CatShardsRequestWrapper(pathParams = pathParams) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to unit test class
assertEquals(pathParamsIndices.size, returnedIndices.keys.size) | ||
testIndices.forEach { (indexName, docCount) -> | ||
if (pathParamsIndices.contains(indexName)) { | ||
assertEquals( | ||
indexName, | ||
returnedIndices[indexName]?.get(CatShardsResponseWrapper.ShardInfo.INDEX_FIELD) as String | ||
) | ||
assertEquals( | ||
docCount.toString(), | ||
returnedIndices[indexName]?.get(CatShardsResponseWrapper.ShardInfo.DOCS_FIELD) as String | ||
) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future follow up to compare against real cat shards call on all values.
Created this issue to track these follow-up items. |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-992-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a7720199d95ffb071c988786afb978172586e834
# Push it to GitHub
git push --set-upstream origin backport/backport-992-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.9 2.9
# Navigate to the new working tree
cd .worktrees/backport-2.9
# Create a new branch
git switch --create backport/backport-992-to-2.9
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a7720199d95ffb071c988786afb978172586e834
# Push it to GitHub
git push --set-upstream origin backport/backport-992-to-2.9
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.9 Then, create a pull request where the |
… cat/indices, and cat/shards. (opensearch-project#992) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored executeTransportAction to use suspendUntil() instead of get() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
… cat/indices, and cat/shards. (opensearch-project#992) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored executeTransportAction to use suspendUntil() instead of get() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
…monitor to call cat/indices, and cat/shards. #992 (#1009) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. (#992) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored executeTransportAction to use suspendUntil() instead of get() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Resolved merge conflicts. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored API calls from suspendUntil() to get(). Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
…monitor to call cat/indices, and cat/shards. #992 (#1009) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. (#992) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored executeTransportAction to use suspendUntil() instead of get() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Resolved merge conflicts. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored API calls from suspendUntil() to get(). Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com> (cherry picked from commit 84e8b00)
…monitor to call cat/indices, and cat/shards. #992 (#1009) (#1011) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. (#992) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored executeTransportAction to use suspendUntil() instead of get() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Resolved merge conflicts. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored API calls from suspendUntil() to get(). Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com> (cherry picked from commit 84e8b00) Co-authored-by: AWSHurneyt <hurneyt@amazon.com>
… cat/indices, and cat/shards. (opensearch-project#992) * Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint errors. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Refactored executeTransportAction to use suspendUntil() instead of get() to receive responses. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
Issue #, if available:
#611
Description of changes:
Implemented support for configuring a cluster metrics monitor to call cat/indices, and cat/shards.
Documentation issue opensearch-project/documentation-website#4437
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.