Skip to content

[Security Solution][Detection Engine] adds id to ES bulk payload in data generator factory#231392

Merged
rylnd merged 6 commits intoelastic:mainfrom
vitaliidm:de_9_2/use_id_in_bulk_es_for_data_generator_factory
Aug 13, 2025
Merged

[Security Solution][Detection Engine] adds id to ES bulk payload in data generator factory#231392
rylnd merged 6 commits intoelastic:mainfrom
vitaliidm:de_9_2/use_id_in_bulk_es_for_data_generator_factory

Conversation

@vitaliidm
Copy link
Copy Markdown
Contributor

@vitaliidm vitaliidm commented Aug 12, 2025

Summary

Passing _id for indexed documents should help to prevent flaky tests when number of indexed documents duplicated due to internal JS client retry

@vitaliidm vitaliidm self-assigned this Aug 12, 2025
@vitaliidm vitaliidm added Team:Detection Engine Security Solution Detection Engine Area v9.2.0 v8.19.0 release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels labels Aug 12, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

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

[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/eql/trial_license_complete_tier/configs/ess.config.ts: 200/200 tests passed.

see run history

@vitaliidm vitaliidm marked this pull request as ready for review August 12, 2025 11:01
@vitaliidm vitaliidm requested a review from a team as a code owner August 12, 2025 11:01
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@vitaliidm vitaliidm requested review from a team and rylnd August 12, 2025 11:01
export const indexDocuments: IndexDocuments = async ({ es, documents, index, log }) => {
const operations = documents.flatMap((doc: object) => [{ index: { _index: index } }, doc]);
const operations = documents.flatMap((doc: object) => [
{ index: { _index: index, _id: v4() } },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't appear that we allow passing of an _id parameter; that would probably be useful for cases where we don't want/need to parse it out of the response, or where use of _id is necessary.

However, we also appear to have a common pattern of using an id field as part of the doc, mapping it as a keyword, and then using that field to retrieve the document. While this isn't an official ECS field as far as I can tell, it does serve the same purpose and has the benefit of use in aggregations, etc.

Still: I think it would be nice to support the specifying of an _id field, in cases where we want/need to use that special field.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rylnd

It doesn't appear that we allow passing of an _id parameter; that would probably be useful for cases where we don't want/need to parse it out of the response, or where use of _id is necessary.

Yes, we don't allow. That was not an intention to enhance current interface of dataGeneratorFactory. Just to fix issue with es client and address flaky tests.

However, we also appear to have a common pattern of using an id field as part of the doc, mapping it as a keyword, and then using that field to retrieve the document. While this isn't an official ECS field as far as I can tell, it does serve the same purpose and has the benefit of use in aggregations, etc.

It is widely used to mark events for a test case. Using ECS property tags can be sufficient too for this purpose too

Still: I think it would be nice to support the specifying of an _id field, in cases where we want/need to use that special field.

I added ability to pass _id

@rylnd
Copy link
Copy Markdown
Contributor

rylnd commented Aug 12, 2025

@vitaliidm once this is merged and backported to both 8.19 and 9.1, we will be able to close any of the referenced issues in #230318 that have since been reopened (due to their use of dataGeneratorFactory) 👍

@rylnd
Copy link
Copy Markdown
Contributor

rylnd commented Aug 12, 2025

Judging by these failures, It also looks like #202945 is caused by this dataGeneratorFactory issue; either that or the test is legitimately failing. We'll need to reinspect once this is merged/backported.

@vitaliidm vitaliidm requested a review from rylnd August 13, 2025 10:39
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @vitaliidm

Copy link
Copy Markdown
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

LGTM, this should fix a lottt of our tests 👍

@rylnd rylnd merged commit b450200 into elastic:main Aug 13, 2025
12 checks passed
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.19, 9.1

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 13, 2025
…ata generator factory (elastic#231392)

## Summary

Passing _id for indexed documents should help to prevent flaky tests
when number of indexed documents duplicated due to internal JS client
retry

(cherry picked from commit b450200)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 13, 2025
…ata generator factory (elastic#231392)

## Summary

Passing _id for indexed documents should help to prevent flaky tests
when number of indexed documents duplicated due to internal JS client
retry

(cherry picked from commit b450200)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.19
9.1

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Aug 13, 2025
…d in data generator factory (#231392) (#231686)

# Backport

This will backport the following commits from `main` to `9.1`:
- [[Security Solution][Detection Engine] adds id to ES bulk payload in
data generator factory
(#231392)](#231392)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Vitalii
Dmyterko","email":"92328789+vitaliidm@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-08-13T18:24:16Z","message":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory (#231392)\n\n## Summary\n\nPassing _id for indexed documents
should help to prevent flaky tests\nwhen number of indexed documents
duplicated due to internal JS
client\nretry","sha":"b4502005fb2cf9bf44b96b4f4146e24cff9b8511","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detection
Engine","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory","number":231392,"url":"https://github.com/elastic/kibana/pull/231392","mergeCommit":{"message":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory (#231392)\n\n## Summary\n\nPassing _id for indexed documents
should help to prevent flaky tests\nwhen number of indexed documents
duplicated due to internal JS
client\nretry","sha":"b4502005fb2cf9bf44b96b4f4146e24cff9b8511"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/231392","number":231392,"mergeCommit":{"message":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory (#231392)\n\n## Summary\n\nPassing _id for indexed documents
should help to prevent flaky tests\nwhen number of indexed documents
duplicated due to internal JS
client\nretry","sha":"b4502005fb2cf9bf44b96b4f4146e24cff9b8511"}}]}]
BACKPORT-->

Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com>
kibanamachine added a commit that referenced this pull request Aug 13, 2025
…ad in data generator factory (#231392) (#231685)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Security Solution][Detection Engine] adds id to ES bulk payload in
data generator factory
(#231392)](#231392)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Vitalii
Dmyterko","email":"92328789+vitaliidm@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-08-13T18:24:16Z","message":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory (#231392)\n\n## Summary\n\nPassing _id for indexed documents
should help to prevent flaky tests\nwhen number of indexed documents
duplicated due to internal JS
client\nretry","sha":"b4502005fb2cf9bf44b96b4f4146e24cff9b8511","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detection
Engine","backport:version","v9.1.0","v8.19.0","v9.2.0"],"title":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory","number":231392,"url":"https://github.com/elastic/kibana/pull/231392","mergeCommit":{"message":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory (#231392)\n\n## Summary\n\nPassing _id for indexed documents
should help to prevent flaky tests\nwhen number of indexed documents
duplicated due to internal JS
client\nretry","sha":"b4502005fb2cf9bf44b96b4f4146e24cff9b8511"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"9.1","label":"v9.1.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/231392","number":231392,"mergeCommit":{"message":"[Security
Solution][Detection Engine] adds id to ES bulk payload in data generator
factory (#231392)\n\n## Summary\n\nPassing _id for indexed documents
should help to prevent flaky tests\nwhen number of indexed documents
duplicated due to internal JS
client\nretry","sha":"b4502005fb2cf9bf44b96b4f4146e24cff9b8511"}}]}]
BACKPORT-->

Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com>
fkanout pushed a commit to fkanout/kibana that referenced this pull request Aug 14, 2025
…ata generator factory (elastic#231392)

## Summary

Passing _id for indexed documents should help to prevent flaky tests
when number of indexed documents duplicated due to internal JS client
retry
This was referenced Aug 15, 2025
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Aug 18, 2025
…ata generator factory (elastic#231392)

## Summary

Passing _id for indexed documents should help to prevent flaky tests
when number of indexed documents duplicated due to internal JS client
retry
qn895 pushed a commit to qn895/kibana that referenced this pull request Aug 26, 2025
…ata generator factory (elastic#231392)

## Summary

Passing _id for indexed documents should help to prevent flaky tests
when number of indexed documents duplicated due to internal JS client
retry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area v8.19.3 v9.1.3 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants