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.

Sign up for GitHub

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

Jump to bottom

[Logs UI] Register Logs UI Locators #155156

Merged
merged 40 commits into from
May 17, 2023
Merged

[Logs UI] Register Logs UI Locators #155156

merged 40 commits into from
May 17, 2023

Conversation

mohamedhamed-ahmed
Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed commented Apr 18, 2023 •
edited
Loading

Summary

Closes #104855

This PR creates 2 registered locators:

  1. Logs Locator
  2. Node Logs Locator

With these 2 locators, we now have a typed navigation to the logs UI which also redirects to Discover in serverless mode.

Testing

Normal behaviour

When Kibana is used as always then on any navigation to the logs UI, the user will be redirected to the stream UI.
All links to link-to routes should still behave as before.

  • Launch the Kibana dev environment with yarn start
  • Navigate to Hosts UI
  • Click the logs tab
  • Add a filter text in the search bar
  • Click on the Open in Logs link
  • Verify that navigation to the Stream view and the state is maintained
Screen.Recording.2023-04-26.at.10.13.02.mov

Serverless behaviour

When Kibana is used in serverless mode, we want to redirect any user landing to Logs UI to the Discover page

  • Launch the Kibana dev environment with yarn serverless-oblt
  • Navigate to Hosts UI
  • Click the logs tab
  • Add a filter text in the search bar
  • Click on the Open in Logs link
  • Verify to be redirected to Discover and that the state is maintained
Screen.Recording.2023-04-26.at.10.15.52.mov

Next Steps

A separate PR will be created to fulfill the below AC

  • All usages of link-to routes in other apps are replaced with usage of the appropriate locator.

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

mohamedhamed-ahmed
mohamedhamed-ahmed commented Apr 24, 2023
View reviewed changes
x-pack/plugins/infra/public/pages/link_to/link_to_logs.test.tsx Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with Functional Tests

mohamedhamed-ahmed
mohamedhamed-ahmed commented Apr 24, 2023
View reviewed changes
x-pack/plugins/infra/public/pages/link_to/redirect_to_logs.test.tsx Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with Functional Tests

@mohamedhamed-ahmed mohamedhamed-ahmed added v8.9.0 Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Apr 24, 2023
@mohamedhamed-ahmed
Allow defining time range in locators
be89547
@mohamedhamed-ahmed
Code Refactoring
47ec73a
@mohamedhamed-ahmed
Code Refactoring
37044d0
@mohamedhamed-ahmed
Merge branch 'main' of https://github.com/elastic/kibana into 104855-…
f33bd7c 
…register-logs-ui-kibana-locators
@mohamedhamed-ahmed
Merge branch 'main' of https://github.com/elastic/kibana into 104855-…
1a8c68a 
…register-logs-ui-kibana-locators
@mohamedhamed-ahmed
Merge branch 'main' of https://github.com/elastic/kibana into 104855-…
f600d36 
…register-logs-ui-kibana-locators
@mohamedhamed-ahmed
Merge branch 'main' of https://github.com/elastic/kibana into 104855-…
0887034 
…register-logs-ui-kibana-locators
@mohamedhamed-ahmed
Code Refactoring
d93c5c9
@mohamedhamed-ahmed
Remove unused references
10eeddd
@mohamedhamed-ahmed
Awaited on import to lower bundle size
470c441
@mohamedhamed-ahmed
Code Refactoring
8fae828
@mohamedhamed-ahmed
Added redirection to discover in serverless
5a16193
@mohamedhamed-ahmed
Fix Tests
69d0f6a
@mohamedhamed-ahmed
change from and to to be timeRange
2e88f65
@mohamedhamed-ahmed
Update discover redirection to support query params
1bee11b
@mohamedhamed-ahmed
bug fix
b038085
@mohamedhamed-ahmed
Merge Main
a546606
@mohamedhamed-ahmed
Code Refactoring
2647b58
@mohamedhamed-ahmed
Code Refactoring
877a9ee
@mohamedhamed-ahmed
Fix bundle size
ec50509
Kerry350
Kerry350 approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

Thanks for making the requested changes. The split registration we talked about also looks good 👌

Kerry350
Kerry350 approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

Thanks for making the requested changes. The split registration we talked about also looks good 👌

Kerry350
Kerry350 approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

Thanks for making the requested changes. The split registration we talked about also looks good 👌

@Kerry350
Copy link
Contributor

(Sorry for the approval spam, GitHub wasn't responding when I clicked approve 🙈)

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1400 1406 +6

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
infra 45 46 +1

Async chunks

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

id before after diff
infra 2.0MB 2.0MB +6.0KB

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
infra 13 14 +1

Page load bundle

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

id before after diff
infra 102.3KB 104.3KB +2.0KB
Unknown metric groups

API count

id before after diff
infra 48 49 +1

async chunk count

id before after diff
infra 28 30 +2

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 400 404 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 480 484 +4
total +6

History

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

@mohamedhamed-ahmed mohamedhamed-ahmed merged commit 5d96ef9 into elastic:main May 17, 2023
@mohamedhamed-ahmed mohamedhamed-ahmed mentioned this pull request May 17, 2023
Closed
2 tasks
jasonrhodes pushed a commit that referenced this pull request May 17, 2023
@mohamedhamed-ahmed @jasonrhodes
[Logs UI] Register Logs UI Locators (#155156)
6cbfd71 
## Summary

Closes #104855

This PR creates 2 registered locators:

1. Logs Locator
2. Node Logs Locator

With these 2 locators, we now have a typed navigation to the logs UI
which also redirects to Discover in serverless mode.

## Testing

### Normal behaviour

When Kibana is used as always then on any navigation to the logs UI, the
user will be redirected to the stream UI.
All links to `link-to` routes should still behave as before.
- Launch the Kibana dev environment with `yarn start`
- Navigate to Hosts UI
- Click the logs tab
- Add a filter text in the search bar
- Click on the Open in Logs link
- Verify that navigation to the Stream view and the state is maintained



https://user-images.githubusercontent.com/11225826/234514430-ddc1ffaa-0cb2-4f2a-84e9-6c6230937d9f.mov



### Serverless behaviour

When Kibana is used in serverless mode, we want to redirect any user
landing to Logs UI to the Discover page
- Launch the Kibana dev environment with `yarn serverless-oblt`
- Navigate to Hosts UI
- Click the logs tab
- Add a filter text in the search bar
- Click on the Open in Logs link
- Verify to be redirected to Discover and that the state is maintained



https://user-images.githubusercontent.com/11225826/234514454-dfb2774e-d6f1-4f4c-ba10-77815dc1ae9d.mov


### Next Steps
A separate PR will be created to fulfill the below AC

- All usages of link-to routes in other apps are replaced with usage of
the appropriate locator.
This was referenced May 24, 2023
Merged
Merged
Merged
mohamedhamed-ahmed added a commit that referenced this pull request May 25, 2023
@mohamedhamed-ahmed @kibanamachine
[Logs+] Replace Monitoring usages of link-to routes (#158367)
43d751e 
part of #157985

## 📝  Summary

After implementing [infra
locators](#155156) to allow
navigation to the logs UI we need to replace all usages of the old
link-to routes so that we have strongly typed navigation to the logs UI.

This PR focuses on replacing `link-to` usages in the Monitoring plugin.

## ✅  Testing

1. Navigate to Stack Monitoring
2. Choose any of the clusters from the list
3. Click logs under Elasticsearch
4. Scroll to the Recent Log Entries section 
5. click on the Logs link `Visit Logs to dive deeper.`



https://github.com/elastic/kibana/assets/11225826/b54da395-4693-4588-a8a3-6b05fdfed1c5

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
mohamedhamed-ahmed added a commit that referenced this pull request May 30, 2023
@mohamedhamed-ahmed
[APM UI] Replace logs link-to routes with appropriate locator (#158365)
adb05f6 
part of #157985

## 📝  Summary

After implementing [infra
locators](#155156) to allow
navigation to the logs UI we need to replace all usages of the old
link-to routes so that we have strongly typed navigation to the logs UI.

This PR focuses on replacing `link-to` usages in the APM plugin.

## ✅  Testing

1. Navigate to APM -> Services
2. Follow video below to navigate to where the logs links are located
3. Links should behave as before and maintain navigation params



https://github.com/elastic/kibana/assets/11225826/c631ea31-c5d9-4d05-9219-7b208daf2c37
mohamedhamed-ahmed added a commit that referenced this pull request May 30, 2023
@mohamedhamed-ahmed
[Logs+] Replace infra public usages of link-to routes (#158362)
c06b5ef 
part of #157985

## 📝  Summary

After implementing [infra
locators](#155156) to allow
navigation to the logs UI we need to replace all usages of the old
link-to routes so that we have strongly typed navigation to the logs UI.

This PR focuses on replacing `link-to` usages in the Infra Public
plugin.

## ✅  Testing

A) 
1. Navigate to Observability -> Infrastructure -> Inventory
2. Choose any of the Host from the waffle
3. Click logs tab
4. Click open in logs link

B) 
1. Navigate to Observability -> Infrastructure -> Inventory
2. Switch to table view
3. Click any of the Host from the table
4. Click Host Logs


https://github.com/elastic/kibana/assets/11225826/6646c496-1760-4788-9532-b318487070d1
@mohamedhamed-ahmed mohamedhamed-ahmed mentioned this pull request Jun 6, 2023
Merged
mohamedhamed-ahmed added a commit that referenced this pull request Jun 8, 2023
@mohamedhamed-ahmed
[Logs+] Replace usages of link-to routes in infra server (#158994)
3785c93 
closes [#157985](#157985)

## 📝  Summary

This PR moves the previously implemented [infra
locators](#155156) to infra common
and replaces the last usage of `link-to` routes in the infra public with
the appropriate locator.

## ✅  Testing

1. Navigate to Observability Alerts
2. Create a new Log Threshold Rule that should fire
3. Open the alert details flyout
4. Click on the `View in app` button
5. Should correctly navigate to Stream view or Discover in case of
serverless.
* All other links to Logs should still work properly as before as well 



https://github.com/elastic/kibana/assets/11225826/9af85ea6-5fdd-4505-9707-6489b760f311
@weltenwort weltenwort mentioned this pull request Aug 30, 2023
Closed
@criamico criamico mentioned this pull request Jul 9, 2024
Merged
criamico added a commit that referenced this pull request Jul 10, 2024
@criamico @kibanamachine @elasticmachine
[Fleet] Display view in logs button when logs app is available (#187871)
696bb88 
Closes #185711

## Summary
This change fixes #185711, but
while working on that I also realised that we should move away from
using hardcoded urls. So this PR does two things:
- Displays the button only when the user has `authz.fleet.readAgents`
privilege
- Refactors the button functionality to use the new locators that take
care of linking to the observability logs/discover app

### Why the refactor
While testing this button, I noticed that the functionality was broken
in some cases, that's because we were manually routing the urls to Logs
UI/Discover apps based if we are in serverless or not.

I found a PR that already implements this functionality:
#155156
I also found #154145 that takes
care of the redirect to the correct app.
So I'm replacing the current manual functionality with these utilities
so that `getLogsLocatorsFromUrlService` takes care of where the open in
logs button should link.



### ESS


https://github.com/elastic/kibana/assets/16084106/3f0760c9-3afb-4793-a3af-317f625b36d7


https://github.com/elastic/kibana/assets/16084106/3436cf5a-36c9-425d-a114-e116ddaa1a03

### Serverless

https://github.com/elastic/kibana/assets/16084106/84176f09-96a4-4932-9508-5f7682d03aae

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 10, 2024
@criamico
[Fleet] Display view in logs button when logs app is available (elast…
c85fd47 
…ic#187871)

Closes elastic#185711

## Summary
This change fixes elastic#185711, but
while working on that I also realised that we should move away from
using hardcoded urls. So this PR does two things:
- Displays the button only when the user has `authz.fleet.readAgents`
privilege
- Refactors the button functionality to use the new locators that take
care of linking to the observability logs/discover app

### Why the refactor
While testing this button, I noticed that the functionality was broken
in some cases, that's because we were manually routing the urls to Logs
UI/Discover apps based if we are in serverless or not.

I found a PR that already implements this functionality:
elastic#155156
I also found elastic#154145 that takes
care of the redirect to the correct app.
So I'm replacing the current manual functionality with these utilities
so that `getLogsLocatorsFromUrlService` takes care of where the open in
logs button should link.

### ESS

https://github.com/elastic/kibana/assets/16084106/3f0760c9-3afb-4793-a3af-317f625b36d7

https://github.com/elastic/kibana/assets/16084106/3436cf5a-36c9-425d-a114-e116ddaa1a03

### Serverless

https://github.com/elastic/kibana/assets/16084106/84176f09-96a4-4932-9508-5f7682d03aae

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 696bb88)
kibanamachine added a commit that referenced this pull request Jul 10, 2024
@kibanamachine @criamico
[8.15] [Fleet] Display view in logs button when logs app is available (…
494668e 
…#187871) (#188000)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Fleet] Display view in logs button when logs app is available
(#187871)](#187871)

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

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

<!--BACKPORT [{"author":{"name":"Cristina
Amico","email":"criamico@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-07-10T15:17:31Z","message":"[Fleet]
Display view in logs button when logs app is available
(#187871)\n\nCloses
https://github.com/elastic/kibana/issues/185711\r\n\r\n##
Summary\r\nThis change fixes
#185711, but\r\nwhile working on
that I also realised that we should move away from\r\nusing hardcoded
urls. So this PR does two things:\r\n- Displays the button only when the
user has `authz.fleet.readAgents`\r\nprivilege\r\n- Refactors the button
functionality to use the new locators that take\r\ncare of linking to
the observability logs/discover app\r\n\r\n### Why the refactor\r\nWhile
testing this button, I noticed that the functionality was broken\r\nin
some cases, that's because we were manually routing the urls to
Logs\r\nUI/Discover apps based if we are in serverless or not.\r\n\r\nI
found a PR that already implements this
functionality:\r\nhttps://github.com//pull/155156\r\nI
also found #154145 that
takes\r\ncare of the redirect to the correct app.\r\nSo I'm replacing
the current manual functionality with these utilities\r\nso that
`getLogsLocatorsFromUrlService` takes care of where the open in\r\nlogs
button should link.\r\n\r\n\r\n\r\n###
ESS\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/3f0760c9-3afb-4793-a3af-317f625b36d7\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/3436cf5a-36c9-425d-a114-e116ddaa1a03\r\n\r\n###
Serverless\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/84176f09-96a4-4932-9508-5f7682d03aae\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"696bb88d7c33eebfeabec6064ea8a97a2e2bb1bb","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v8.15.0","v8.16.0"],"title":"[Fleet]
Display view in logs button when logs app is
available","number":187871,"url":"https://github.com/elastic/kibana/pull/187871","mergeCommit":{"message":"[Fleet]
Display view in logs button when logs app is available
(#187871)\n\nCloses
https://github.com/elastic/kibana/issues/185711\r\n\r\n##
Summary\r\nThis change fixes
#185711, but\r\nwhile working on
that I also realised that we should move away from\r\nusing hardcoded
urls. So this PR does two things:\r\n- Displays the button only when the
user has `authz.fleet.readAgents`\r\nprivilege\r\n- Refactors the button
functionality to use the new locators that take\r\ncare of linking to
the observability logs/discover app\r\n\r\n### Why the refactor\r\nWhile
testing this button, I noticed that the functionality was broken\r\nin
some cases, that's because we were manually routing the urls to
Logs\r\nUI/Discover apps based if we are in serverless or not.\r\n\r\nI
found a PR that already implements this
functionality:\r\nhttps://github.com//pull/155156\r\nI
also found #154145 that
takes\r\ncare of the redirect to the correct app.\r\nSo I'm replacing
the current manual functionality with these utilities\r\nso that
`getLogsLocatorsFromUrlService` takes care of where the open in\r\nlogs
button should link.\r\n\r\n\r\n\r\n###
ESS\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/3f0760c9-3afb-4793-a3af-317f625b36d7\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/3436cf5a-36c9-425d-a114-e116ddaa1a03\r\n\r\n###
Serverless\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/84176f09-96a4-4932-9508-5f7682d03aae\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"696bb88d7c33eebfeabec6064ea8a97a2e2bb1bb"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187871","number":187871,"mergeCommit":{"message":"[Fleet]
Display view in logs button when logs app is available
(#187871)\n\nCloses
https://github.com/elastic/kibana/issues/185711\r\n\r\n##
Summary\r\nThis change fixes
#185711, but\r\nwhile working on
that I also realised that we should move away from\r\nusing hardcoded
urls. So this PR does two things:\r\n- Displays the button only when the
user has `authz.fleet.readAgents`\r\nprivilege\r\n- Refactors the button
functionality to use the new locators that take\r\ncare of linking to
the observability logs/discover app\r\n\r\n### Why the refactor\r\nWhile
testing this button, I noticed that the functionality was broken\r\nin
some cases, that's because we were manually routing the urls to
Logs\r\nUI/Discover apps based if we are in serverless or not.\r\n\r\nI
found a PR that already implements this
functionality:\r\nhttps://github.com//pull/155156\r\nI
also found #154145 that
takes\r\ncare of the redirect to the correct app.\r\nSo I'm replacing
the current manual functionality with these utilities\r\nso that
`getLogsLocatorsFromUrlService` takes care of where the open in\r\nlogs
button should link.\r\n\r\n\r\n\r\n###
ESS\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/3f0760c9-3afb-4793-a3af-317f625b36d7\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/3436cf5a-36c9-425d-a114-e116ddaa1a03\r\n\r\n###
Serverless\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/84176f09-96a4-4932-9508-5f7682d03aae\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"696bb88d7c33eebfeabec6064ea8a97a2e2bb1bb"}}]}]
BACKPORT-->

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

@weltenwort weltenwort weltenwort left review comments

@Kerry350 Kerry350 Kerry350 approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Logs UI] Register Kibana locators to allow linking to the Logs UI
6 participants
@mohamedhamed-ahmed @apmmachine @elasticmachine @Kerry350 @kibana-ci @weltenwort