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

Implemented filtering on the ISM eplain API #998

Merged
merged 9 commits into from
Dec 14, 2023
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

Check warning on line 24 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L20-L24

Added lines #L20 - L24 were not covered by tests
) : Writeable {

@Throws(IOException::class)
constructor(sin: StreamInput) : this(
policyID = sin.readOptionalString(),
state = sin.readOptionalString(),
actionType = sin.readOptionalString(),
failed = sin.readOptionalBoolean()
)

Check warning on line 33 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L28-L33

Added lines #L28 - L33 were not covered by tests

@Throws(IOException::class)
override fun writeTo(out: StreamOutput) {
out.writeOptionalString(policyID)
out.writeOptionalString(state)
out.writeOptionalString(actionType)
out.writeOptionalBoolean(failed)

Check warning on line 40 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L37-L40

Added lines #L37 - L40 were not covered by tests
}

fun byMetaData(metaData: ManagedIndexMetaData): Boolean {
var isValid = true

Check warning on line 44 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L44

Added line #L44 was not covered by tests

val stateMetaData = metaData.stateMetaData

Check warning on line 46 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L46

Added line #L46 was not covered by tests
if (state != null && (stateMetaData == null || stateMetaData.name != state)) {
isValid = false

Check warning on line 48 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L48

Added line #L48 was not covered by tests
}

val actionMetaData = metaData.actionMetaData

Check warning on line 51 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L51

Added line #L51 was not covered by tests
if (actionType != null && (actionMetaData == null || actionMetaData.name != actionType)) {
isValid = false

Check warning on line 53 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L53

Added line #L53 was not covered by tests
}

val retryInfoMetaData = metaData.policyRetryInfo

Check warning on line 56 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L56

Added line #L56 was not covered by tests
val actionFailedNotValid = actionMetaData == null || actionMetaData.failed != failed
val retryFailedNotValid = retryInfoMetaData == null || retryInfoMetaData.failed != failed
if (failed != null && actionFailedNotValid && retryFailedNotValid) {
isValid = false

Check warning on line 60 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L60

Added line #L60 was not covered by tests
}

return isValid

Check warning on line 63 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L63

Added line #L63 was not covered by tests
}

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 {
Copy link
Member

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

var policyID: String? = null
var state: String? = null
var actionType: String? = null
var failed: Boolean? = null

Check warning on line 79 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L76-L79

Added lines #L76 - L79 were not covered by tests

ensureExpectedToken(Token.START_OBJECT, xcp.currentToken(), xcp)

Check warning on line 81 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L81

Added line #L81 was not covered by tests
while (xcp.nextToken() != Token.END_OBJECT) {
val fieldName = xcp.currentName()
xcp.nextToken()

Check warning on line 84 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L83-L84

Added lines #L83 - L84 were not covered by tests

when (fieldName) {

Check warning on line 86 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L86

Added line #L86 was not covered by tests
FILTER_FIELD -> {
ensureExpectedToken(Token.START_OBJECT, xcp.currentToken(), xcp)

Check warning on line 88 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L88

Added line #L88 was not covered by tests
while (xcp.nextToken() != Token.END_OBJECT) {
val filter = xcp.currentName()
xcp.nextToken()

Check warning on line 91 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L90-L91

Added lines #L90 - L91 were not covered by tests

when (filter) {
POLICY_ID_FIELD -> policyID = xcp.text()
STATE_FIELD -> state = xcp.text()
ACTION_FIELD -> actionType = xcp.text()
FAILED_FIELD -> failed = xcp.booleanValue()

Check warning on line 97 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L94-L97

Added lines #L94 - L97 were not covered by tests
}
}
}
}
}

return ExplainFilter(policyID, state, actionType, failed)

Check warning on line 104 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L104

Added line #L104 was not covered by tests
}
}
}

fun BoolQueryBuilder.filterByPolicyID(explainFilter: ExplainFilter?): BoolQueryBuilder {
if (explainFilter?.policyID != null) {
this.filter(QueryBuilders.termsQuery(MANAGED_INDEX_POLICY_ID_FIELD, explainFilter.policyID))

Check warning on line 111 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/model/ExplainFilter.kt#L111

Added line #L111 was not covered by tests
}

return this
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
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
Expand All @@ -28,6 +31,7 @@
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)
Expand All @@ -52,6 +56,14 @@
ReplacedRoute(
GET, "$EXPLAIN_BASE_URI/{index}",
GET, "$LEGACY_EXPLAIN_BASE_URI/{index}"
),
ReplacedRoute(
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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}"
)
)
}
Expand All @@ -69,6 +81,14 @@

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)

Check warning on line 87 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/RestExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/RestExplainAction.kt#L85-L87

Added lines #L85 - L87 were not covered by tests
} else {
null
}

val clusterManagerTimeout = parseClusterManagerTimeout(
request, DeprecationLogger.getLogger(RestExplainAction::class.java), name
)
Expand All @@ -78,6 +98,7 @@
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.opensearch.core.common.io.stream.StreamInput
import org.opensearch.core.common.io.stream.StreamOutput
import org.opensearch.common.unit.TimeValue
import org.opensearch.indexmanagement.common.model.rest.SearchParams
import org.opensearch.indexmanagement.indexstatemanagement.model.ExplainFilter
import org.opensearch.indexmanagement.indexstatemanagement.util.DEFAULT_INDEX_TYPE
import java.io.IOException

