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

[One Discover] Add app menu actions for Observability projects #198987

Merged
merged 24 commits into from
Nov 14, 2024

Conversation

tonyghiani
Copy link
Contributor

@tonyghiani tonyghiani commented Nov 5, 2024

📓 Summary

Closes #182230

This work introduces a new observability root profile and uses the new extension point to register custom actions on the app menu.
The registered actions and link will appear only with the new project navigation enabled on an Observability project:

  • A link to the data sets quality page
  • On the alerts sub menu...
    • replace the default search rule creation with the observability custom threshold rule
    • add an entry to directly create an SLO for the current search

To access the SLO capabilities without breaking the dependencies hierarchy of the new sustainable architecture, the feature is registered by the common plugin discover-shared in SLO and consumed then by Discover using the IoC principle.

🖼️ Screenshots

Observability project solution - show new menu

Screenshot 2024-11-06 at 12 37 02

Search project solution - hide new menu

Screenshot 2024-11-06 at 12 36 19

Default navigation mode - hide new menu

Screenshot 2024-11-06 at 12 35 43

🎥 Demo

Screen.Recording.2024-11-06.at.12.56.37.mov

@tonyghiani tonyghiani changed the title [One Discover] Add app menu actions on for Observability projects [One Discover] Add app menu actions for Observability projects Nov 5, 2024
@tonyghiani tonyghiani 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-logs Observability Logs User Experience Team Project:OneDiscover Enrich Discover with contextual awareness labels Nov 6, 2024
@tonyghiani tonyghiani marked this pull request as ready for review November 6, 2024 13:53
@tonyghiani tonyghiani requested review from a team as code owners November 6, 2024 13:53
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Kibana.jsonc LGTM

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Nov 6, 2024
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

github-actions bot commented Nov 6, 2024

🤖 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!)

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Code-only review so far, I'll test locally tomorrow and review again. Code changes look great! Just one request: any chance we could add some simple functional tests for the app menu in the O11y profile?

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Code changes look good and it's working well overall! Just noticed a few things while testing to look into.

});
};

const registerCreateSLOAction = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I ran into an issue where the request failed when I tried to create an SLO, seemingly because of the filter I was using in Discover:
CleanShot 2024-11-07 at 23 58 17@2x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reproduced it but it looks like a bug on the SLOs end, we are just embedding the creation flyout.
Could you open an issue for the SLOs team please?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was unable to reproduce this outside of Discover directly in the SLOs app. It seems they might be stripping the $state property from filters or something similar before saving:
CleanShot 2024-11-12 at 22 09 27@2x

Do you know of another area in Kibana where I can create SLOs to try to reproduce it? Mainly I'm unsure if this is a bug that affects other usages too, or if the expectation is that consumers somehow modify the filters as they do before saving.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey Davis, I tried this on Logs Explorer too and it is affected in the same way.
As you thought, the expectation is that the filters don't have the $state property, which is reason why it fails validating the payload sent to the server and fails the SLO creation.

I stripped the property client side and it works fine now

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I confirmed it works now!

Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed left a comment

Choose a reason for hiding this comment

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

Obs-ux-logs code changes review only, LGTM!

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Another round of review! Looks good to me, the only thing I'd like to understand before approving is if the SLO filters bug is really something encountered elsewhere or if we are supposed to be doing something to handle it in Discover. My other feedback is not blocking.

});
};

const registerCreateSLOAction = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I was unable to reproduce this outside of Discover directly in the SLOs app. It seems they might be stripping the $state property from filters or something similar before saving:
CleanShot 2024-11-12 at 22 09 27@2x

Do you know of another area in Kibana where I can create SLOs to try to reproduce it? Mainly I'm unsure if this is a bug that affects other usages too, or if the expectation is that consumers somehow modify the filters as they do before saving.

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Great work! The SLO issue is resolved and this now LGTM 👍

});
};

const registerCreateSLOAction = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I confirmed it works now!

@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 14, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 65d854d
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-198987-65d854dbc5ce

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #92 / aiops change point detection allows change point selection for detailed view

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 972 976 +4

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
slo 44 45 +1

Async chunks

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

id before after diff
discover 806.8KB 809.6KB +2.7KB

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
discoverShared 2 3 +1
slo 3 4 +1
total +2

Page load bundle

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

id before after diff
discover 51.0KB 51.0KB +32.0B
slo 28.5KB 28.6KB +100.0B
total +132.0B
Unknown metric groups

API count

id before after diff
slo 44 45 +1

History

@tonyghiani tonyghiani merged commit 9d38922 into elastic:main Nov 14, 2024
32 checks passed
@tonyghiani tonyghiani deleted the 182230-app-menu-logs-experience branch November 14, 2024 10:00
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

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

You might need to backport the following PRs to 8.x:
- chore(slo): Migrate to server-route-repository (#198726)

Manual backport

To create the backport manually run:

node scripts/backport --pr 198987

Questions ?

Please refer to the Backport tool documentation

@tonyghiani
Copy link
Contributor Author

💚 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

tonyghiani added a commit to tonyghiani/kibana that referenced this pull request Nov 14, 2024
…ic#198987)

## 📓 Summary

Closes elastic#182230

This work introduces a new observability root profile and uses the new
extension point to register custom actions on the app menu.
The registered actions and link will appear only with the new project
navigation enabled on an Observability project:
- A link to the data sets quality page
- On the alerts sub menu...
- replace the default search rule creation with the observability custom
threshold rule
  - add an entry to directly create an SLO for the current search

To access the SLO capabilities without breaking the dependencies
hierarchy of the new sustainable architecture, the feature is registered
by the common plugin `discover-shared` in SLO and consumed then by
Discover using the IoC principle.

## 🖼️ Screenshots

### Observability project solution - show new menu

<img width="3004" alt="Screenshot 2024-11-06 at 12 37 02"
src="https://github.com/user-attachments/assets/d70b532d-1889-4d5b-b2ee-de2f048560f4">

### Search project solution - hide new menu

<img width="3006" alt="Screenshot 2024-11-06 at 12 36 19"
src="https://github.com/user-attachments/assets/660893c3-f6b5-4b06-b8de-50a61a6bdb98">

### Default navigation mode - hide new menu

<img width="3002" alt="Screenshot 2024-11-06 at 12 35 43"
src="https://github.com/user-attachments/assets/674c5a08-0084-40e5-ae34-a56c363cacce">

## 🎥  Demo

https://github.com/user-attachments/assets/104e6074-0401-4fd2-a8e6-8b05f2c070d7

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 9d38922)

# Conflicts:
#	x-pack/plugins/observability_solution/slo/public/pages/slo_edit/shared_flyout/get_create_slo_flyout.tsx
#	x-pack/plugins/observability_solution/slo/public/plugin.ts
#	x-pack/plugins/observability_solution/slo/public/types.ts
#	x-pack/plugins/observability_solution/slo/tsconfig.json
#	x-pack/test_serverless/functional/test_suites/common/discover_ml_uptime/discover/search_source_alert.ts
tonyghiani added a commit that referenced this pull request Nov 14, 2024
…198987) (#200145)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[One Discover] Add app menu actions for Observability projects
(#198987)](#198987)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Marco Antonio
Ghiani","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-14T10:00:38Z","message":"[One
Discover] Add app menu actions for Observability projects
(#198987)\n\n## 📓 Summary\r\n\r\nCloses #182230 \r\n\r\nThis work
introduces a new observability root profile and uses the
new\r\nextension point to register custom actions on the app
menu.\r\nThe registered actions and link will appear only with the new
project\r\nnavigation enabled on an Observability project:\r\n- A link
to the data sets quality page\r\n- On the alerts sub menu...\r\n-
replace the default search rule creation with the observability
custom\r\nthreshold rule\r\n - add an entry to directly create an SLO
for the current search\r\n\r\nTo access the SLO capabilities without
breaking the dependencies\r\nhierarchy of the new sustainable
architecture, the feature is registered\r\nby the common plugin
`discover-shared` in SLO and consumed then by\r\nDiscover using the IoC
principle.\r\n\r\n## 🖼️ Screenshots\r\n\r\n### Observability project
solution - show new menu\r\n\r\n<img width=\"3004\" alt=\"Screenshot
2024-11-06 at 12 37
02\"\r\nsrc=\"https://github.com/user-attachments/assets/d70b532d-1889-4d5b-b2ee-de2f048560f4\">\r\n\r\n###
Search project solution - hide new menu\r\n\r\n<img width=\"3006\"
alt=\"Screenshot 2024-11-06 at 12 36
19\"\r\nsrc=\"https://github.com/user-attachments/assets/660893c3-f6b5-4b06-b8de-50a61a6bdb98\">\r\n\r\n###
Default navigation mode - hide new menu\r\n\r\n<img width=\"3002\"
alt=\"Screenshot 2024-11-06 at 12 35
43\"\r\nsrc=\"https://github.com/user-attachments/assets/674c5a08-0084-40e5-ae34-a56c363cacce\">\r\n\r\n##
🎥
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/104e6074-0401-4fd2-a8e6-8b05f2c070d7\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani <[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"9d38922401d0bbd0d95d750f68fec77ca22758fb","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.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-logs","Team:obs-ux-management","Project:OneDiscover"],"number":198987,"url":"https://github.com/elastic/kibana/pull/198987","mergeCommit":{"message":"[One
Discover] Add app menu actions for Observability projects
(#198987)\n\n## 📓 Summary\r\n\r\nCloses #182230 \r\n\r\nThis work
introduces a new observability root profile and uses the
new\r\nextension point to register custom actions on the app
menu.\r\nThe registered actions and link will appear only with the new
project\r\nnavigation enabled on an Observability project:\r\n- A link
to the data sets quality page\r\n- On the alerts sub menu...\r\n-
replace the default search rule creation with the observability
custom\r\nthreshold rule\r\n - add an entry to directly create an SLO
for the current search\r\n\r\nTo access the SLO capabilities without
breaking the dependencies\r\nhierarchy of the new sustainable
architecture, the feature is registered\r\nby the common plugin
`discover-shared` in SLO and consumed then by\r\nDiscover using the IoC
principle.\r\n\r\n## 🖼️ Screenshots\r\n\r\n### Observability project
solution - show new menu\r\n\r\n<img width=\"3004\" alt=\"Screenshot
2024-11-06 at 12 37
02\"\r\nsrc=\"https://github.com/user-attachments/assets/d70b532d-1889-4d5b-b2ee-de2f048560f4\">\r\n\r\n###
Search project solution - hide new menu\r\n\r\n<img width=\"3006\"
alt=\"Screenshot 2024-11-06 at 12 36
19\"\r\nsrc=\"https://github.com/user-attachments/assets/660893c3-f6b5-4b06-b8de-50a61a6bdb98\">\r\n\r\n###
Default navigation mode - hide new menu\r\n\r\n<img width=\"3002\"
alt=\"Screenshot 2024-11-06 at 12 35
43\"\r\nsrc=\"https://github.com/user-attachments/assets/674c5a08-0084-40e5-ae34-a56c363cacce\">\r\n\r\n##
🎥
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/104e6074-0401-4fd2-a8e6-8b05f2c070d7\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani <[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"9d38922401d0bbd0d95d750f68fec77ca22758fb"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198987","number":198987,"mergeCommit":{"message":"[One
Discover] Add app menu actions for Observability projects
(#198987)\n\n## 📓 Summary\r\n\r\nCloses #182230 \r\n\r\nThis work
introduces a new observability root profile and uses the
new\r\nextension point to register custom actions on the app
menu.\r\nThe registered actions and link will appear only with the new
project\r\nnavigation enabled on an Observability project:\r\n- A link
to the data sets quality page\r\n- On the alerts sub menu...\r\n-
replace the default search rule creation with the observability
custom\r\nthreshold rule\r\n - add an entry to directly create an SLO
for the current search\r\n\r\nTo access the SLO capabilities without
breaking the dependencies\r\nhierarchy of the new sustainable
architecture, the feature is registered\r\nby the common plugin
`discover-shared` in SLO and consumed then by\r\nDiscover using the IoC
principle.\r\n\r\n## 🖼️ Screenshots\r\n\r\n### Observability project
solution - show new menu\r\n\r\n<img width=\"3004\" alt=\"Screenshot
2024-11-06 at 12 37
02\"\r\nsrc=\"https://github.com/user-attachments/assets/d70b532d-1889-4d5b-b2ee-de2f048560f4\">\r\n\r\n###
Search project solution - hide new menu\r\n\r\n<img width=\"3006\"
alt=\"Screenshot 2024-11-06 at 12 36
19\"\r\nsrc=\"https://github.com/user-attachments/assets/660893c3-f6b5-4b06-b8de-50a61a6bdb98\">\r\n\r\n###
Default navigation mode - hide new menu\r\n\r\n<img width=\"3002\"
alt=\"Screenshot 2024-11-06 at 12 35
43\"\r\nsrc=\"https://github.com/user-attachments/assets/674c5a08-0084-40e5-ae34-a56c363cacce\">\r\n\r\n##
🎥
Demo\r\n\r\n\r\nhttps://github.com/user-attachments/assets/104e6074-0401-4fd2-a8e6-8b05f2c070d7\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani <[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"9d38922401d0bbd0d95d750f68fec77ca22758fb"}}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…ic#198987)

## 📓 Summary

Closes elastic#182230 

This work introduces a new observability root profile and uses the new
extension point to register custom actions on the app menu.
The registered actions and link will appear only with the new project
navigation enabled on an Observability project:
- A link to the data sets quality page
- On the alerts sub menu...
- replace the default search rule creation with the observability custom
threshold rule
  - add an entry to directly create an SLO for the current search

To access the SLO capabilities without breaking the dependencies
hierarchy of the new sustainable architecture, the feature is registered
by the common plugin `discover-shared` in SLO and consumed then by
Discover using the IoC principle.

## 🖼️ Screenshots

### Observability project solution - show new menu

<img width="3004" alt="Screenshot 2024-11-06 at 12 37 02"
src="https://github.com/user-attachments/assets/d70b532d-1889-4d5b-b2ee-de2f048560f4">

### Search project solution - hide new menu

<img width="3006" alt="Screenshot 2024-11-06 at 12 36 19"
src="https://github.com/user-attachments/assets/660893c3-f6b5-4b06-b8de-50a61a6bdb98">

### Default navigation mode - hide new menu

<img width="3002" alt="Screenshot 2024-11-06 at 12 35 43"
src="https://github.com/user-attachments/assets/674c5a08-0084-40e5-ae34-a56c363cacce">

## 🎥  Demo


https://github.com/user-attachments/assets/104e6074-0401-4fd2-a8e6-8b05f2c070d7

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…ic#198987)

## 📓 Summary

Closes elastic#182230 

This work introduces a new observability root profile and uses the new
extension point to register custom actions on the app menu.
The registered actions and link will appear only with the new project
navigation enabled on an Observability project:
- A link to the data sets quality page
- On the alerts sub menu...
- replace the default search rule creation with the observability custom
threshold rule
  - add an entry to directly create an SLO for the current search

To access the SLO capabilities without breaking the dependencies
hierarchy of the new sustainable architecture, the feature is registered
by the common plugin `discover-shared` in SLO and consumed then by
Discover using the IoC principle.

## 🖼️ Screenshots

### Observability project solution - show new menu

<img width="3004" alt="Screenshot 2024-11-06 at 12 37 02"
src="https://github.com/user-attachments/assets/d70b532d-1889-4d5b-b2ee-de2f048560f4">

### Search project solution - hide new menu

<img width="3006" alt="Screenshot 2024-11-06 at 12 36 19"
src="https://github.com/user-attachments/assets/660893c3-f6b5-4b06-b8de-50a61a6bdb98">

### Default navigation mode - hide new menu

<img width="3002" alt="Screenshot 2024-11-06 at 12 35 43"
src="https://github.com/user-attachments/assets/674c5a08-0084-40e5-ae34-a56c363cacce">

## 🎥  Demo


https://github.com/user-attachments/assets/104e6074-0401-4fd2-a8e6-8b05f2c070d7

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Co-authored-by: kibanamachine <[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 Project:OneDiscover Enrich Discover with contextual awareness release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team Team:obs-ux-management Observability Management User Experience Team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover][Observability] Register Logs Explorer top navigation
8 participants