[Security][Detection Engine] ESQL Rule Execution Logic Integration Test

[hannahbrooks]

Summary

Resolves #235895

The test that was failing was:

should generate alerts over multiple pages from different indices but same event id for mv_expand when number alerts exceeds max signal

This means that when the same document exists in multiple indices (same id), and a rule uses mv_expand, the system should create alerts from all indices. This can take multiple runs because the number of alerts created may exceed max_signals (maximum number of alerts that can be created per run).

Bug

When the document was being inserted in both indices that the test has, they shared the same timestamp. So when Elasticsearch was pulling these documents, it would be pulling inconsistently from index ecs_compliant and ecs_compliant_synthetic_source leading to unpredictable ordering. updateExcludedDocuments would not consistently receive the same documents to exclude across runs.

Fix

I added an index based increment to the milliseconds of the timestamp. This lets Elasticsearch always be able to order documents when they share an id. This was decided to be work around. Now instead, I have added a tiebreaker to the sort, being the _index to add deterministic ordering.

Testing

I ran a loop to ensure that the test passed at least 10 times in a row.

for i in {1..10}; do
  echo "=== Run $i ==="
  npm run rule_execution_logic:esql:runner:ess > /tmp/test-run-$i.log 2>&1
  if [ $? -ne 0 ]; then
    echo "=== FAILED on run $i — see /tmp/test-run-$i.log ==="
    break
  fi
  echo "=== PASSED run $i ==="
  rm /tmp/test-run-$i.log
done

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• Unit or functional tests were updated or added to match the most common scenarios
• Flaky Test Runner was used on any tests changed
• Review the backport guidelines and apply applicable backport:* labels.

[elasticmachine]

Pinging @elastic/security-detection-engine (Team:Detection Engine)

[kibanamachine]

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#10741

[✅] x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/configs/ess.config.ts: 100/100 tests passed.
[✅] x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/configs/serverless.config.ts: 100/100 tests passed.

see run history

[rylnd]

Thank you for the investigation and writeup, here! Nice work.

However: TL;DR I don't think we should pretend this situation can't happen.

Since we are not fixing the fact of these circumstances, and instead are modifying our test to avoid these circumstances:

How does the nondeterministic ordering exhibited in these test failures affect rule execution?

• If it affects rule execution in any way, I think we need to either fix that behavior, or, if we can't (e.g. due to an ES|QL limitation) or it's determined not to be a bug, then we should at least document how it can manifest/affect things.
• If it can't affect rule execution, then I think we should try instead to make the test robust to that situation (by relaxing constraints within the test, somehow), rather than avoiding the situation entirely.

[hannahbrooks]

Replying to @rylnd:

How does the nondeterministic ordering exhibited in these test failures affect rule execution?
Even though rare, the nondeterministic ordering CAN affect rule execution if the following conditions are present (like in the test):

1. Results hit maxSignals (usually due to using mv_expand)
2. Documents across indices have the same _id and @timestamp (creating no tiebreaker)

Can we fix the behaviour?
What we need to add is some sort of a tiebreaker. However, the sort order is part of the user's query and if we append a second sort we overwrite the original sort. Modifying the existing sort would require parsing the query, which doesn't currently happen. For the test, I can add a sort with _index and a comment why.

Follow-up
I can open an issue to investigate adding _index automatically. There would be some edge cases though (eg. no sort, no _index, already sorting by _index). Maybe that can help us determine if this a bug or if it should be documented.

[kibanamachine]

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#10776

[✅] x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/configs/ess.config.ts: 100/100 tests passed.
[✅] x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/esql/trial_license_complete_tier/configs/serverless.config.ts: 100/100 tests passed.

see run history

[rylnd]

@vitaliidm since you wrote this test initially: do you have any thoughts as to how to best address this? I'm still not quite sure if this is a bug with the rule executor, or simply "user misconfiguration" (i.e. if you only sort on @timestamp, and you're mv_expanding to duplicate both _id and @timestamp, it's possible you'll either miss or duplicate alerts).

[vitaliidm]

I agree with @rylnd

