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

[Logs] Update logs stream links to logs explorer #190835

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

Kerry350
Copy link
Contributor

@Kerry350 Kerry350 commented Aug 20, 2024

Summary

Implements #190782, as part of the deprecate Logs UI project: https://github.com/elastic/observability-dev/issues/3242.

Overview

  • Previously the top level logs locators would redirect to the locators registered in Infra first (for the stream), and logs explorer second if the stream wasn't available. This fork no longer exists, with the top level locators navigating to the logs explorer only.

  • Infra no longer registers locators for the logs stream.

  • Consumer / solution uses have been updated.

Semi-breaking change

  • Previously we had link-to routes that would translate into an internal link. These no longer accept a log view (as this is a stream concept, and is also soon to be deprecated itself). This means bookmarked link-to routes may now ultimately point to slightly different data (depending on the users log sources advanced setting). I don't really see a full proof way around this if we are to truly deprecate all the pieces we need. Also, with the link-to routes always being a translation layer this might be absolutely fine — we are just translating things differently now.

Reviewer notes

  • The ES Deprecation logs page can be found at: /app/management/stack/upgrade_assistant/es_deprecation_logs.

Change points

(Not fully exhaustive)

Screenshot 2024-08-30 at 17 04 24

Screenshot 2024-08-30 at 17 15 37

Screenshot 2024-08-30 at 17 18 51

Screenshot 2024-08-30 at 17 21 32

Screenshot 2024-08-30 at 17 23 35

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

@Kerry350
Copy link
Contributor Author

/ci

@Kerry350 Kerry350 force-pushed the 190782-update-open-in-logs-links branch from 9d90093 to e4faf21 Compare August 21, 2024 15:27
@Kerry350
Copy link
Contributor Author

/ci

@Kerry350 Kerry350 force-pushed the 190782-update-open-in-logs-links branch 3 times, most recently from 2e4531c to 21f58f7 Compare August 23, 2024 18:18
@Kerry350
Copy link
Contributor Author

/ci

@Kerry350 Kerry350 force-pushed the 190782-update-open-in-logs-links branch from 21f58f7 to 8a7e511 Compare August 23, 2024 22:12
@Kerry350
Copy link
Contributor Author

/ci

@Kerry350 Kerry350 force-pushed the 190782-update-open-in-logs-links branch from 8a7e511 to 29c86fd Compare August 30, 2024 15:58
@Kerry350
Copy link
Contributor Author

/ci

@Kerry350 Kerry350 marked this pull request as ready for review September 2, 2024 10:56
@Kerry350 Kerry350 requested review from a team as code owners September 2, 2024 10:57
@Kerry350
Copy link
Contributor Author

Kerry350 commented Sep 2, 2024

@elasticmachine merge upstream

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-management Observability Management User Experience Team labels Sep 2, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@elasticmachine
Copy link
Contributor

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

@Kerry350 Kerry350 removed the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 2, 2024
@botelastic botelastic bot added Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-management Observability Management User Experience Team labels Sep 2, 2024
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

