-
Notifications
You must be signed in to change notification settings - Fork 130
Implemented filtering on the ISM eplain API #998
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
Changes from all commits
c06f95d
c58d8d1
2be9253
73af3b7
871952c
f75579c
d4ca2c8
f6ba9dd
6139c9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.indexmanagement.indexstatemanagement.model | ||
|
|
||
| import org.opensearch.core.common.io.stream.StreamInput | ||
| import org.opensearch.core.common.io.stream.StreamOutput | ||
| import org.opensearch.core.common.io.stream.Writeable | ||
| import org.opensearch.core.xcontent.XContentParser | ||
| import org.opensearch.core.xcontent.XContentParser.Token | ||
| import org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken | ||
| import org.opensearch.index.query.BoolQueryBuilder | ||
| import org.opensearch.index.query.QueryBuilders | ||
| import org.opensearch.indexmanagement.indexstatemanagement.util.MANAGED_INDEX_POLICY_ID_FIELD | ||
| import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ManagedIndexMetaData | ||
| import java.io.IOException | ||
|
|
||
| data class ExplainFilter( | ||
| val policyID: String? = null, | ||
| val state: String? = null, | ||
| val actionType: String? = null, | ||
| val failed: Boolean? = null | ||
| ) : Writeable { | ||
|
|
||
| @Throws(IOException::class) | ||
| constructor(sin: StreamInput) : this( | ||
| policyID = sin.readOptionalString(), | ||
| state = sin.readOptionalString(), | ||
| actionType = sin.readOptionalString(), | ||
| failed = sin.readOptionalBoolean() | ||
| ) | ||
|
|
||
| @Throws(IOException::class) | ||
| override fun writeTo(out: StreamOutput) { | ||
| out.writeOptionalString(policyID) | ||
| out.writeOptionalString(state) | ||
| out.writeOptionalString(actionType) | ||
| out.writeOptionalBoolean(failed) | ||
| } | ||
|
|
||
| fun byMetaData(metaData: ManagedIndexMetaData): Boolean { | ||
| var isValid = true | ||
|
|
||
| val stateMetaData = metaData.stateMetaData | ||
| if (state != null && (stateMetaData == null || stateMetaData.name != state)) { | ||
| isValid = false | ||
| } | ||
|
|
||
| val actionMetaData = metaData.actionMetaData | ||
| if (actionType != null && (actionMetaData == null || actionMetaData.name != actionType)) { | ||
| isValid = false | ||
| } | ||
|
|
||
| val retryInfoMetaData = metaData.policyRetryInfo | ||
| val actionFailedNotValid = actionMetaData == null || actionMetaData.failed != failed | ||
| val retryFailedNotValid = retryInfoMetaData == null || retryInfoMetaData.failed != failed | ||
| if (failed != null && actionFailedNotValid && retryFailedNotValid) { | ||
| isValid = false | ||
| } | ||
|
|
||
| return isValid | ||
| } | ||
|
|
||
| companion object { | ||
| const val FILTER_FIELD = "filter" | ||
| const val POLICY_ID_FIELD = "policy_id" | ||
| const val STATE_FIELD = "state" | ||
| const val ACTION_FIELD = "action_type" | ||
| const val FAILED_FIELD = "failed" | ||
|
|
||
| @JvmStatic | ||
| @Throws(IOException::class) | ||
| fun parse(xcp: XContentParser): ExplainFilter { | ||
| var policyID: String? = null | ||
| var state: String? = null | ||
| var actionType: String? = null | ||
| var failed: Boolean? = null | ||
|
|
||
| ensureExpectedToken(Token.START_OBJECT, xcp.currentToken(), xcp) | ||
| while (xcp.nextToken() != Token.END_OBJECT) { | ||
| val fieldName = xcp.currentName() | ||
| xcp.nextToken() | ||
|
|
||
| when (fieldName) { | ||
| FILTER_FIELD -> { | ||
| ensureExpectedToken(Token.START_OBJECT, xcp.currentToken(), xcp) | ||
| while (xcp.nextToken() != Token.END_OBJECT) { | ||
| val filter = xcp.currentName() | ||
| xcp.nextToken() | ||
|
|
||
| when (filter) { | ||
| POLICY_ID_FIELD -> policyID = xcp.text() | ||
| STATE_FIELD -> state = xcp.text() | ||
| ACTION_FIELD -> actionType = xcp.text() | ||
| FAILED_FIELD -> failed = xcp.booleanValue() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return ExplainFilter(policyID, state, actionType, failed) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun BoolQueryBuilder.filterByPolicyID(explainFilter: ExplainFilter?): BoolQueryBuilder { | ||
| if (explainFilter?.policyID != null) { | ||
| this.filter(QueryBuilders.termsQuery(MANAGED_INDEX_POLICY_ID_FIELD, explainFilter.policyID)) | ||
| } | ||
|
|
||
| return this | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,11 @@ import org.apache.logging.log4j.LogManager | |
| import org.opensearch.client.node.NodeClient | ||
| import org.opensearch.core.common.Strings | ||
| import org.opensearch.common.logging.DeprecationLogger | ||
| import org.opensearch.core.xcontent.XContentParser.Token | ||
| import org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken | ||
| import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.ISM_BASE_URI | ||
| import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.LEGACY_ISM_BASE_URI | ||
| import org.opensearch.indexmanagement.indexstatemanagement.model.ExplainFilter | ||
| import org.opensearch.indexmanagement.indexstatemanagement.transport.action.explain.ExplainAction | ||
| import org.opensearch.indexmanagement.indexstatemanagement.transport.action.explain.ExplainRequest | ||
| import org.opensearch.indexmanagement.indexstatemanagement.util.DEFAULT_EXPLAIN_VALIDATE_ACTION | ||
|
|
@@ -28,6 +31,7 @@ import org.opensearch.rest.RestHandler.ReplacedRoute | |
| import org.opensearch.rest.RestHandler.Route | ||
| import org.opensearch.rest.RestRequest | ||
| import org.opensearch.rest.RestRequest.Method.GET | ||
| import org.opensearch.rest.RestRequest.Method.POST | ||
| import org.opensearch.rest.action.RestToXContentListener | ||
|
|
||
| private val log = LogManager.getLogger(RestExplainAction::class.java) | ||
|
|
@@ -52,6 +56,14 @@ class RestExplainAction : BaseRestHandler() { | |
| ReplacedRoute( | ||
| GET, "$EXPLAIN_BASE_URI/{index}", | ||
| GET, "$LEGACY_EXPLAIN_BASE_URI/{index}" | ||
| ), | ||
| ReplacedRoute( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we able to use explain API with filter enabled across all indices and on specific indices? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it works for both |
||
| POST, EXPLAIN_BASE_URI, | ||
| POST, LEGACY_EXPLAIN_BASE_URI | ||
| ), | ||
| ReplacedRoute( | ||
| POST, "$EXPLAIN_BASE_URI/{index}", | ||
| POST, "$LEGACY_EXPLAIN_BASE_URI/{index}" | ||
| ) | ||
| ) | ||
| } | ||
|
|
@@ -69,6 +81,14 @@ class RestExplainAction : BaseRestHandler() { | |
|
|
||
| val indexType = request.param(TYPE_PARAM_KEY, DEFAULT_INDEX_TYPE) | ||
|
|
||
| val explainFilter = if (request.method() == RestRequest.Method.POST) { | ||
| val xcp = request.contentParser() | ||
| ensureExpectedToken(Token.START_OBJECT, xcp.nextToken(), xcp) | ||
| ExplainFilter.parse(xcp) | ||
| } else { | ||
| null | ||
| } | ||
|
|
||
| val clusterManagerTimeout = parseClusterManagerTimeout( | ||
| request, DeprecationLogger.getLogger(RestExplainAction::class.java), name | ||
| ) | ||
|
|
@@ -78,6 +98,7 @@ class RestExplainAction : BaseRestHandler() { | |
| request.paramAsBoolean("local", false), | ||
| clusterManagerTimeout, | ||
| searchParams, | ||
| explainFilter, | ||
| request.paramAsBoolean(SHOW_POLICY_QUERY_PARAM, DEFAULT_EXPLAIN_SHOW_POLICY), | ||
| request.paramAsBoolean(SHOW_VALIDATE_ACTION, DEFAULT_EXPLAIN_VALIDATE_ACTION), | ||
| indexType | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexCon | |
| import org.opensearch.indexmanagement.indexstatemanagement.model.Policy | ||
| import org.opensearch.indexmanagement.common.model.rest.SearchParams | ||
| import org.opensearch.indexmanagement.indexstatemanagement.ManagedIndexRunner.actionValidation | ||
| import org.opensearch.indexmanagement.indexstatemanagement.model.filterByPolicyID | ||
| import org.opensearch.indexmanagement.indexstatemanagement.transport.action.managedIndex.ManagedIndexAction | ||
| import org.opensearch.indexmanagement.indexstatemanagement.transport.action.managedIndex.ManagedIndexRequest | ||
| import org.opensearch.indexmanagement.indexstatemanagement.util.DEFAULT_INDEX_TYPE | ||
|
|
@@ -159,9 +160,10 @@ class TransportExplainAction @Inject constructor( | |
| .must( | ||
| QueryBuilders | ||
| .queryStringQuery(params.queryString) | ||
| .defaultField(MANAGED_INDEX_NAME_KEYWORD_FIELD) | ||
| .field(MANAGED_INDEX_NAME_KEYWORD_FIELD) | ||
| .defaultOperator(Operator.AND) | ||
| ).filter(QueryBuilders.termsQuery(MANAGED_INDEX_INDEX_UUID_FIELD, indexUUIDs)) | ||
| .filterByPolicyID(request.explainFilter) | ||
|
|
||
| val searchSourceBuilder = SearchSourceBuilder() | ||
| .from(params.from) | ||
|
|
@@ -291,8 +293,32 @@ class TransportExplainAction @Inject constructor( | |
| mgetMetadataReq, | ||
| object : ActionListener<MultiGetResponse> { | ||
| override fun onResponse(response: MultiGetResponse) { | ||
| val metadataMap: Map<ManagedIndexMetadataDocUUID, ManagedIndexMetadataMap?> = | ||
| var metadataMap: Map<ManagedIndexMetadataDocUUID, ManagedIndexMetadataMap?> = | ||
| response.responses.associate { it.id to getMetadata(it.response)?.toMap() } | ||
|
|
||
| if (request.explainFilter != null) { | ||
| metadataMap = metadataMap.filter { (_, value) -> | ||
| var isValid = true | ||
|
|
||
| if (value != null) { | ||
| val metaData = ManagedIndexMetaData.fromMap(value) | ||
|
|
||
| if (!request.explainFilter.byMetaData(metaData)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we removing metaData.index from indexNames and indexNamesToUUIDs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When creating the actual response, all the index names are looped over. So, in order to remove the filtered indices, they must be removed from indexNames. |
||
| indexNames.remove(metaData.index) | ||
| indexNamesToUUIDs.remove(metaData.index) | ||
|
|
||
| if (managedIndices.contains(metaData.index)) { | ||
| totalManagedIndices-- | ||
| } | ||
|
|
||
| isValid = false | ||
| } | ||
| } | ||
|
|
||
| isValid | ||
| } | ||
| } | ||
|
|
||
| buildResponse(indexNamesToUUIDs, metadataMap, clusterStateIndexMetadatas, threadContext) | ||
| } | ||
|
|
||
|
|
||
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.
Please add unit tests for the codecov warning. Feel free to add in another PR
Let me know if you need any guidance on writing UTs