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

[Inventory][ECO] Create header action menu #193398

Merged
merged 10 commits into from
Sep 20, 2024

Conversation

crespocarlos
Copy link
Contributor

@crespocarlos crespocarlos commented Sep 19, 2024

closes #192326

Summary

This PR introduces the "Add data" item to the header menu:

add_data.mov
image

Note

I have refactored plugin.ts, moving the ReactDOM.render call to application.tsx. I've also created a new component to render the context providers.

useKibana and InventoryKibanaContext were simplified.

Besides, the analytics events created for the EEM Service Inventory 'Add data' button were replicated for this button.

How to test

  • Add xpack.inventory.enabled: true to kibana.dev.yml
  • Start ES and Kibana locally
  • Navigate to Observability -> Inventory

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@crespocarlos crespocarlos force-pushed the 192326-add-data-header-menu branch from 1f15546 to 5f7869c Compare September 19, 2024 09:17
@crespocarlos crespocarlos added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0 labels Sep 19, 2024
@crespocarlos crespocarlos force-pushed the 192326-add-data-header-menu branch from 5f7869c to 3cd4161 Compare September 19, 2024 09:40
@crespocarlos
Copy link
Contributor Author

/ci

@crespocarlos crespocarlos force-pushed the 192326-add-data-header-menu branch from e305ee5 to a728b6e Compare September 19, 2024 12:01
@crespocarlos
Copy link
Contributor Author

/ci

Comment on lines 14 to +15

const useTypedKibana = () => {
return useKibana<InventoryKibanaContext>().services;
};
const useTypedKibana = useKibana as () => KibanaReactContextValue<InventoryKibanaContext>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note

useKibana was simplified to match the standard found across Kibana. core, dependencies, and service are accessed through service

@crespocarlos crespocarlos marked this pull request as ready for review September 19, 2024 12:45
@crespocarlos crespocarlos requested review from a team as code owners September 19, 2024 12:45
@elasticmachine
Copy link
Contributor

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

@crespocarlos crespocarlos added backport:version Backport to applied version labels v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) and removed backport:version Backport to applied version labels labels Sep 19, 2024
@crespocarlos crespocarlos marked this pull request as draft September 19, 2024 13:40
@crespocarlos crespocarlos marked this pull request as ready for review September 19, 2024 13:51
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Sep 20, 2024
@rmyz
Copy link
Contributor

rmyz commented Sep 20, 2024

When running it in local, I'm getting the following error when I enter the Inventory view:
image

Please note that I don't have any entity, which may be the cause
image

Copy link
Contributor

@rmyz rmyz left a comment

Choose a reason for hiding this comment

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

small comments, other than that, LGTM

@crespocarlos
Copy link
Contributor Author

When running it in local, I'm getting the following error when I enter the Inventory view: image

Please note that I don't have any entity, which may be the cause image

Yeah. It was on my task list to create a ticket for that. We need to add an error handling to the API call.

Copy link
Contributor

@rmyz rmyz left a comment

Choose a reason for hiding this comment

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

LGTM

@cauemarcondes
Copy link
Contributor

When running it in local, I'm getting the following error when I enter the Inventory view: image
Please note that I don't have any entity, which may be the cause image

Yeah. It was on my task list to create a ticket for that. We need to add an error handling to the API call.

@crespocarlos you don't need to create a new issue for this. This will be fixed when we work on this issue. The problem happens because EEM hasn't been initialized yet and there's no index to query data from.

@crespocarlos
Copy link
Contributor Author

@crespocarlos you don't need to create a new issue for this. This will be fixed when we work on this issue. The problem happens because EEM hasn't been initialized yet and there's no index to query data from.

@cauemarcondes , we'd still need to handle exceptions on the client side. Not necessarily this problem, but any other possible errors the API may throw.

@cauemarcondes
Copy link
Contributor

we'd still need to handle exceptions on the client side. Not necessarily this problem, but any other possible errors the API may throw.

Sounds good then. I thought you'd be creating an issue for that specific problem.

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@crespocarlos
Copy link
Contributor Author

@elasticmachine merge upstream

@crespocarlos crespocarlos enabled auto-merge (squash) September 20, 2024 14:59
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 20, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
inventory 128 143 +15

Async chunks

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

id before after diff
inventory 51.2KB 59.8KB +8.7KB

Page load bundle

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

id before after diff
inventory 14.9KB 9.5KB -5.3KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@crespocarlos crespocarlos merged commit 1a192bc into elastic:main Sep 20, 2024
31 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 20, 2024
closes [elastic#192326](elastic#192326)

## Summary

This PR introduces the "Add data" item to the header menu:

https://github.com/user-attachments/assets/78ea3667-4ef1-4f02-a513-76e7ca896e67

<img width="600" alt="image"
src="https://github.com/user-attachments/assets/afd21f2d-da66-4d10-83c0-29500591cf3c">

>[!NOTE]
>I have refactored` plugin.ts`, moving the `ReactDOM.render` call to
`application.tsx`. I've also created a new component to render the
context providers.
>
>`useKibana` and `InventoryKibanaContext` were simplified.
>
>Besides, the analytics events created for the EEM Service Inventory
'Add data' button were replicated for this button.

### How to test

- Add `xpack.inventory.enabled: true` to kibana.dev.yml
- Start ES and Kibana locally
- Navigate to Observability -> Inventory

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 1a192bc)
@kibanamachine
Copy link
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 Sep 20, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Inventory][ECO] Create header action menu
(#193398)](#193398)

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

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

<!--BACKPORT [{"author":{"name":"Carlos
Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-20T17:31:54Z","message":"[Inventory][ECO]
Create header action menu (#193398)\n\ncloses
[#192326](https://github.com/elastic/kibana/issues/192326)\r\n\r\n##
Summary\r\n\r\nThis PR introduces the \"Add data\" item to the header
menu:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/78ea3667-4ef1-4f02-a513-76e7ca896e67\r\n\r\n\r\n<img
width=\"600\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/afd21f2d-da66-4d10-83c0-29500591cf3c\">\r\n\r\n\r\n>[!NOTE]\r\n>I
have refactored` plugin.ts`, moving the `ReactDOM.render` call
to\r\n`application.tsx`. I've also created a new component to render
the\r\ncontext providers.\r\n>\r\n>`useKibana` and
`InventoryKibanaContext` were simplified.\r\n>\r\n>Besides, the
analytics events created for the EEM Service Inventory\r\n'Add data'
button were replicated for this button.\r\n\r\n### How to test\r\n\r\n-
Add `xpack.inventory.enabled: true` to kibana.dev.yml\r\n- Start ES and
Kibana locally\r\n- Navigate to Observability ->
Inventory\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"1a192bcc002bdaf31733a3a8f3ec3b62d3b5b8ef","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.16.0"],"title":"[Inventory][ECO]
Create header action
menu","number":193398,"url":"https://github.com/elastic/kibana/pull/193398","mergeCommit":{"message":"[Inventory][ECO]
Create header action menu (#193398)\n\ncloses
[#192326](https://github.com/elastic/kibana/issues/192326)\r\n\r\n##
Summary\r\n\r\nThis PR introduces the \"Add data\" item to the header
menu:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/78ea3667-4ef1-4f02-a513-76e7ca896e67\r\n\r\n\r\n<img
width=\"600\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/afd21f2d-da66-4d10-83c0-29500591cf3c\">\r\n\r\n\r\n>[!NOTE]\r\n>I
have refactored` plugin.ts`, moving the `ReactDOM.render` call
to\r\n`application.tsx`. I've also created a new component to render
the\r\ncontext providers.\r\n>\r\n>`useKibana` and
`InventoryKibanaContext` were simplified.\r\n>\r\n>Besides, the
analytics events created for the EEM Service Inventory\r\n'Add data'
button were replicated for this button.\r\n\r\n### How to test\r\n\r\n-
Add `xpack.inventory.enabled: true` to kibana.dev.yml\r\n- Start ES and
Kibana locally\r\n- Navigate to Observability ->
Inventory\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"1a192bcc002bdaf31733a3a8f3ec3b62d3b5b8ef"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193398","number":193398,"mergeCommit":{"message":"[Inventory][ECO]
Create header action menu (#193398)\n\ncloses
[#192326](https://github.com/elastic/kibana/issues/192326)\r\n\r\n##
Summary\r\n\r\nThis PR introduces the \"Add data\" item to the header
menu:\r\n\r\n\r\nhttps://github.com/user-attachments/assets/78ea3667-4ef1-4f02-a513-76e7ca896e67\r\n\r\n\r\n<img
width=\"600\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/afd21f2d-da66-4d10-83c0-29500591cf3c\">\r\n\r\n\r\n>[!NOTE]\r\n>I
have refactored` plugin.ts`, moving the `ReactDOM.render` call
to\r\n`application.tsx`. I've also created a new component to render
the\r\ncontext providers.\r\n>\r\n>`useKibana` and
`InventoryKibanaContext` were simplified.\r\n>\r\n>Besides, the
analytics events created for the EEM Service Inventory\r\n'Add data'
button were replicated for this button.\r\n\r\n### How to test\r\n\r\n-
Add `xpack.inventory.enabled: true` to kibana.dev.yml\r\n- Start ES and
Kibana locally\r\n- Navigate to Observability ->
Inventory\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"1a192bcc002bdaf31733a3a8f3ec3b62d3b5b8ef"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Carlos Crespo <[email protected]>
@roshan-elastic
Copy link

FYI @akhileshpok - We'll have this similar 'add data' button at top (see video) which will point to either the onboarding journey or docs for adding services from logs.

The same two options will be available from the inventory empty state too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) 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 v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Inventory] Add data link
9 participants