Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO] Use new Entity v2 endpoints in Inventory #203452

Merged
merged 54 commits into from
Jan 8, 2025

Conversation

rmyz
Copy link
Contributor

@rmyz rmyz commented Dec 9, 2024

Summary

Closes #202295
Closes #202296

This PR adapts Inventory to use the new Entity v2 endpoints.

Testing

  • Use any synthtrace scenario that loads service/hosts/containers data
  • Navigate and make sure everything works as expected (navigation to Discovery/Infra/Services pages, interacting with the table, searching for some specific entity, interacting with the type filter)
  • To check the alerts work, it's easier to connect to a remote cluster.

@rmyz rmyz self-assigned this Dec 9, 2024
@rmyz rmyz changed the title 202295 eco use new entity apis for inventory [ECO] Use new Entity v2 endpoints in Inventory Dec 9, 2024
@rmyz
Copy link
Contributor Author

rmyz commented Dec 9, 2024

/ci

@rmyz
Copy link
Contributor Author

rmyz commented Dec 10, 2024

/ci

@rmyz rmyz marked this pull request as ready for review December 14, 2024 12:41
@rmyz rmyz requested review from a team as code owners December 14, 2024 12:41
@rmyz rmyz added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Dec 14, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Dec 14, 2024
@rmyz
Copy link
Contributor Author

rmyz commented Jan 7, 2025

Hey @iblancof, I took a look at the points that were not working:

  • Missing alerts ✅
    This has been fixed, so it should work from now on. Thanks for catching this up!

  • Old entities index in request present ✅
    When I search any term, it no longer shows that request, so maybe there was leftover code that had been removed.

Error on Kubernetes Clusters entity detail ✅

  • This is because locally, we don't have the dashboards created by default, as depending on the entity, we use a hardcoded ID to show one dashboard or another, so it fails. To make it work, you need to install the Kubernetes integration to have the dashboards locally.

Copy link
Member

@jennypavlova jennypavlova left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 It works as expected :)
🔽 Just some nits

@iblancof
Copy link
Contributor

iblancof commented Jan 8, 2025

Hi @rmyz, thanks for the update!
I tested locally again the PR to double check.

Missing alerts

This has been fixed, so it should work from now on. Thanks for catching this up!

Got the alerts now, nice! 🚀

Old entities index in request present

When I search any term, it no longer shows that request, so maybe there was leftover code that had been removed.

I don't see the requests anymore but it seems the search got broken.
The suggested filters are not appearing anymore:

Current Expected
Screenshot 2025-01-08 at 10 21 16 Screenshot 2025-01-08 at 10 21 29

The request seems to be made to /internal/inventory/entities/types and it feels strange to query that endpoint from the search bar.

Screenshot 2025-01-08 at 10 22 32

Error on Kubernetes Clusters entity detail

This is because locally, we don't have the dashboards created by default, as depending on the entity, we use a hardcoded ID to show one dashboard or another, so it fails. To make it work, you need to install the Kubernetes integration to have the dashboards locally.

I did not try it, but that makes sense, thanks!

@rmyz
Copy link
Contributor Author

rmyz commented Jan 8, 2025

Thanks again for testing it @iblancof

The request seems to be made to /internal/inventory/entities/types and it feels strange to query that endpoint from the search bar.

This is expected, by how the whole flow is built, we first query the entity types, then the entities according to those types, in any case, if we query something that's exclusively for one entity, we would only get that entity type and get the entity once we open the dropdown.

I don't see the requests anymore but it seems the search got broken.
The suggested filters are not appearing anymore

I'm currently investigating this, no changes were made to the search component, so this may be broken for some time.

@rmyz
Copy link
Contributor Author

rmyz commented Jan 8, 2025

I don't see the requests anymore but it seems the search got broken.
The suggested filters are not appearing anymore

I found the issue @iblancof, we are using a wrong dataView, causing the search component to not show filter suggestions.

As the whole Inventory (also search) cannot be used because it's currently broken (EEM did their changes, which are not compatible with what we have) until we merge this PR, we will fix the search in #204026.

@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 8, 2025

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: a9dc255
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-203452-a9dc255c5110

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
inventory 237 236 -1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
entityManager 43 45 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
inventory 231.7KB 231.5KB -249.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
inventory 4 3 -1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
entityManager 8.5KB 8.5KB -5.0B
observabilityShared 93.3KB 93.7KB +442.0B
total +437.0B
Unknown metric groups

API count

id before after diff
entityManager 43 45 +2

History

cc @rmyz

@iblancof
Copy link
Contributor

iblancof commented Jan 8, 2025

I found the issue @iblancof, we are using a wrong dataView, causing the search component to not show filter suggestions.

As the whole Inventory (also search) cannot be used because it's currently broken (EEM did their changes, which are not compatible with what we have) until we merge this PR, we will fix the search in #204026.

Thanks for all the updates @rmyz!

@rmyz rmyz merged commit eb919c9 into elastic:main Jan 8, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 203452

Questions ?

Please refer to the Backport tool documentation

crespocarlos pushed a commit to crespocarlos/kibana that referenced this pull request Jan 8, 2025
## Summary

Closes elastic#202295
Closes elastic#202296

This PR adapts Inventory to use the new Entity v2 endpoints.

## Testing
- Use any synthtrace scenario that loads service/hosts/containers data
- Navigate and make sure everything works as expected (navigation to
Discovery/Infra/Services pages, interacting with the table, searching
for some specific entity, interacting with the type filter)
- To check the alerts work, it's easier to connect to a remote cluster.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Jenny <[email protected]>
@rmyz rmyz deleted the 202295-eco-use-new-entity-apis-for-inventory branch January 8, 2025 14:39
@rmyz
Copy link
Contributor Author

rmyz commented Jan 8, 2025

💚 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

rmyz added a commit to rmyz/kibana that referenced this pull request Jan 8, 2025
## Summary

Closes elastic#202295
Closes elastic#202296

This PR adapts Inventory to use the new Entity v2 endpoints.

## Testing
- Use any synthtrace scenario that loads service/hosts/containers data
- Navigate and make sure everything works as expected (navigation to
Discovery/Infra/Services pages, interacting with the table, searching
for some specific entity, interacting with the type filter)
- To check the alerts work, it's easier to connect to a remote cluster.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Jenny <[email protected]>
(cherry picked from commit eb919c9)

# Conflicts:
#	x-pack/solutions/observability/plugins/inventory/server/routes/entities/get_group_by_terms_agg.test.ts
#	x-pack/solutions/observability/plugins/inventory/server/routes/entities/get_group_by_terms_agg.ts
#	x-pack/solutions/observability/plugins/inventory/server/routes/entities/get_latest_entities_alerts.ts
#	x-pack/solutions/observability/plugins/inventory/server/routes/entities/route.ts
jennypavlova added a commit that referenced this pull request Jan 8, 2025
#205534)

Closes #203095
Closes #204263


## Summary

This PR removes the page for the enabling functionality and welcome
screen. After the migration to v2 API we don't need to enable it anymore
as we are not using transforms

### Before: 