@tonyghiani tonyghiani self-requested a review September 4, 2024 10:10
@@ -86,6 +86,9 @@ export const LogEntryTimestampColumn = dynamic(
export const ScrollableLogTextStreamView = dynamic(
() => import('./components/logging/log_text_stream/scrollable_log_text_stream_view')
);
export const OpenInLogsExplorerButton = dynamic(
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 4, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1504 1500 -4
upgradeAssistant 169 178 +9
total +5

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 35 34 -1
logsShared 282 281 -1
total -2

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 -15.0B
infra 1.5MB 1.5MB -8.4KB
logsShared 139.6KB 140.3KB +719.0B
observability 462.0KB 462.2KB +203.0B
upgradeAssistant 157.0KB 157.2KB +172.0B
total -7.4KB

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 6 5 -1

Page load bundle

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

id before after diff
fleet 169.9KB 169.7KB -176.0B
infra 97.4KB 96.5KB -997.0B
logsShared 173.7KB 173.4KB -354.0B
observability 102.4KB 102.6KB +140.0B
upgradeAssistant 24.7KB 24.6KB -107.0B
total -1.5KB
Unknown metric groups

API count

id before after diff
infra 38 37 -1

async chunk count

id before after diff
infra 17 16 -1
logsShared 13 14 +1
total -0

ESLint disabled line counts

id before after diff
infra 39 40 +1
logsShared 17 18 +1
total +2

Total ESLint disabled count

id before after diff
infra 48 49 +1
logsShared 20 21 +1
total +2

History

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

cc @Kerry350

Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@ElenaStoeva ElenaStoeva left a comment

Choose a reason for hiding this comment

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

Upgrade assistant changes lgtm, verified locally.

@gbamparop
Copy link
Contributor

Semi-breaking change

  • Previously we had link-to routes that would translate into an internal link. These no longer accept a log view (as this is a stream concept, and is also soon to be deprecated itself). This means bookmarked link-to routes may now ultimately point to slightly different data (depending on the users log sources advanced setting). I don't really see a full proof way around this if we are to truly deprecate all the pieces we need. Also, with the link-to routes always being a translation layer this might be absolutely fine — we are just translating things differently now.

We've discussed this with @LucaWintergerst and @alex-fedotyev today, it's an edge case and shouldn't be a blocker for this PR.

@Kerry350 Kerry350 merged commit 773f3b3 into elastic:main Sep 5, 2024
26 checks passed
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Sep 5, 2024
mohamedhamed-ahmed added a commit that referenced this pull request Nov 14, 2024
## Summary

Fixes
#199902 (comment).

This was missed in #190835 due to
Stack Monitoring's lack of type checking in certain files. `infra` no
longer exposes `logsLocators`.

This uses `getLogsLocatorsFromUrlService`, technically we could go to
Discover now knowing that Logs Explorer will be deprecated in the
future. But it will make more sense to convert
`getLogsLocatorsFromUrlService` over to using the Discover locators in
one when that happens. This puts us on the same page as
#190835.

This link should now work, and have the correct filters applied.

![Screenshot 2024-11-13 at 16 09
15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 14, 2024
…0043)

## Summary

Fixes
elastic#199902 (comment).

This was missed in elastic#190835 due to
Stack Monitoring's lack of type checking in certain files. `infra` no
longer exposes `logsLocators`.

This uses `getLogsLocatorsFromUrlService`, technically we could go to
Discover now knowing that Logs Explorer will be deprecated in the
future. But it will make more sense to convert
`getLogsLocatorsFromUrlService` over to using the Discover locators in
one when that happens. This puts us on the same page as
elastic#190835.

This link should now work, and have the correct filters applied.

![Screenshot 2024-11-13 at 16 09
15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
(cherry picked from commit 4e852ea)
mohamedhamed-ahmed pushed a commit to mohamedhamed-ahmed/kibana that referenced this pull request Nov 14, 2024
…0043)

## Summary

Fixes
elastic#199902 (comment).

This was missed in elastic#190835 due to
Stack Monitoring's lack of type checking in certain files. `infra` no
longer exposes `logsLocators`.

This uses `getLogsLocatorsFromUrlService`, technically we could go to
Discover now knowing that Logs Explorer will be deprecated in the
future. But it will make more sense to convert
`getLogsLocatorsFromUrlService` over to using the Discover locators in
one when that happens. This puts us on the same page as
elastic#190835.

This link should now work, and have the correct filters applied.

![Screenshot 2024-11-13 at 16 09
15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
(cherry picked from commit 4e852ea)

# Conflicts:
#	x-pack/plugins/monitoring/kibana.jsonc
kibanamachine added a commit that referenced this pull request Nov 14, 2024
) (#200227)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Stack Monitoring / Logs] Fix Stack Monitoring logs links
(#200043)](#200043)

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

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

<!--BACKPORT [{"author":{"name":"Kerry
Gallagher","email":"kerry.gallagher@elastic.co"},"sourceCommit":{"committedDate":"2024-11-14T17:26:24Z","message":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs links (#200043)\n\n##
Summary\r\n\r\nFixes\r\nhttps://github.com//issues/199902#issuecomment-2474040264.\r\n\r\nThis
was missed in #190835 due
to\r\nStack Monitoring's lack of type checking in certain files. `infra`
no\r\nlonger exposes `logsLocators`.\r\n\r\nThis uses
`getLogsLocatorsFromUrlService`, technically we could go to\r\nDiscover
now knowing that Logs Explorer will be deprecated in the\r\nfuture. But
it will make more sense to convert\r\n`getLogsLocatorsFromUrlService`
over to using the Discover locators in\r\none when that happens. This
puts us on the same page
as\r\nhttps://github.com//pull/190835.\r\n\r\nThis link
should now work, and have the correct filters
applied.\r\n\r\n![Screenshot 2024-11-13 at 16
09\r\n15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)\r\n\r\n---------\r\n\r\nCo-authored-by:
mohamedhamed-ahmed
<mohamed.ahmed@elastic.co>","sha":"4e852ea041b63e3e3ad918ceee1bc3861dd1e519","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:cloud-deploy","Team:obs-ux-logs","backport:version","v8.17.0","v8.16.1"],"title":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs
links","number":200043,"url":"https://github.com/elastic/kibana/pull/200043","mergeCommit":{"message":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs links (#200043)\n\n##
Summary\r\n\r\nFixes\r\nhttps://github.com//issues/199902#issuecomment-2474040264.\r\n\r\nThis
was missed in #190835 due
to\r\nStack Monitoring's lack of type checking in certain files. `infra`
no\r\nlonger exposes `logsLocators`.\r\n\r\nThis uses
`getLogsLocatorsFromUrlService`, technically we could go to\r\nDiscover
now knowing that Logs Explorer will be deprecated in the\r\nfuture. But
it will make more sense to convert\r\n`getLogsLocatorsFromUrlService`
over to using the Discover locators in\r\none when that happens. This
puts us on the same page
as\r\nhttps://github.com//pull/190835.\r\n\r\nThis link
should now work, and have the correct filters
applied.\r\n\r\n![Screenshot 2024-11-13 at 16
09\r\n15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)\r\n\r\n---------\r\n\r\nCo-authored-by:
mohamedhamed-ahmed
<mohamed.ahmed@elastic.co>","sha":"4e852ea041b63e3e3ad918ceee1bc3861dd1e519"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200043","number":200043,"mergeCommit":{"message":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs links (#200043)\n\n##
Summary\r\n\r\nFixes\r\nhttps://github.com//issues/199902#issuecomment-2474040264.\r\n\r\nThis
was missed in #190835 due
to\r\nStack Monitoring's lack of type checking in certain files. `infra`
no\r\nlonger exposes `logsLocators`.\r\n\r\nThis uses
`getLogsLocatorsFromUrlService`, technically we could go to\r\nDiscover
now knowing that Logs Explorer will be deprecated in the\r\nfuture. But
it will make more sense to convert\r\n`getLogsLocatorsFromUrlService`
over to using the Discover locators in\r\none when that happens. This
puts us on the same page
as\r\nhttps://github.com//pull/190835.\r\n\r\nThis link
should now work, and have the correct filters
applied.\r\n\r\n![Screenshot 2024-11-13 at 16
09\r\n15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)\r\n\r\n---------\r\n\r\nCo-authored-by:
mohamedhamed-ahmed
<mohamed.ahmed@elastic.co>","sha":"4e852ea041b63e3e3ad918ceee1bc3861dd1e519"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kerry Gallagher <kerry.gallagher@elastic.co>
mohamedhamed-ahmed added a commit that referenced this pull request Nov 14, 2024
…0043) (#200231)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Stack Monitoring / Logs] Fix Stack Monitoring logs links
(#200043)](#200043)

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

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

<!--BACKPORT [{"author":{"name":"Kerry
Gallagher","email":"kerry.gallagher@elastic.co"},"sourceCommit":{"committedDate":"2024-11-14T17:26:24Z","message":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs links (#200043)\n\n##
Summary\r\n\r\nFixes\r\nhttps://github.com//issues/199902#issuecomment-2474040264.\r\n\r\nThis
was missed in #190835 due
to\r\nStack Monitoring's lack of type checking in certain files. `infra`
no\r\nlonger exposes `logsLocators`.\r\n\r\nThis uses
`getLogsLocatorsFromUrlService`, technically we could go to\r\nDiscover
now knowing that Logs Explorer will be deprecated in the\r\nfuture. But
it will make more sense to convert\r\n`getLogsLocatorsFromUrlService`
over to using the Discover locators in\r\none when that happens. This
puts us on the same page
as\r\nhttps://github.com//pull/190835.\r\n\r\nThis link
should now work, and have the correct filters
applied.\r\n\r\n![Screenshot 2024-11-13 at 16
09\r\n15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)\r\n\r\n---------\r\n\r\nCo-authored-by:
mohamedhamed-ahmed
<mohamed.ahmed@elastic.co>","sha":"4e852ea041b63e3e3ad918ceee1bc3861dd1e519","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:cloud-deploy","Team:obs-ux-logs","backport:version","v8.17.0","v8.16.1"],"number":200043,"url":"https://github.com/elastic/kibana/pull/200043","mergeCommit":{"message":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs links (#200043)\n\n##
Summary\r\n\r\nFixes\r\nhttps://github.com//issues/199902#issuecomment-2474040264.\r\n\r\nThis
was missed in #190835 due
to\r\nStack Monitoring's lack of type checking in certain files. `infra`
no\r\nlonger exposes `logsLocators`.\r\n\r\nThis uses
`getLogsLocatorsFromUrlService`, technically we could go to\r\nDiscover
now knowing that Logs Explorer will be deprecated in the\r\nfuture. But
it will make more sense to convert\r\n`getLogsLocatorsFromUrlService`
over to using the Discover locators in\r\none when that happens. This
puts us on the same page
as\r\nhttps://github.com//pull/190835.\r\n\r\nThis link
should now work, and have the correct filters
applied.\r\n\r\n![Screenshot 2024-11-13 at 16
09\r\n15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)\r\n\r\n---------\r\n\r\nCo-authored-by:
mohamedhamed-ahmed
<mohamed.ahmed@elastic.co>","sha":"4e852ea041b63e3e3ad918ceee1bc3861dd1e519"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200043","number":200043,"mergeCommit":{"message":"[Stack
Monitoring / Logs] Fix Stack Monitoring logs links (#200043)\n\n##
Summary\r\n\r\nFixes\r\nhttps://github.com//issues/199902#issuecomment-2474040264.\r\n\r\nThis
was missed in #190835 due
to\r\nStack Monitoring's lack of type checking in certain files. `infra`
no\r\nlonger exposes `logsLocators`.\r\n\r\nThis uses
`getLogsLocatorsFromUrlService`, technically we could go to\r\nDiscover
now knowing that Logs Explorer will be deprecated in the\r\nfuture. But
it will make more sense to convert\r\n`getLogsLocatorsFromUrlService`
over to using the Discover locators in\r\none when that happens. This
puts us on the same page
as\r\nhttps://github.com//pull/190835.\r\n\r\nThis link
should now work, and have the correct filters
applied.\r\n\r\n![Screenshot 2024-11-13 at 16
09\r\n15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)\r\n\r\n---------\r\n\r\nCo-authored-by:
mohamedhamed-ahmed
<mohamed.ahmed@elastic.co>","sha":"4e852ea041b63e3e3ad918ceee1bc3861dd1e519"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/200227","number":200227,"state":"OPEN"},{"branch":"8.16","label":"v8.16.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kerry Gallagher <kerry.gallagher@elastic.co>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…0043)

## Summary

Fixes
elastic#199902 (comment).

This was missed in elastic#190835 due to
Stack Monitoring's lack of type checking in certain files. `infra` no
longer exposes `logsLocators`.

This uses `getLogsLocatorsFromUrlService`, technically we could go to
Discover now knowing that Logs Explorer will be deprecated in the
future. But it will make more sense to convert
`getLogsLocatorsFromUrlService` over to using the Discover locators in
one when that happens. This puts us on the same page as
elastic#190835.

This link should now work, and have the correct filters applied.

![Screenshot 2024-11-13 at 16 09
15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…0043)

## Summary

Fixes
elastic#199902 (comment).

This was missed in elastic#190835 due to
Stack Monitoring's lack of type checking in certain files. `infra` no
longer exposes `logsLocators`.

This uses `getLogsLocatorsFromUrlService`, technically we could go to
Discover now knowing that Logs Explorer will be deprecated in the
future. But it will make more sense to convert
`getLogsLocatorsFromUrlService` over to using the Discover locators in
one when that happens. This puts us on the same page as
elastic#190835.

This link should now work, and have the correct filters applied.

![Screenshot 2024-11-13 at 16 09
15](https://github.com/user-attachments/assets/e1f8fd18-ac73-4179-af4c-1727a2afeeec)

---------

Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-logs Observability Logs User Experience Team Team:obs-ux-management Observability Management User Experience Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.