Skip to content

EQL: backport fix for async missing events and re-enable the feature#98130

Merged
luigidellaquila merged 2 commits intoelastic:8.9from
luigidellaquila:eql/backport_async_missing
Aug 3, 2023
Merged

EQL: backport fix for async missing events and re-enable the feature#98130
luigidellaquila merged 2 commits intoelastic:8.9from
luigidellaquila:eql/backport_async_missing

Conversation

@luigidellaquila
Copy link
Contributor

This PR backports #97718 to v 8.9.1

The problem: missing events were represented as null values in intermediate states (before converting them to {missing: true} JSON), but in some circumstances events have to be serialized, eg. when executing an async search, and that caused a NPE.

The fix: this PR replaces the null placeholder with a constant Event instance to represent missing events, allowing proper serialization.

Backport details: In the main branch, the fix also includes a TransportVersion change to add the new "missing" attribute to the serialization. Since backporting this part is particularly tricky, here we are using a work-around to avoid a version increment: we'll consider events with _index: "" (empty index) as missing events; the missing attribute won't be serialized, preserving the existing serialization format.

@luigidellaquila luigidellaquila added the :Analytics/EQL EQL querying label Aug 2, 2023
@luigidellaquila luigidellaquila requested a review from astefan August 2, 2023 09:50
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

@elasticsearchmachine elasticsearchmachine added Team:QL (Deprecated) Meta label for query languages team v8.9.1 labels Aug 2, 2023
@elasticsearchmachine
Copy link
Collaborator

Hi @luigidellaquila, I've created a changelog YAML for you.

@ChrisHegarty
Copy link
Contributor

..
Backport details: In the main branch, the fix also includes a TransportVersion change to add the new "missing" attribute to the serialization. Since backporting this part is particularly tricky, here we are using a work-around to avoid a version increment: we'll consider events with _index: "" (empty index) as missing events; the missing attribute won't be serialized, preserving the existing serialization format.

In this particular case, I agree that the workaround ( as you have described ) is the safer way to proceed, rather than trying to determine the Transport range to eliminate and individual checks to update in main.

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

@luigidellaquila
Copy link
Contributor Author

Thank you @ChrisHegarty @astefan!
Merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/EQL EQL querying >bug Team:QL (Deprecated) Meta label for query languages team v8.9.1

4 participants