-
Notifications
You must be signed in to change notification settings - Fork 25.6k
EQL: Introduce support for sequence maxspan #58635
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
Conversation
|
Pinging @elastic/es-ql (:Query Languages/EQL) |
|
|
||
| [[queries]] | ||
| query = ''' | ||
| sequence with maxspan=1h |
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.
@rw-access this test and the rest should not return any results; any maxspan lower than 15h would not match a sequence.
Entries 67 and 68 have the same timestamp (131509374395921780) but 69 has a timestamp of 131509374446778110 meaning the distance between the beginning of the sequence and the 69 is:
131509374446778110 - 131509374395921780 = 50856330 millis or ~14h.
Am I missing something?
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.
The timestamp units are windows filetime. One unit is 1e-7 seconds. I'm guessing that's what you're running into
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.
Thanks. This would need to be aligned with ECS/unix timestamp.
astefan
left a comment
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.
LGTM. Left few really minor comments.
| "@timestamp" : { | ||
| "type" : "alias", | ||
| "path" : "timestamp" | ||
| "type" : "date" |
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.
Why did you change this?
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.
Because timestamp is not unix time but rather window filetime (see this comment and until the dataset gets updated, working on a separate field is the cleaner.
| public Criterion useMarker(Object[] marker) { | ||
| searchSource.searchAfter(marker); | ||
| return this; | ||
| public Iterable<SearchHit> iterateable(List<SearchHit> hits) { |
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.
iterable sounds better.
| * greater than the given argument alongside its position in the list. | ||
| */ | ||
| public Tuple<Sequence, Integer> after(long timestamp, Comparable<Object> tiebreaker) { | ||
| public Tuple<Sequence, Integer> after(Ordinal ordinal) { |
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.
before and after have almost the same code. Could you reuse it in a common method?
EQL sequences can specify now a maximum time allowed for their span (computed between the first and the last matching event). (cherry picked from commit 747c359)
Still in draft while sorting out the tests situation