EQL: Introduce basic execution pipeline #51809
Conversation
The main classes that form the 'execution' pipeline are added - most of them have no functionality; the purpose of this PR is to add flesh out the contract between the various moving parts so that work can start on them independently.
|
Pinging @elastic/es-search (:Search/EQL) |
|
|
||
| Failure other = (Failure) obj; | ||
| return Objects.equals(node, other.node); | ||
| } |
There was a problem hiding this comment.
include message to hashCode and equals impl?
| }); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
nit: a bit deeply nested imho, not sure what are the current guidelines
There was a problem hiding this comment.
There are no limits on nesting - it's really a matter of styles. Due to the use of lambdas, some bits tend to be nested and improving that by extracting the code into methods doesn't always work especially when using a lot of variables...
| @@ -60,6 +87,11 @@ boolean isSnapshot() { | |||
| return Build.CURRENT.isSnapshot(); | |||
| } | |||
|
|
|||
| // TODO: this needs to be used by all plugin methods - including getActions and createComponents | |||
There was a problem hiding this comment.
probably address this TODO before merging to master?
There was a problem hiding this comment.
I don't think there's a way to do that currently - the solution is to have a constructor that accepts settings however we provide our settings after instantiation ...
| return; | ||
| } | ||
| if (ae instanceof Unresolvable) { | ||
| // handle Attributes different to provide more context |
|
|
||
| public class PlanExecutor { | ||
| private final Client client; | ||
| private final NamedWriteableRegistry writableRegistry; |
There was a problem hiding this comment.
writableRegistry -> writeableRegistry
There was a problem hiding this comment.
I'm on the fence here - the correct word is writable not writeable; not sure which way to go...
|
|
||
| /** | ||
| * The verifier has the role of checking the analyzed tree for failures and build a list of failures following this check. | ||
| * It is created in the plan executor along with the metrics instance passed as constructor parameter. |
There was a problem hiding this comment.
We are assuming that metrics will be passed in the constructor in the same way as SQL, right?
There was a problem hiding this comment.
Correct as after the analyzer the type of query becomes clear.
|
|
||
| private final String[] indices; | ||
| private final TimeValue requestTimeout; | ||
| private final String clientId; |
There was a problem hiding this comment.
Is the clientId acting as a sticky session kind of identifier?
There was a problem hiding this comment.
Not really sticky however I imagined it would be useful for metrics such as knowing whether we've been called by SIEM or some different entity.
|
|
||
| public class Sequence { | ||
|
|
||
| private final List<Tuple<Object, List<SearchHit>>> events; |
There was a problem hiding this comment.
at some point, we'll also add join_keys and the other fields mentioned in the initial issue, but this is already good enough as a placeholder until then
There was a problem hiding this comment.
Right - the response will be fleshed out. For sequences I've added the join keys in the tuple under object (the response json is an array but the examples contain only one key).
| @@ -13,12 +13,6 @@ | |||
|
|
|||
| import java.util.List; | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
should this stay?
also, does this make sense for QL?
There was a problem hiding this comment.
No, that's an accidental commit that needs removal.
* EQL: Plug query params into the AstBuilder (#51886) As the eventType is customizable, plug that into the parser based on the given request. (cherry picked from commit 5b4a3a3) * EQL: Add field resolution and verification (#51872) Add basic field resolution inside the Analyzer and a basic Verifier to check for any unresolved fields. (cherry picked from commit 7087358) * EQL: Introduce basic execution pipeline (#51809) Add main classes that form the 'execution' pipeline are added - most of them have no functionality; the purpose of this PR is to add flesh out the contract between the various moving parts so that work can start on them independently. (cherry picked from commit 9a1bae5) * EQL: Add AstBuilder to convert to QL tree (#51558) * EQL: Add AstBuilder visitors * EQL: Add tests for wildcards and sets * EQL: Fix licensing * EQL: Fix ExpressionTests.java license * EQL: Cleanup imports * EQL: PR feedback and remove LiteralBuilder * EQL: Split off logical plan from expressions * EQL: Remove stray import * EQL: Add predicate handling for set checks * EQL: Remove commented out dead code * EQL: Remove wildcard test, wait until analyzer (cherry picked from commit a462700) * EQL grammar updates and tests (#49658) * EQL: Additional tests and grammar updates * EQL: Add backtick escaped identifiers * EQL: Adding keywords to language * EQL: Add checks for unsupported syntax * EQL: Testing updates and PR feedback * EQL: Add string escapes * EQL: Cleanup grammar for identifier * EQL: Remove tabs from .eql tests (cherry picked from commit 6f1890b)
Add main classes that form the 'execution' pipeline are added - most of
them have no functionality; the purpose of this PR is to add flesh out
the contract between the various moving parts so that work can start on
them independently.