![image](https://github.com/user-attachments/assets/0c8d5841-9189-4551-a1be-87801cfcf57d)


![image](https://github.com/user-attachments/assets/75a73dd8-6b16-452b-abcf-41e1fcca6645)

### After: 
The same without the extra step to enable the entities and the welcome
screen:
- No data: 

![image](https://github.com/user-attachments/assets/dbfdf501-36e7-4b6f-a8b6-ed0a6748ab62)

- With data:

![image](https://github.com/user-attachments/assets/0bbb2983-4e0a-477c-ac11-d2256b5ff854)

## Testing

- In a local environment enable the entities feature flag ( it should be
a clean env as the entities should not be enabled before ):
<img width="1911" alt="image"
src="https://github.com/user-attachments/assets/75d6f77d-5039-41ca-80ca-34c3bf99844e"
/>

- Go to Inventory 
- Check before and after ingesting data
- Synthtrace: `node scripts/synthtrace logs_traces_hosts.ts` - ~⚠️ this
case can't be checked before
#203452 is merged~ -
#203452 is merged 🎉
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 8, 2025
elastic#205534)

Closes elastic#203095
Closes elastic#204263

## Summary

This PR removes the page for the enabling functionality and welcome
screen. After the migration to v2 API we don't need to enable it anymore
as we are not using transforms

### Before:

![image](https://github.com/user-attachments/assets/0c8d5841-9189-4551-a1be-87801cfcf57d)

![image](https://github.com/user-attachments/assets/75a73dd8-6b16-452b-abcf-41e1fcca6645)

### After:
The same without the extra step to enable the entities and the welcome
screen:
- No data:

![image](https://github.com/user-attachments/assets/dbfdf501-36e7-4b6f-a8b6-ed0a6748ab62)

- With data:

![image](https://github.com/user-attachments/assets/0bbb2983-4e0a-477c-ac11-d2256b5ff854)

## Testing

- In a local environment enable the entities feature flag ( it should be
a clean env as the entities should not be enabled before ):
<img width="1911" alt="image"
src="https://github.com/user-attachments/assets/75d6f77d-5039-41ca-80ca-34c3bf99844e"
/>

- Go to Inventory
- Check before and after ingesting data
- Synthtrace: `node scripts/synthtrace logs_traces_hosts.ts` - ~⚠️ this
case can't be checked before
elastic#203452 is merged~ -
elastic#203452 is merged 🎉

(cherry picked from commit d6e28f7)
kibanamachine added a commit that referenced this pull request Jan 8, 2025
…y model (#205534) (#205958)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ECO][Inventory v2] Remove the landing page to enable the entity
model (#205534)](#205534)

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

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

<!--BACKPORT
[{"author":{"name":"jennypavlova","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-08T19:30:17Z","message":"[ECO][Inventory
v2] Remove the landing page to enable the entity model
(#205534)\n\nCloses #203095\r\nCloses
https://github.com/elastic/kibana/issues/204263\r\n\r\n\r\n##
Summary\r\n\r\nThis PR removes the page for the enabling functionality
and welcome\r\nscreen. After the migration to v2 API we don't need to
enable it anymore\r\nas we are not using transforms\r\n\r\n### Before:
\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/0c8d5841-9189-4551-a1be-87801cfcf57d)\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/75a73dd8-6b16-452b-abcf-41e1fcca6645)\r\n\r\n###
After: \r\nThe same without the extra step to enable the entities and
the welcome\r\nscreen:\r\n- No data:
\r\n\r\n![image](https://github.com/user-attachments/assets/dbfdf501-36e7-4b6f-a8b6-ed0a6748ab62)\r\n\r\n-
With
data:\r\n\r\n![image](https://github.com/user-attachments/assets/0bbb2983-4e0a-477c-ac11-d2256b5ff854)\r\n\r\n##
Testing\r\n\r\n- In a local environment enable the entities feature flag
( it should be\r\na clean env as the entities should not be enabled
before ):\r\n<img width=\"1911\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/75d6f77d-5039-41ca-80ca-34c3bf99844e\"\r\n/>\r\n\r\n-
Go to Inventory \r\n- Check before and after ingesting data\r\n-
Synthtrace: `node scripts/synthtrace logs_traces_hosts.ts` - ~⚠️
this\r\ncase can't be checked
before\r\nhttps://github.com//pull/203452 is merged~
-\r\nhttps://github.com//pull/203452 is merged
🎉","sha":"d6e28f766ac5363b4a6888871a2813538a4ee367","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-ux-infra_services"],"title":"[ECO][Inventory
v2] Remove the landing page to enable the entity
model","number":205534,"url":"https://github.com/elastic/kibana/pull/205534","mergeCommit":{"message":"[ECO][Inventory
v2] Remove the landing page to enable the entity model
(#205534)\n\nCloses #203095\r\nCloses
https://github.com/elastic/kibana/issues/204263\r\n\r\n\r\n##
Summary\r\n\r\nThis PR removes the page for the enabling functionality
and welcome\r\nscreen. After the migration to v2 API we don't need to
enable it anymore\r\nas we are not using transforms\r\n\r\n### Before:
\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/0c8d5841-9189-4551-a1be-87801cfcf57d)\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/75a73dd8-6b16-452b-abcf-41e1fcca6645)\r\n\r\n###
After: \r\nThe same without the extra step to enable the entities and
the welcome\r\nscreen:\r\n- No data:
\r\n\r\n![image](https://github.com/user-attachments/assets/dbfdf501-36e7-4b6f-a8b6-ed0a6748ab62)\r\n\r\n-
With
data:\r\n\r\n![image](https://github.com/user-attachments/assets/0bbb2983-4e0a-477c-ac11-d2256b5ff854)\r\n\r\n##
Testing\r\n\r\n- In a local environment enable the entities feature flag
( it should be\r\na clean env as the entities should not be enabled
before ):\r\n<img width=\"1911\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/75d6f77d-5039-41ca-80ca-34c3bf99844e\"\r\n/>\r\n\r\n-
Go to Inventory \r\n- Check before and after ingesting data\r\n-
Synthtrace: `node scripts/synthtrace logs_traces_hosts.ts` - ~⚠️
this\r\ncase can't be checked
before\r\nhttps://github.com//pull/203452 is merged~
-\r\nhttps://github.com//pull/203452 is merged
🎉","sha":"d6e28f766ac5363b4a6888871a2813538a4ee367"}},"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/205534","number":205534,"mergeCommit":{"message":"[ECO][Inventory
v2] Remove the landing page to enable the entity model
(#205534)\n\nCloses #203095\r\nCloses
https://github.com/elastic/kibana/issues/204263\r\n\r\n\r\n##
Summary\r\n\r\nThis PR removes the page for the enabling functionality
and welcome\r\nscreen. After the migration to v2 API we don't need to
enable it anymore\r\nas we are not using transforms\r\n\r\n### Before:
\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/0c8d5841-9189-4551-a1be-87801cfcf57d)\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/75a73dd8-6b16-452b-abcf-41e1fcca6645)\r\n\r\n###
After: \r\nThe same without the extra step to enable the entities and
the welcome\r\nscreen:\r\n- No data:
\r\n\r\n![image](https://github.com/user-attachments/assets/dbfdf501-36e7-4b6f-a8b6-ed0a6748ab62)\r\n\r\n-
With
data:\r\n\r\n![image](https://github.com/user-attachments/assets/0bbb2983-4e0a-477c-ac11-d2256b5ff854)\r\n\r\n##
Testing\r\n\r\n- In a local environment enable the entities feature flag
( it should be\r\na clean env as the entities should not be enabled
before ):\r\n<img width=\"1911\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/75d6f77d-5039-41ca-80ca-34c3bf99844e\"\r\n/>\r\n\r\n-
Go to Inventory \r\n- Check before and after ingesting data\r\n-
Synthtrace: `node scripts/synthtrace logs_traces_hosts.ts` - ~⚠️
this\r\ncase can't be checked
before\r\nhttps://github.com//pull/203452 is merged~
-\r\nhttps://github.com//pull/203452 is merged
🎉","sha":"d6e28f766ac5363b4a6888871a2813538a4ee367"}}]}] BACKPORT-->

Co-authored-by: jennypavlova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ECO][Inventory v2] Fetch all entities [ECO][Inventory v2] Fetch Entity Types
9 participants