Skip to content

[eem] _search fixes + integration tests#203403

Merged
klacabane merged 24 commits intoelastic:mainfrom
klacabane:eem-search-api-tests
Dec 12, 2024
Merged

[eem] _search fixes + integration tests#203403
klacabane merged 24 commits intoelastic:mainfrom
klacabane:eem-search-api-tests

Conversation

@klacabane
Copy link
Copy Markdown
Contributor

@klacabane klacabane commented Dec 9, 2024

  • small refactor of _search code which is now resilient to source-level failure. We now return { entities: EntityV2[], errors: string[] } where errors lists source failures while still collecting successful sources in entities
  • add integration tests
  • fixes some issues in the merging logic uncovered by the tests
  • change metadata aggregation from VALUES to TOP 10 since the former is in tech preview

@klacabane klacabane self-assigned this Dec 9, 2024
@klacabane klacabane added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Team:obs-entities DEPRECATED - Observability Entities Team labels Dec 9, 2024
@klacabane klacabane marked this pull request as ready for review December 9, 2024 12:36
@klacabane klacabane requested a review from a team as a code owner December 9, 2024 12:36
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-entities (Team:obs-entities)

Copy link
Copy Markdown
Contributor

@miltonhultgren miltonhultgren left a comment

Choose a reason for hiding this comment

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

Should we add some tests for filtering and additional metadata aggs?

@klacabane
Copy link
Copy Markdown
Contributor Author

Should we add some tests for filtering and additional metadata aggs?

a bit noisy but added in 6f4ec6e

@elastic-vault-github-plugin-prod elastic-vault-github-plugin-prod bot requested a review from a team as a code owner December 10, 2024 19:10
Comment on lines 640 to 643
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.

Nice, and here we see why it would be valuable for customers to add a field alias in this case instead but now we at least have tests for this 👍🏼

Copy link
Copy Markdown
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

x-pack/test/tsconfig.json changes LGTM

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #81 / Entity Manager _search API includes source and additional metadata fields

Metrics [docs]

✅ unchanged

History

cc @klacabane

@klacabane klacabane merged commit b866e32 into elastic:main Dec 12, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

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

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Dec 12, 2024
- small refactor of _search code which is now resilient to source-level
failure. We now return `{ entities: EntityV2[], errors: string[] }`
where `errors` lists source failures while still collecting successful
sources in `entities`
- add integration tests
- fixes some issues in the merging logic uncovered by the tests
- change metadata aggregation from `VALUES` to `TOP 10` since the former
is in tech preview

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit b866e32)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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 Dec 12, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[eem] _search fixes + integration tests
(#203403)](#203403)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Kevin
Lacabane","email":"kevin.lacabane@elastic.co"},"sourceCommit":{"committedDate":"2024-12-12T09:11:14Z","message":"[eem]
_search fixes + integration tests (#203403)\n\n- small refactor of
_search code which is now resilient to source-level\r\nfailure. We now
return `{ entities: EntityV2[], errors: string[] }`\r\nwhere `errors`
lists source failures while still collecting successful\r\nsources in
`entities`\r\n- add integration tests\r\n- fixes some issues in the
merging logic uncovered by the tests\r\n- change metadata aggregation
from `VALUES` to `TOP 10` since the former\r\nis in tech
preview\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"b866e3290d98556078b7f124495de6b8706d46bb","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Team:obs-entities"],"title":"[eem]
_search fixes + integration
tests","number":203403,"url":"https://github.com/elastic/kibana/pull/203403","mergeCommit":{"message":"[eem]
_search fixes + integration tests (#203403)\n\n- small refactor of
_search code which is now resilient to source-level\r\nfailure. We now
return `{ entities: EntityV2[], errors: string[] }`\r\nwhere `errors`
lists source failures while still collecting successful\r\nsources in
`entities`\r\n- add integration tests\r\n- fixes some issues in the
merging logic uncovered by the tests\r\n- change metadata aggregation
from `VALUES` to `TOP 10` since the former\r\nis in tech
preview\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"b866e3290d98556078b7f124495de6b8706d46bb"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203403","number":203403,"mergeCommit":{"message":"[eem]
_search fixes + integration tests (#203403)\n\n- small refactor of
_search code which is now resilient to source-level\r\nfailure. We now
return `{ entities: EntityV2[], errors: string[] }`\r\nwhere `errors`
lists source failures while still collecting successful\r\nsources in
`entities`\r\n- add integration tests\r\n- fixes some issues in the
merging logic uncovered by the tests\r\n- change metadata aggregation
from `VALUES` to `TOP 10` since the former\r\nis in tech
preview\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"b866e3290d98556078b7f124495de6b8706d46bb"}}]}]
BACKPORT-->

Co-authored-by: Kevin Lacabane <kevin.lacabane@elastic.co>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
- small refactor of _search code which is now resilient to source-level
failure. We now return `{ entities: EntityV2[], errors: string[] }`
where `errors` lists source failures while still collecting successful
sources in `entities`
- add integration tests
- fixes some issues in the merging logic uncovered by the tests
- change metadata aggregation from `VALUES` to `TOP 10` since the former
is in tech preview

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:obs-entities DEPRECATED - Observability Entities Team v8.18.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants