EQL: Introduce repeatable queries by costin · Pull Request #75082 · elastic/elasticsearch

costin · 2021-07-07T17:01:59Z

Allow individual queries within a sequence to be repeated through a
dedicated keyword without having physical duplication.

sequence
queryA [runs=2]
queryB
queryC [runs=3]
queryD

is the same as:

sequence
queryA
queryA
queryB
queryC
queryC
queryC
queryD

but more concise.

Allow individual queries within a sequence to be repeated through a dedicated keyword without having physical duplication. sequence queryA repeat=2 queryB queryC repeat=3 queryD is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise.

astefan

Looks great.
Some unit tests need to be added for the new exceptions. And the scenarios where multiple repeats are used in the same sequence need clarification.

astefan · 2021-07-08T04:38:43Z

x-pack/plugin/eql/qa/common/src/main/resources/test_queries.toml

+query = '''
+sequence
+  [process where opcode == 1] by unique_pid,  process_path
+  [file where opcode == 0]    by unique_pid,  process_path  repeat=3


How about the following scenario?

[file where opcode == 0] by unique_pid, process_path repeat=2 [file where opcode == 0] by unique_pid, process_path repeat=1

astefan · 2021-07-08T04:39:56Z

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/LogicalPlanBuilder.java

+            if (numberOfQueries > 256) {
+                throw new ParsingException(
+                    source(sequenceTermCtx),
+                    "Sequence cannot contains more than 256 queries; found [{}]",


Suggested change

"Sequence cannot contains more than 256 queries; found [{}]",

"Sequence cannot contain more than 256 queries; found [{}]",

astefan · 2021-07-08T04:40:29Z

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/LogicalPlanBuilder.java

+        }
+
+        if (queries.size() < 2) {
+            throw new ParsingException(source, "A sequence requires a minimum of 2  queries, found [{}]", queries.size());


Suggested change

throw new ParsingException(source, "A sequence requires a minimum of 2 queries, found [{}]", queries.size());

throw new ParsingException(source, "A sequence requires a minimum of 2 queries, found [{}]", queries.size());

Can you also test this new exception?

…table-queries

…table-queries Update EQL repeatable through runs option

astefan

LGTM. Left one minor comment.

astefan · 2021-09-29T11:49:24Z

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/LogicalPlanBuilder.java

+                }
+            }
+
+            int runs = 1;


If this is limited to 100, why not using byte?

Habit - since it's a local variable it doesn't make a difference.

elasticsearchmachine · 2021-09-29T12:10:29Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 75082

Allow individual queries within a sequence to be ran multiple times through using the [runs=number] construct as a suffix without having to redeclare the query. sequence queryA [runs=2] queryB queryC [runs=3] queryD is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise. (cherry picked from commit c7ef3a6)

costin · 2021-09-29T15:02:41Z

/cc @jrodewig

Allow individual queries within a sequence to be ran multiple times through using the [runs=number] construct as a suffix without having to redeclare the query. sequence queryA [runs=2] queryB queryC [runs=3] queryD is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise. (cherry picked from commit c7ef3a6)

Documents the `runs` keyword for running the same event criteria successively in a sequence query. Relates to #75082.

Documents the `runs` keyword for running the same event criteria successively in a sequence query. Relates to #75082. # Conflicts: # docs/reference/release-notes/highlights.asciidoc

rw-access · 2021-10-04T14:54:37Z

what's up with the brackets? the [runs=X] syntax doesn't make sense (choice of word aside) and now the meaning of brackets is very overloaded and confusing.

before, it just had exactly one meaning that was used in a lot of contexts. it wasn't on accident that it always contained an event filter: [event category where condition]

some of those usages: terms in a sequence or join, descendant of, until, etc.
it could also be used to index into arrays but clearly distinct field[1]

also the placement (after by) does seem a little odd in choice.

i would expect this to be more intuitive: (again, choice of word aside)

sequence
  [process where opcode == 1]          by unique_pid
  [file where    opcode == 0] runs=3   by unique_pid

costin · 2021-10-07T07:24:52Z

Hi Ross. Thanks for the feedback.

tl;dr

To avoid stretching [], it is being replaced with with a construct that already has the semantics we want and sounds good enough.

sequence
  [process where opcode == 1] by unique_pid
  [file where    opcode == 0] by unique_pid with runs=2
  [file where    opcode == 0] by unique_pid

Long story

There were several discussions going back and forth on how to declare runs which is a meta property of the query.
The overall goal in the syntax is to preserve the duplication without affecting the core query so that moving from one declaration to multiple and back is as easy as possible:

sequence
  [queryA] by X
  [queryA] by X
  [queryA] by X

// somehow becomes
sequence
  [queryA] by X // make it 3 times

This favors declaring the property as suffix, just like by and with.
Per this PR, another goal was to minimize the amount of symbols so to avoid adding a new one (such as {}), [] was used which at the time seemed like a good idea (famous last words).

Thanks to your feedback we reconsidered the reuse of [] and opted for with since it fits a lot of the properties wanted and continues the style of using prepositions/glue words as markers and block starters.
It keeps the same rules as within sequence declaration (needs to be as a suffix), allows enumeration for possible extensions in the future and allows other potential prepositions to be added further down the road:

// variants

[queryA]
[queryB] by X
[queryC] with runs=3
[queryD] by Z with runs=2

rw-access · 2021-10-07T14:09:18Z

I like it! Thanks!

Allow individual queries within a sequence to be repeated through a dedicated keyword without having physical duplication. Change from using [runs=2] to "with runs=2" Before: sequence queryA [runs=2] queryB queryC [runs=3] queryD Now: sequence queryA with runs=2 queryB queryC with runs=3 queryD Which essentially is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise. Supersedes elastic#75082

Allow individual queries within a sequence to be repeated through a dedicated keyword without having physical duplication. Change from using [runs=2] to "with runs=2" Before: sequence queryA [runs=2] queryB queryC [runs=3] queryD Now: sequence queryA with runs=2 queryB queryC with runs=3 queryD Which essentially is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise. Supersedes #75082

Allow individual queries within a sequence to be repeated through a dedicated keyword without having physical duplication. Change from using [runs=2] to "with runs=2" Before: sequence queryA [runs=2] queryB queryC [runs=3] queryD Now: sequence queryA with runs=2 queryB queryC with runs=3 queryD Which essentially is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise. Supersedes elastic#75082

Allow individual queries within a sequence to be repeated through a dedicated keyword without having physical duplication. Change from using [runs=2] to "with runs=2" Before: sequence queryA [runs=2] queryB queryC [runs=3] queryD Now: sequence queryA with runs=2 queryB queryC with runs=3 queryD Which essentially is the same as: sequence queryA queryA queryB queryC queryC queryC queryD but more concise. Supersedes #75082

costin added 2 commits July 7, 2021 19:41

Revert TOML formatting

7f999ff

costin added the :Analytics/EQL EQL querying label Jul 7, 2021

costin requested a review from astefan July 7, 2021 17:01

elasticsearchmachine added the v8.0.0 label Jul 7, 2021

astefan reviewed Jul 8, 2021

View reviewed changes

costin added 3 commits July 8, 2021 13:52

Address feedback

2ad8d14

Merge remote-tracking branch 'remotes/upstream/master' into eql/repea…

ed19ec4

…table-queries

Merge remote-tracking branch 'remotes/upstream/master' into eql/repea…

a79d365

…table-queries Update EQL repeatable through runs option

costin marked this pull request as ready for review September 29, 2021 10:11

costin added >feature release highlight Team:QL (Deprecated) Meta label for query languages team auto-backport-and-merge v7.16.0 labels Sep 29, 2021

Use correct format in correctness test

a3e0e49

astefan approved these changes Sep 29, 2021

View reviewed changes

costin merged commit c7ef3a6 into elastic:master Sep 29, 2021

costin deleted the eql/repeatable-queries branch September 29, 2021 12:09

costin mentioned this pull request Sep 29, 2021

EQL: Introduce repeatable queries (#75082) #78450

Merged

brokensound77 mentioned this pull request Sep 29, 2021

Add support for repeated sequences under elasticsearch_syntax endgameinc/eql#57

Closed

jrodewig mentioned this pull request Sep 29, 2021

[DOCS] EQL: Document runs keyword #78478

Merged

jrodewig added a commit that referenced this pull request Sep 30, 2021

[DOCS] EQL: Document runs keyword (#78478)

22db8a0

Documents the `runs` keyword for running the same event criteria successively in a sequence query. Relates to #75082.

costin mentioned this pull request Oct 10, 2021

EQL: Refine repeatable queries #78895

Merged

jakelandis added v8.0.0-beta1 and removed v8.0.0 labels Oct 27, 2021

	"Sequence cannot contains more than 256 queries; found [{}]",
	"Sequence cannot contain more than 256 queries; found [{}]",

	throw new ParsingException(source, "A sequence requires a minimum of 2 queries, found [{}]", queries.size());
	throw new ParsingException(source, "A sequence requires a minimum of 2 queries, found [{}]", queries.size());

Conversation

costin commented Jul 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

astefan Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

astefan Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

astefan Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

astefan Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

astefan left a comment

Choose a reason for hiding this comment

Uh oh!

astefan Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

costin Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Sep 29, 2021

💔 Backport failed

Uh oh!

costin commented Sep 29, 2021

Uh oh!

rw-access commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

costin commented Oct 7, 2021

tl;dr

Long story

Uh oh!

rw-access commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

costin commented Jul 7, 2021 •

edited

Loading

rw-access commented Oct 4, 2021 •

edited

Loading

rw-access commented Oct 7, 2021 •

edited

Loading