Skip to content

Conversation

@costin
Copy link
Member

@costin costin commented Feb 27, 2020

Metric mappings have lots of mappings which, in case of individual field selection, can easily hit the limit of maximum doc value fields or automatons.

As fields are not used in EQL, this commit disables field extraction and returns the entire source instead.

@costin costin added the :Analytics/EQL EQL querying label Feb 27, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/EQL)

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM. Left two minor comments.


// NB: the sortBuilder takes care of eliminating duplicates
container.fields().forEach(f -> f.v1().collectFields(sortBuilder));
sortBuilder.build(source);
Copy link
Contributor

Choose a reason for hiding this comment

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

The sortBuilder is still defined above. Needs removal? Also, the two comments above are not needed anymore.

String now = DateUtils.nowWithMillisResolution().format(DateTimeFormatter.ISO_DATE_TIME);
StringBuilder sb = new StringBuilder();
sb.append("{");
for (int i = 0; i < 250; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a comment here explaining the reason for 250 value (that value larger than the 100 doc_values limit in ES). Maybe take the default limit in ES and use here a value made of that limit + 100 or something like that?

@costin
Copy link
Member Author

costin commented Feb 28, 2020

@elasticmachine run elasticsearch-ci/bwc

@costin costin merged commit 79ca586 into elastic:master Feb 28, 2020
@costin costin deleted the eql/source-only branch February 28, 2020 11:45
costin added a commit that referenced this pull request Feb 28, 2020
Return the whole source of matching events

(cherry picked from commit 79ca586)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/EQL EQL querying

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants