[Plugins] skeleton for events correlation engine plugin #6892
Conversation
sbcd90
requested review from
reta,
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
gbbafna,
setiah,
kartg,
kotwanikunal,
mch2,
nknize,
owaiskazi19,
Rishikesh1159,
ryanbogan,
saratvemulapalli,
shwetathareja,
dreamer-89,
tlfeng,
VachaShah and
xuezhou25
as code owners
Gradle Check (Jenkins) Run Completed with:
|
sbcd90
changed the title
Gradle Check (Jenkins) Run Completed with:
|
|
the @nknize @getsaurabh02 kindly have a look. |
There was a problem hiding this comment.
Thanks @sbcd90 for putting up this frame. Looks a good start to me and added few minor comments.
I see we will have other basic Actions components coming in soon such as to handle Correlation Joins or Correlation Rule Parsing, which we can add in subsequent PRs too.
| package org.opensearch.plugin.correlation.core; | ||
|
|
||
| public class CorrelationConstants { | ||
| public static final String METHOD_PARAMETER_M = "m"; |
There was a problem hiding this comment.
Wondering why is this needed, and can we add java doc to identify the usage here.
There was a problem hiding this comment.
removed the variable as this is just the skeleton plugin.
|
|
||
| import java.io.IOException; | ||
|
|
||
| public class RestSearchCorrelationAction extends BaseRestHandler { |
There was a problem hiding this comment.
nit : Should it be RestSearchCorrelatedEventsAction to keep it consistent with the related Actions? I am open to both
|
|
||
| import org.opensearch.action.ActionType; | ||
|
|
||
| public class IndexCorrelationRuleAction extends ActionType<IndexCorrelationRuleResponse> { |
There was a problem hiding this comment.
We will also need to corresponding GetCorrelationRuleAction along with its REST and Transport implementations?
There was a problem hiding this comment.
will add more actions with individual prs.
|
Can we also add some java docs to new Actions especially those bring added at the Transport and Rest layers for developer productivity. |
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Subhobrata Dey <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
@getsaurabh02 As this is just the skeleton, i haven't added any Javadoc as there is mostly no logic. will add code & javadoc for it in subsequent prs. |
The skeleton itself can have the java doc stating the intent behind. I am fine if we are planning to add them with the code. |
There was a problem hiding this comment.
What do you think about moving this to the sandbox while it is under development?
I definitely don't want to stand in the way of incremental development, and I certainly don't expect this whole feature to shipped in a single PR, but I can't really provide a meaningful review of empty classes. Is there no way to structure this such that you're delivering sections of functional and unit tested code in each PR?
|
hi @andrross , as we're planning to deliver this plugin for |
Description
Initial pr for defining skeleton of events correlation engine plugin. RFC is here
Issues Resolved
The 1st issue in the META issue will be resolved.
Check List
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.