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

[LogsUI] Add UI setting to hide Logs Stream and dashboard panel option #194519

Merged

Conversation

tonyghiani
Copy link
Contributor

@tonyghiani tonyghiani commented Oct 1, 2024

Release note

The logs stream app is hidden by default - users are advised to use the logs explorer app instead. The logs explorer can be found in the left hand observability menu under Logs > Explorer. When using Elastic Cloud and the observability navigation, Logs Explorer will be displayed as a separate tab under the "Discover" menu item. To re-enable the logs stream app, navigate to Stack management > Advanced settings and enable the observability:enableLogsStream setting.

📓 Summary

Closes #193319
Closes #193320

This work is part of the effort to progressively deprecate the existing Logs Stream feature.

Changes taken on this PR consist of:

  • Create a new uiSettings observability:enableLogsStream which defaults to false on the stateful/cloud deployments and is not available in serverless ones (still, defaults to false behind the scene).
  • When observability:enableLogsStream is false, the Logs Stream page route is not registered, and neither is its deep link for global search.
  • When observability:enableLogsStream is false, the panels list on Dashboard won't show anymore the option Logs Stream (Deprecated) to prevent usage of this embeddable in new dashboards. The embeddable is still registered for retro-compatibility with active dashboards, and it has now a callout explaining the status of this embeddable (unmaintained/deprecated).
  • Rename logs ML to "Logs Anomalies" and "Logs Categories".

Other minor improvements regard:

  • Remove duplicate Xstate utils and use the relative package instead.
  • Remove the duplicated useBoolean hook used in the deprecation callout.
  • Sync deep links registration with available routes through a single getLogsRoutes util.

🎥 Recordings

Logs Stream app removed

remove_logs_stream_app.mov

Logs Stream dashboard panel entry removed

remove_logs_steram_panel.mov

Logs Stream app removed from project nav

remove_logs_stream_project.mov

Embeddable deprecation callout

Screenshot 2024-10-02 at 10 22 09

Unavailable setting in serverless

serverless.mov

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

