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

[Fleet] Display view in logs button when logs app is available #187871

Merged
merged 8 commits into from
Jul 10, 2024
Merged

[Fleet] Display view in logs button when logs app is available #187871

merged 8 commits into from
Jul 10, 2024

Conversation

criamico
Copy link
Contributor

@criamico criamico commented Jul 9, 2024 •
edited by kibanamachine
Loading

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

Screen.Recording.2024-07-09.at.15.06.11.mov
Screen.Recording.2024-07-09.at.15.09.04.mov

Serverless

Serverless_obs.mov

@criamico criamico self-assigned this Jul 9, 2024
@criamico criamico added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 9, 2024
@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!)

@criamico criamico added the release_note:skip Skip the PR/issue when compiling release notes label Jul 9, 2024
@criamico criamico changed the title 185711 view in logs button [Fleet] Fix view in logs button by using logsLocator from getLogsLocatorsFromUrlService Jul 9, 2024
@criamico
Copy link
Contributor Author

criamico commented Jul 9, 2024

/ci

@criamico criamico changed the title [Fleet] Fix view in logs button by using logsLocator from getLogsLocatorsFromUrlService [Fleet] Display view in logs button when logs app is available Jul 10, 2024
@criamico criamico marked this pull request as ready for review July 10, 2024 08:30
@criamico criamico requested a review from a team as a code owner July 10, 2024 08:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@criamico
Copy link
Contributor Author

@elasticmachine merge upstream

juliaElastic
juliaElastic approved these changes Jul 10, 2024
View reviewed changes
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

Looks great! Is there any unit tests to add on the getFormattedRange function?

@criamico
Copy link
Contributor Author

criamico commented Jul 10, 2024 •
edited
Loading

Is there any unit tests to add on the getFormattedRange function?

@juliaElastic all it does is to convert the string values startTime and endTime to number. Strangely this new locator accepts the range values in unix format rather than the usual strings, so I had to add this small utility.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 1210 1205 -5

Async chunks

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

id before after diff
fleet 1.8MB 1.8MB -9.6KB

Page load bundle

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

id before after diff
fleet 167.4KB 167.6KB +290.0B

History

cc @criamico

@criamico criamico merged commit 696bb88 into elastic:main Jul 10, 2024
22 checks passed
@criamico criamico deleted the 185711_view_in_logs_button branch July 10, 2024 15:17
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 <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 696bb88)
@kibanamachine kibanamachine mentioned this pull request Jul 10, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.15

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 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":"[email protected]"},"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
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","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
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","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
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"696bb88d7c33eebfeabec6064ea8a97a2e2bb1bb"}}]}]
BACKPORT-->

Co-authored-by: Cristina Amico <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliaElastic juliaElastic juliaElastic approved these changes

Assignees

@criamico criamico

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.15.0 v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Enhancement][Fleet]: Open in logs should be disabled when Logs tab access is not provided.
5 participants
@criamico @obltmachine @elasticmachine @kibanamachine @juliaElastic