Expand All @@ -21,6 +22,7 @@ class ExplainRequest : ActionRequest {
val local: Boolean
val clusterManagerTimeout: TimeValue
val searchParams: SearchParams
val explainFilter: ExplainFilter?
val showPolicy: Boolean
val validateAction: Boolean
val indexType: String
Expand All @@ -31,6 +33,7 @@ class ExplainRequest : ActionRequest {
local: Boolean,
clusterManagerTimeout: TimeValue,
searchParams: SearchParams,
explainFilter: ExplainFilter?,
showPolicy: Boolean,
validateAction: Boolean,
indexType: String
Expand All @@ -39,6 +42,7 @@ class ExplainRequest : ActionRequest {
this.local = local
this.clusterManagerTimeout = clusterManagerTimeout
this.searchParams = searchParams
this.explainFilter = explainFilter
this.showPolicy = showPolicy
this.validateAction = validateAction
this.indexType = indexType
Expand All @@ -50,6 +54,7 @@ class ExplainRequest : ActionRequest {
local = sin.readBoolean(),
clusterManagerTimeout = sin.readTimeValue(),
searchParams = SearchParams(sin),
explainFilter = sin.readOptionalWriteable(::ExplainFilter),
showPolicy = sin.readBoolean(),
validateAction = sin.readBoolean(),
indexType = sin.readString()
Expand All @@ -72,6 +77,7 @@ class ExplainRequest : ActionRequest {
out.writeBoolean(local)
out.writeTimeValue(clusterManagerTimeout)
searchParams.writeTo(out)
out.writeOptionalWriteable(explainFilter)
out.writeBoolean(showPolicy)
out.writeBoolean(validateAction)
out.writeString(indexType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
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
Expand Down Expand Up @@ -159,9 +160,10 @@
.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)
Expand Down Expand Up @@ -291,8 +293,32 @@
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

Check warning on line 301 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt#L300-L301

Added lines #L300 - L301 were not covered by tests

if (value != null) {
val metaData = ManagedIndexMetaData.fromMap(value)

Check warning on line 304 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt#L304

Added line #L304 was not covered by tests

if (!request.explainFilter.byMetaData(metaData)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we removing metaData.index from indexNames and indexNamesToUUIDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)

Check warning on line 308 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt#L307-L308

Added lines #L307 - L308 were not covered by tests

if (managedIndices.contains(metaData.index)) {
totalManagedIndices--

Check warning on line 311 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt#L311

Added line #L311 was not covered by tests
}

isValid = false

Check warning on line 314 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt#L314

Added line #L314 was not covered by tests
}
}

isValid

Check warning on line 318 in src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt

View check run for this annotation

Codecov / codecov/patch

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/explain/TransportExplainAction.kt#L318

Added line #L318 was not covered by tests
}
}

buildResponse(indexNamesToUUIDs, metadataMap, clusterStateIndexMetadatas, threadContext)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const val MANAGED_INDEX_FIELD = "managed_index"
const val MANAGED_INDEX_NAME_KEYWORD_FIELD = "$MANAGED_INDEX_FIELD.name.keyword"
const val MANAGED_INDEX_INDEX_FIELD = "$MANAGED_INDEX_FIELD.index"
const val MANAGED_INDEX_INDEX_UUID_FIELD = "$MANAGED_INDEX_FIELD.index_uuid"
const val MANAGED_INDEX_POLICY_ID_FIELD = "$MANAGED_INDEX_FIELD.policy_id"

const val DEFAULT_JOB_SORT_FIELD = MANAGED_INDEX_INDEX_FIELD
const val DEFAULT_POLICY_SORT_FIELD = "policy.policy_id.keyword"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ class ExplainRequestTests : OpenSearchTestCase() {
val local = true
val clusterManagerTimeout = TimeValue.timeValueSeconds(30)
val params = SearchParams(0, 20, "sort-field", "asc", "*")
val filter = null
val showPolicy = false
val showValidationResult = false
val req = ExplainRequest(indices, local, clusterManagerTimeout, params, showPolicy, showValidationResult, DEFAULT_INDEX_TYPE)
val req = ExplainRequest(indices, local, clusterManagerTimeout, params, filter, showPolicy, showValidationResult, DEFAULT_INDEX_TYPE)

val out = BytesStreamOutput()
req.writeTo(out)
Expand All @@ -36,9 +37,10 @@ class ExplainRequestTests : OpenSearchTestCase() {
val local = true
val clusterManagerTimeout = TimeValue.timeValueSeconds(30)
val params = SearchParams(0, 20, "sort-field", "asc", "*")
val filter = null
val showPolicy = false
val showValidationResult = false
val req = ExplainRequest(indices, local, clusterManagerTimeout, params, showPolicy, showValidationResult, "non-existent-index-type")
val req = ExplainRequest(indices, local, clusterManagerTimeout, params, filter, showPolicy, showValidationResult, "non-existent-index-type")

val actualException: String? = req.validate()?.validationErrors()?.firstOrNull()
val expectedException: String = ExplainRequest.MULTIPLE_INDICES_CUSTOM_INDEX_TYPE_ERROR
Expand Down
Loading