value: false,
description: i18n.translate('xpack.infra.enableLogsStreamDescription', {
defaultMessage:
'Enables the legacy Logs Stream application. When enabled, the dashboard panel for legacy logs streams is also available. ',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdbirnstiehl Could you double-check this ui setting description, please?

@tonyghiani tonyghiani added release_note:deprecation 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 labels Oct 2, 2024
@tonyghiani tonyghiani marked this pull request as ready for review October 2, 2024 09:20
@tonyghiani
Copy link
Contributor Author

@flash1293 at the current stage, the Settings navigation link in the top nav will still show up in the Logs Category and Logs Anomalies pages as it was by default.
Those settings won't affect the behaviour on Logs Explorer, IMO it might be misleading for the user if they select some defaults columns there and those are not reflected on the data grid, which would be the expected behaviour for users who didn't use the Logs Stream earlier.

Alternatively we can show the settings page in the side nav directly.

This sounds like a more convenient option, as we can append this navigation entry after Logs Anomalies and Logs Categories when the Logs Stream is disabled.
Regarding the new side nav, we could also add the settings entry after those entries.

Are we good to proceed with this way? Let me know your thoughts

@flash1293
Copy link
Contributor

flash1293 commented Oct 2, 2024

Those settings won't affect the behaviour on Logs Explorer, IMO it might be misleading for the user if they select some defaults columns there and those are not reflected on the data grid, which would be the expected behaviour for users who didn't use the Logs Stream earlier.

That's a good point, you are right

This sounds like a more convenient option, as we can append this navigation entry after Logs Anomalies and Logs Categories when the Logs Stream is disabled.
Regarding the new side nav, we could also add the settings entry after those entries.

Yeah, that sounds good to me - for the new side nav we should put it under "Other tools". And probably add some kind of deprecation banner on top of it. Sorry for just coming up with this now.

Marco Antonio Ghiani added 2 commits October 2, 2024 17:56
@tonyghiani
Copy link
Contributor Author

Yeah, that sounds good to me - for the new side nav we should put it under "Other tools". And probably add some kind of deprecation banner on top of it. Sorry for just coming up with this now.

No prob @flash1293 , it was a fairly simple change, I reused the deprecation banner we implemented for the Stream page with a slightly different phrasing to redirect the user to use Logs Explorer.

Screenshot 2024-10-02 at 17 51 52 Screenshot 2024-10-02 at 17 50 59

Copy link
Member

@maryam-saeidi maryam-saeidi 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-management LGTM (code review only)

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Telemetry changes LGTM

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 7, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 2d9d15b
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-194519-2d9d15ba2821

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1504 1508 +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
@kbn/management-settings-ids 141 142 +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 1.6MB 1.6MB +57.0KB
observability 467.6KB 467.8KB +134.0B
total +57.1KB

Page load bundle

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

id before after diff
infra 52.5KB 52.7KB +249.0B
Unknown metric groups

API count

id before after diff
@kbn/management-settings-ids 143 144 +1

async chunk count

id before after diff
infra 23 31 +8

History

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

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.

LGTM!! Amazing work mate and thanks for refactoring some parts there 🚀

The only thing that was a bit confusing to me is that when the Logs Stream is disabled I saw the new Settings side nav entry with the new callout but nothing pointing me to how I can re-enable Stream if I want to.

Also as a user of Stream if I now suddenly see Page not found when I open stream links with no hints on what happened not sure about this but maybe if we have a hint on how it could be re-enabled or so.

@tonyghiani
Copy link
Contributor Author

The only thing that was a bit confusing to me is that when the Logs Stream is disabled I saw the new Settings side nav entry with the new callout but nothing pointing me to how I can re-enable Stream if I want to.

This was the approach we agreed on the comments above to still give the user a quick touch-point on the settings since the log view is still used for other features such as anomalies and categories.
I don't think we really want to give the user a direct hint to re-enable the logs stream, as we instead suggest to use Logs Explorer when they go to the settings page.

Also as a user of Stream if I now suddenly see Page not found when I open stream links with no hints on what happened not sure about this but maybe if we have a hint on how it could be re-enabled or so.

This is a good point, we could add a redirect to Logs Explorer in case of a direct hit to the /stream URL, wdyt @flash1293 ?

@tonyghiani
Copy link
Contributor Author

I added the redirect to Logs Explorer when directly accessing the Logs Stream route while disabled, that feels the most natural behaviour.

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 8, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 1778b8e
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-194519-1778b8e8a871

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1504 1508 +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
@kbn/management-settings-ids 141 142 +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 1.6MB 1.6MB +57.1KB
observability 467.6KB 467.8KB +134.0B
total +57.3KB

Page load bundle

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

id before after diff
infra 52.5KB 52.7KB +249.0B
Unknown metric groups

API count

id before after diff
@kbn/management-settings-ids 143 144 +1

async chunk count

id before after diff
infra 23 31 +8

History

@tonyghiani tonyghiani merged commit 9907601 into elastic:main Oct 9, 2024
24 checks passed
@tonyghiani tonyghiani deleted the 193320-advance-setting-to-show-logs-ui branch October 9, 2024 08:14
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 194519

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 Oct 9, 2024
elastic#194519)

## 📓 Summary

Closes elastic#193319
Closes elastic#193320

This work is part of the effort to progressively deprecate the existing
Logs Stream feature.

Changes taken on this PR consist of:
- Create a new uiSettings `observability:enableLogsStream` which
defaults to `false` on the stateful/cloud deployments and is not
available in serverless ones (still, defaults to `false` behind the
scene).
- When `observability:enableLogsStream` is `false`, the Logs Stream page
route is not registered, and neither is its deep link for global search.
- When `observability:enableLogsStream` is `false`, the panels list on
Dashboard won't show anymore the option `Logs Stream (Deprecated)` to
prevent usage of this embeddable in new dashboards. The embeddable is
still registered for retro-compatibility with active dashboards, and it
has now a callout explaining the status of this embeddable
(unmaintained/deprecated).
- Rename logs ML to "Logs Anomalies" and "Logs Categories".

Other minor improvements regard:
- Remove duplicate Xstate utils and use the relative package instead.
- Remove the duplicated `useBoolean` hook used in the deprecation
callout.
- Sync deep links registration with available routes through a single
`getLogsRoutes` util.

## 🎥 Recordings

### Logs Stream app removed

https://github.com/user-attachments/assets/f4173294-8789-4abd-9972-29c9b7c197ed

### Logs Stream dashboard panel entry removed

https://github.com/user-attachments/assets/7f99ca2a-c030-4867-b976-8fdc1df09d29

### Logs Stream app removed from project nav

https://github.com/user-attachments/assets/de51bdd6-820a-4c03-8b64-fb1a6ced0a12

### Embeddable deprecation callout

<img width="949" alt="Screenshot 2024-10-02 at 10 22 09"
src="https://github.com/user-attachments/assets/99fd5554-004b-45e4-81db-cb23947e210e">

### Unavailable setting in serverless

https://github.com/user-attachments/assets/91bf6c37-0845-4918-a485-b6250bbd96bf

---------

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

# Conflicts:
#	src/plugins/telemetry/schema/oss_plugins.json
tonyghiani added a commit that referenced this pull request Oct 9, 2024
… option (#194519) (#195542)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[LogsUI] Add UI setting to hide Logs Stream and dashboard panel
option (#194519)](#194519)

<!--- 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-10-09T08:14:06Z","message":"[LogsUI]
Add UI setting to hide Logs Stream and dashboard panel option
(#194519)\n\n## 📓 Summary\r\n\r\nCloses #193319 \r\nCloses #193320
\r\n\r\nThis work is part of the effort to progressively deprecate the
existing\r\nLogs Stream feature.\r\n\r\nChanges taken on this PR consist
of:\r\n- Create a new uiSettings `observability:enableLogsStream`
which\r\ndefaults to `false` on the stateful/cloud deployments and is
not\r\navailable in serverless ones (still, defaults to `false` behind
the\r\nscene).\r\n- When `observability:enableLogsStream` is `false`,
the Logs Stream page\r\nroute is not registered, and neither is its deep
link for global search.\r\n- When `observability:enableLogsStream` is
`false`, the panels list on\r\nDashboard won't show anymore the option
`Logs Stream (Deprecated)` to\r\nprevent usage of this embeddable in new
dashboards. The embeddable is\r\nstill registered for
retro-compatibility with active dashboards, and it\r\nhas now a callout
explaining the status of this
embeddable\r\n(unmaintained/deprecated).\r\n- Rename logs ML to \"Logs
Anomalies\" and \"Logs Categories\".\r\n\r\nOther minor improvements
regard:\r\n- Remove duplicate Xstate utils and use the relative package
instead.\r\n- Remove the duplicated `useBoolean` hook used in the
deprecation\r\ncallout.\r\n- Sync deep links registration with available
routes through a single\r\n`getLogsRoutes` util.\r\n\r\n## 🎥
Recordings\r\n\r\n### Logs Stream app
removed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/f4173294-8789-4abd-9972-29c9b7c197ed\r\n\r\n###
Logs Stream dashboard panel entry
removed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/7f99ca2a-c030-4867-b976-8fdc1df09d29\r\n\r\n###
Logs Stream app removed from project
nav\r\n\r\n\r\nhttps://github.com/user-attachments/assets/de51bdd6-820a-4c03-8b64-fb1a6ced0a12\r\n\r\n###
Embeddable deprecation callout\r\n\r\n<img width=\"949\"
alt=\"Screenshot 2024-10-02 at 10 22
09\"\r\nsrc=\"https://github.com/user-attachments/assets/99fd5554-004b-45e4-81db-cb23947e210e\">\r\n\r\n###
Unavailable setting in
serverless\r\n\r\n\r\nhttps://github.com/user-attachments/assets/91bf6c37-0845-4918-a485-b6250bbd96bf\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani <[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Mike Birnstiehl
<[email protected]>","sha":"9907601dd148ba59420bffda45ff584686f47b43","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:deprecation","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-logs","Team:obs-ux-management","apm:review"],"number":194519,"url":"https://github.com/elastic/kibana/pull/194519","mergeCommit":{"message":"[LogsUI]
Add UI setting to hide Logs Stream and dashboard panel option
(#194519)\n\n## 📓 Summary\r\n\r\nCloses #193319 \r\nCloses #193320
\r\n\r\nThis work is part of the effort to progressively deprecate the
existing\r\nLogs Stream feature.\r\n\r\nChanges taken on this PR consist
of:\r\n- Create a new uiSettings `observability:enableLogsStream`
which\r\ndefaults to `false` on the stateful/cloud deployments and is
not\r\navailable in serverless ones (still, defaults to `false` behind
the\r\nscene).\r\n- When `observability:enableLogsStream` is `false`,
the Logs Stream page\r\nroute is not registered, and neither is its deep
link for global search.\r\n- When `observability:enableLogsStream` is
`false`, the panels list on\r\nDashboard won't show anymore the option
`Logs Stream (Deprecated)` to\r\nprevent usage of this embeddable in new
dashboards. The embeddable is\r\nstill registered for
retro-compatibility with active dashboards, and it\r\nhas now a callout
explaining the status of this
embeddable\r\n(unmaintained/deprecated).\r\n- Rename logs ML to \"Logs
Anomalies\" and \"Logs Categories\".\r\n\r\nOther minor improvements
regard:\r\n- Remove duplicate Xstate utils and use the relative package
instead.\r\n- Remove the duplicated `useBoolean` hook used in the
deprecation\r\ncallout.\r\n- Sync deep links registration with available
routes through a single\r\n`getLogsRoutes` util.\r\n\r\n## 🎥
Recordings\r\n\r\n### Logs Stream app
removed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/f4173294-8789-4abd-9972-29c9b7c197ed\r\n\r\n###
Logs Stream dashboard panel entry
removed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/7f99ca2a-c030-4867-b976-8fdc1df09d29\r\n\r\n###
Logs Stream app removed from project
nav\r\n\r\n\r\nhttps://github.com/user-attachments/assets/de51bdd6-820a-4c03-8b64-fb1a6ced0a12\r\n\r\n###
Embeddable deprecation callout\r\n\r\n<img width=\"949\"
alt=\"Screenshot 2024-10-02 at 10 22
09\"\r\nsrc=\"https://github.com/user-attachments/assets/99fd5554-004b-45e4-81db-cb23947e210e\">\r\n\r\n###
Unavailable setting in
serverless\r\n\r\n\r\nhttps://github.com/user-attachments/assets/91bf6c37-0845-4918-a485-b6250bbd96bf\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani <[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Mike Birnstiehl
<[email protected]>","sha":"9907601dd148ba59420bffda45ff584686f47b43"}},"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/194519","number":194519,"mergeCommit":{"message":"[LogsUI]
Add UI setting to hide Logs Stream and dashboard panel option
(#194519)\n\n## 📓 Summary\r\n\r\nCloses #193319 \r\nCloses #193320
\r\n\r\nThis work is part of the effort to progressively deprecate the
existing\r\nLogs Stream feature.\r\n\r\nChanges taken on this PR consist
of:\r\n- Create a new uiSettings `observability:enableLogsStream`
which\r\ndefaults to `false` on the stateful/cloud deployments and is
not\r\navailable in serverless ones (still, defaults to `false` behind
the\r\nscene).\r\n- When `observability:enableLogsStream` is `false`,
the Logs Stream page\r\nroute is not registered, and neither is its deep
link for global search.\r\n- When `observability:enableLogsStream` is
`false`, the panels list on\r\nDashboard won't show anymore the option
`Logs Stream (Deprecated)` to\r\nprevent usage of this embeddable in new
dashboards. The embeddable is\r\nstill registered for
retro-compatibility with active dashboards, and it\r\nhas now a callout
explaining the status of this
embeddable\r\n(unmaintained/deprecated).\r\n- Rename logs ML to \"Logs
Anomalies\" and \"Logs Categories\".\r\n\r\nOther minor improvements
regard:\r\n- Remove duplicate Xstate utils and use the relative package
instead.\r\n- Remove the duplicated `useBoolean` hook used in the
deprecation\r\ncallout.\r\n- Sync deep links registration with available
routes through a single\r\n`getLogsRoutes` util.\r\n\r\n## 🎥
Recordings\r\n\r\n### Logs Stream app
removed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/f4173294-8789-4abd-9972-29c9b7c197ed\r\n\r\n###
Logs Stream dashboard panel entry
removed\r\n\r\n\r\nhttps://github.com/user-attachments/assets/7f99ca2a-c030-4867-b976-8fdc1df09d29\r\n\r\n###
Logs Stream app removed from project
nav\r\n\r\n\r\nhttps://github.com/user-attachments/assets/de51bdd6-820a-4c03-8b64-fb1a6ced0a12\r\n\r\n###
Embeddable deprecation callout\r\n\r\n<img width=\"949\"
alt=\"Screenshot 2024-10-02 at 10 22
09\"\r\nsrc=\"https://github.com/user-attachments/assets/99fd5554-004b-45e4-81db-cb23947e210e\">\r\n\r\n###
Unavailable setting in
serverless\r\n\r\n\r\nhttps://github.com/user-attachments/assets/91bf6c37-0845-4918-a485-b6250bbd96bf\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani <[email protected]>\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Mike Birnstiehl
<[email protected]>","sha":"9907601dd148ba59420bffda45ff584686f47b43"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review 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 release_note:deprecation Team:obs-ux-logs Observability Logs User Experience Team Team:obs-ux-management Observability Management User Experience Team v8.16.0 v9.0.0
Projects
None yet