We should
a) try to fix
b) document potential inconsistent behaviour

Documenting can be the easiest way to proceed, considering why this happens and implications:

1. We exclude documents from search because we lack of ES|QL results pagination, so it's already workaround that we would like to get rid off as soon as pagination feature added
2. What is happening is exclusively related with MV_EXPAND usage and multiply alerts creating from a single document
3. It can also only happen when we hit max signals. This narrows down possible occurrences.

What are the implications when we hit this edge case?

Per design, we do not generate more than max signals alerts from a single document.
Worst case: up to max_signals - 1 alerts from one index's copy of the document won't be created across rule re-executions, because the boundary document flips non-deterministically between indices. The document itself still generates alerts from at least one index.

Possible fixes:

Adding _index as a sort in the query by default could prevent this from happening. At the same time it can prevent detecting of alerts from other indices, as adding that sort would prioritise results from one index over the rest(this could happen if user does not use @timestamp or any other sort for their query and could even lead to excluding alerts from events: if we create max_signals from index_1 and during next execution event from index_2 falls our of rule interval)

So, I would suggest

1. adding bullet point in https://www.elastic.co/docs/solutions/security/detect-and-alert/create-detection-rule#esql-query-design, where we already suggest using @timestamp to consider using _index as tiebreaker, whenever MV_EXPAND used
2. Creating an issue where we can dig deeper into what can be done. Btw, we have Automatically inject metadata _id into ES|QL Detection Rules #248194 on adding _id automatically. But it might have some compromises, since any field can be transformed in an ES|QL query

[hannahbrooks]

I agree with @rylnd

We should a) try to fix b) document potential inconsistent behaviour

Documenting can be the easiest way to proceed, considering why this happens and implications:

1. We exclude documents from search because we lack of ES|QL results pagination, so it's already workaround that we would like to get rid off as soon as pagination feature added
2. What is happening is exclusively related with MV_EXPAND usage and multiply alerts creating from a single document
3. It can also only happen when we hit max signals. This narrows down possible occurrences.

What are the implications when we hit this edge case?

Per design, we do not generate more than max signals alerts from a single document. Worst case: up to max_signals - 1 alerts from one index's copy of the document won't be created across rule re-executions, because the boundary document flips non-deterministically between indices. The document itself still generates alerts from at least one index.

Possible fixes:

Adding _index as a sort in the query by default could prevent this from happening. At the same time it can prevent detecting of alerts from other indices, as adding that sort would prioritise results from one index over the rest(this could happen if user does not use @timestamp or any other sort for their query and could even lead to excluding alerts from events: if we create max_signals from index_1 and during next execution event from index_2 falls our of rule interval)

So, I would suggest

1. adding bullet point in https://www.elastic.co/docs/solutions/security/detect-and-alert/create-detection-rule#esql-query-design, where we already suggest using @timestamp to consider using _index as tiebreaker, whenever MV_EXPAND used
2. Creating an issue where we can dig deeper into what can be done. Btw, we have Automatically inject metadata _id into ES|QL Detection Rules #248194 on adding _id automatically. But it might have some compromises, since any field can be transformed in an ES|QL query

Thank you for this input. I agree with your suggestions, and I will proceed with:

• updating the docs
• creating an issue

[rylnd]

The test fix as written looks good, as long as the followup issue is created as discussed here.

P.S. Once this is merged, don't forget to close the associated failed-test issue!

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: d8d01f9

Metrics [docs]

✅ unchanged

History

• 💔 Build #396393 failed e5dd7e2
• 💔 Build #396353 failed 6d41dff
• 💚 Build #394172 succeeded 67cafac

cc @hannahbrooks

[kibanamachine]

Starting backport for target branches: 8.19, 9.2, 9.3

https://github.com/elastic/kibana/actions/runs/22188978052

[kibanamachine]

💔 Some backports could not be created

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts You might need to backport the following PRs to 8.19: - [ska] relocation security_solution_* FTR tests (#231416)
✅	9.2
✅	9.3

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 252936

Questions ?

Please refer to the Backport tool documentation

[hannahbrooks]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation