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.

Sign up for GitHub

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

Reference get monitor and search monitor action / request / responses from common-utils #1305

Merged
merged 4 commits into from
Nov 28, 2023
Merged

Reference get monitor and search monitor action / request / responses from common-utils #1305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2e46dde
Use get monitor action / req / resp from common-utils
ohltyler Nov 21, 2023
762efda
Clean up remaining references
ohltyler Nov 21, 2023
9d05072
Refactor search monitor action/request also
ohltyler Nov 21, 2023
15464b2
Rewrite get monitor and search monitor transport action requests
ohltyler Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,8 @@ import org.opensearch.alerting.action.ExecuteWorkflowAction
import org.opensearch.alerting.action.GetDestinationsAction
import org.opensearch.alerting.action.GetEmailAccountAction
import org.opensearch.alerting.action.GetEmailGroupAction
import org.opensearch.alerting.action.GetMonitorAction
import org.opensearch.alerting.action.SearchEmailAccountAction
import org.opensearch.alerting.action.SearchEmailGroupAction
import org.opensearch.alerting.action.SearchMonitorAction
import org.opensearch.alerting.alerts.AlertIndices
import org.opensearch.alerting.core.JobSweeper
import org.opensearch.alerting.core.ScheduledJobIndices
Expand Down Expand Up @@ -193,9 +191,9 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R
return listOf(
ActionPlugin.ActionHandler(ScheduledJobsStatsAction.INSTANCE, ScheduledJobsStatsTransportAction::class.java),
ActionPlugin.ActionHandler(AlertingActions.INDEX_MONITOR_ACTION_TYPE, TransportIndexMonitorAction::class.java),
ActionPlugin.ActionHandler(GetMonitorAction.INSTANCE, TransportGetMonitorAction::class.java),
ActionPlugin.ActionHandler(AlertingActions.GET_MONITOR_ACTION_TYPE, TransportGetMonitorAction::class.java),
ActionPlugin.ActionHandler(ExecuteMonitorAction.INSTANCE, TransportExecuteMonitorAction::class.java),
ActionPlugin.ActionHandler(SearchMonitorAction.INSTANCE, TransportSearchMonitorAction::class.java),
ActionPlugin.ActionHandler(AlertingActions.SEARCH_MONITORS_ACTION_TYPE, TransportSearchMonitorAction::class.java),
ActionPlugin.ActionHandler(AlertingActions.DELETE_MONITOR_ACTION_TYPE, TransportDeleteMonitorAction::class.java),
ActionPlugin.ActionHandler(AlertingActions.ACKNOWLEDGE_ALERTS_ACTION_TYPE, TransportAcknowledgeAlertAction::class.java),
ActionPlugin.ActionHandler(
Expand Down
15 changes: 0 additions & 15 deletions alerting/src/main/kotlin/org/opensearch/alerting/action/GetMonitorAction.kt
View file
Open in desktop

This file was deleted.

56 changes: 0 additions & 56 deletions alerting/src/main/kotlin/org/opensearch/alerting/action/GetMonitorRequest.kt
View file
Open in desktop

This file was deleted.

133 changes: 0 additions & 133 deletions alerting/src/main/kotlin/org/opensearch/alerting/action/GetMonitorResponse.kt
View file
Open in desktop

This file was deleted.

16 changes: 0 additions & 16 deletions alerting/src/main/kotlin/org/opensearch/alerting/action/SearchMonitorAction.kt
View file
Open in desktop

This file was deleted.

38 changes: 0 additions & 38 deletions alerting/src/main/kotlin/org/opensearch/alerting/action/SearchMonitorRequest.kt
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestGetMonitorAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ package org.opensearch.alerting.resthandler

import org.apache.logging.log4j.LogManager
import org.opensearch.alerting.AlertingPlugin
import org.opensearch.alerting.action.GetMonitorAction
import org.opensearch.alerting.action.GetMonitorRequest
import org.opensearch.alerting.util.context
import org.opensearch.client.node.NodeClient
import org.opensearch.commons.alerting.action.AlertingActions
import org.opensearch.commons.alerting.action.GetMonitorRequest
import org.opensearch.rest.BaseRestHandler
import org.opensearch.rest.BaseRestHandler.RestChannelConsumer
import org.opensearch.rest.RestHandler.ReplacedRoute
Expand Down Expand Up @@ -69,7 +69,7 @@ class RestGetMonitorAction : BaseRestHandler() {
val getMonitorRequest = GetMonitorRequest(monitorId, RestActions.parseVersion(request), request.method(), srcContext)
return RestChannelConsumer {
channel ->
client.execute(GetMonitorAction.INSTANCE, getMonitorRequest, RestToXContentListener(channel))
client.execute(AlertingActions.GET_MONITOR_ACTION_TYPE, getMonitorRequest, RestToXContentListener(channel))
}
}
}
6 changes: 3 additions & 3 deletions alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestSearchMonitorAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import org.apache.logging.log4j.LogManager
import org.opensearch.action.search.SearchRequest
import org.opensearch.action.search.SearchResponse
import org.opensearch.alerting.AlertingPlugin
import org.opensearch.alerting.action.SearchMonitorAction
import org.opensearch.alerting.action.SearchMonitorRequest
import org.opensearch.alerting.alerts.AlertIndices.Companion.ALL_ALERT_INDEX_PATTERN
import org.opensearch.alerting.settings.AlertingSettings
import org.opensearch.alerting.util.context
Expand All @@ -20,6 +18,8 @@ import org.opensearch.common.settings.Settings
import org.opensearch.common.xcontent.LoggingDeprecationHandler
import org.opensearch.common.xcontent.XContentFactory.jsonBuilder
import org.opensearch.common.xcontent.XContentType
import org.opensearch.commons.alerting.action.AlertingActions
import org.opensearch.commons.alerting.action.SearchMonitorRequest
import org.opensearch.commons.alerting.model.ScheduledJob
import org.opensearch.commons.alerting.model.ScheduledJob.Companion.SCHEDULED_JOBS_INDEX
import org.opensearch.core.common.bytes.BytesReference
Expand Down Expand Up @@ -101,7 +101,7 @@ class RestSearchMonitorAction(

val searchMonitorRequest = SearchMonitorRequest(searchRequest)
return RestChannelConsumer { channel ->
client.execute(SearchMonitorAction.INSTANCE, searchMonitorRequest, searchMonitorResponse(channel))
client.execute(AlertingActions.SEARCH_MONITORS_ACTION_TYPE, searchMonitorRequest, searchMonitorResponse(channel))
}
}

Expand Down
7 changes: 3 additions & 4 deletions ...ting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ import org.opensearch.action.search.SearchResponse
import org.opensearch.action.support.ActionFilters
import org.opensearch.action.support.HandledTransportAction
import org.opensearch.action.update.UpdateRequest
import org.opensearch.alerting.action.GetMonitorAction
import org.opensearch.alerting.action.GetMonitorRequest
import org.opensearch.alerting.action.GetMonitorResponse
import org.opensearch.alerting.opensearchapi.suspendUntil
import org.opensearch.alerting.settings.AlertingSettings
import org.opensearch.alerting.util.AlertingException
Expand All @@ -38,6 +35,8 @@ import org.opensearch.common.xcontent.XContentType
import org.opensearch.commons.alerting.action.AcknowledgeAlertRequest
import org.opensearch.commons.alerting.action.AcknowledgeAlertResponse
import org.opensearch.commons.alerting.action.AlertingActions
import org.opensearch.commons.alerting.action.GetMonitorRequest
import org.opensearch.commons.alerting.action.GetMonitorResponse
import org.opensearch.commons.alerting.model.Alert
import org.opensearch.commons.alerting.model.Monitor
import org.opensearch.commons.alerting.util.optionalTimeField
Expand Down Expand Up @@ -94,7 +93,7 @@ class TransportAcknowledgeAlertAction @Inject constructor(
RestRequest.Method.GET,
FetchSourceContext.FETCH_SOURCE
)
execute(GetMonitorAction.INSTANCE, getMonitorRequest, it)
execute(AlertingActions.GET_MONITOR_ACTION_TYPE, getMonitorRequest, it)
}
if (getMonitorResponse.monitor == null) {
actionListener.onFailure(
Expand Down
7 changes: 3 additions & 4 deletions alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetFindingsAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import org.opensearch.action.search.SearchRequest
import org.opensearch.action.search.SearchResponse
import org.opensearch.action.support.ActionFilters
import org.opensearch.action.support.HandledTransportAction
import org.opensearch.alerting.action.GetMonitorAction
import org.opensearch.alerting.action.GetMonitorRequest
import org.opensearch.alerting.action.GetMonitorResponse
import org.opensearch.alerting.alerts.AlertIndices.Companion.ALL_FINDING_INDEX_PATTERN
import org.opensearch.alerting.opensearchapi.suspendUntil
import org.opensearch.alerting.settings.AlertingSettings
Expand All @@ -35,6 +32,8 @@ import org.opensearch.common.xcontent.XContentType
import org.opensearch.commons.alerting.action.AlertingActions
import org.opensearch.commons.alerting.action.GetFindingsRequest
import org.opensearch.commons.alerting.action.GetFindingsResponse
import org.opensearch.commons.alerting.action.GetMonitorRequest
import org.opensearch.commons.alerting.action.GetMonitorResponse
import org.opensearch.commons.alerting.model.Finding
import org.opensearch.commons.alerting.model.FindingDocument
import org.opensearch.commons.alerting.model.FindingWithDocs
Expand Down Expand Up @@ -167,7 +166,7 @@ class TransportGetFindingsSearchAction @Inject constructor(
)
val getMonitorResponse: GetMonitorResponse =
[email protected] {
execute(GetMonitorAction.INSTANCE, getMonitorRequest, it)
execute(AlertingActions.GET_MONITOR_ACTION_TYPE, getMonitorRequest, it)
Copy link
Member

Choose a reason for hiding this comment

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

rewrite request to avoid serialization issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is following strategies of your previous refactoring PRs - see https://github.com/opensearch-project/alerting/pull/606/files

Could you explain more what the potential issue could be?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/opensearch-project/alerting/blob/main/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt#L121-L123

we had faced ser/deserialization issues due to different plugins not sharing the same classloader amongst themselves

so, although these tests may pass now, the plugin invoking get monitor request would fail if the request is not transformed and re-deserialized in the transport layer

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it, thanks for the clarification. Updated the transport actions in latest commit, and did another round of manual verification on the changed APIs

}
indexName = getMonitorResponse.monitor?.dataSources?.findingsIndex ?: ALL_FINDING_INDEX_PATTERN
}
Expand Down
Loading
Loading