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

Disable log stream and settings pages #203996

Merged
merged 16 commits into from
Dec 19, 2024

Conversation

gbamparop
Copy link
Contributor

@gbamparop gbamparop commented Dec 12, 2024

Release note

Logs Stream and the logs settings page in Observability are removed. Use the Discover application, which now offers a contextual experience for logs, to explore your logs. The logs stream panel in dashboards is removed, use Discover sessions instead.

📓 Summary

The Logs Stream app in Observability and the log stream panel available in dashboards were hidden behind an advanced setting as part of #194519 in 8.16.0. At the same time, a link was added to the left navigation for the logs settings page.

This PR which targets 9.0.0 disables these pages as well as the dashboard panel:

  • Removes the navigation item for the logs settings page and the corresponding route
  • Removes the observability:enableLogsStream setting and keeps the redirects to logs explorer. The locators will be updated to point to Discover as part of [Logs Explorer] Remove Logs Explorer #182229.

Removing the code that renders the logs stream and the settings pages will be done in a follow-up issue.

Left navigation

Classic

image

Solution

image

Navigating to /app/logs/settings

image

Closes https://github.com/elastic/observability-dev/issues/4156

@gbamparop
Copy link
Contributor Author

Release note

The logs stream and the logs settings pages in Observability are removed and the Discover application, which now offers a contextual experience for logs, can be used for log exploration.

@mdbirnstiehl do you have any suggestions for the release notes?

@elastic-vault-github-plugin-prod elastic-vault-github-plugin-prod bot requested a review from a team as a code owner December 12, 2024 11:55
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Dec 12, 2024
@elasticmachine
Copy link
Contributor

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

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 docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Member

@wayneseymour wayneseymour left a comment

Choose a reason for hiding this comment

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

test/tsconfig.json LGTM

@@ -415,12 +415,6 @@ function createNavTree({ streamsAvailable }: { streamsAvailable?: boolean }) {
defaultMessage: 'Logs categories',
}),
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think we should also remove the log stream entry above here in the same file right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot, removed here

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.

Overall looks good, thanks for this work 👏
Regarding the functional tests for the logs stream and settings page, should they be removed too or at least skipped since those features won't be registered behind any flag?

@gbamparop
Copy link
Contributor Author

gbamparop commented Dec 12, 2024

Overall looks good, thanks for this work 👏 Regarding the functional tests for the logs stream and settings page, should they be removed too or at least skipped since those features won't be registered behind any flag?

That's a good point, I skipped the functional tests here and added an item to the acceptance criteria of the follow-up issue to remove them.

@gbamparop
Copy link
Contributor Author

Overall looks good, thanks for this work 👏 Regarding the functional tests for the logs stream and settings page, should they be removed too or at least skipped since those features won't be registered behind any flag?

That's a good point, I skipped the functional tests here and added an item to the acceptance criteria of the follow-up issue to remove them.

The same can possibly be done for API tests.

@mdbirnstiehl
Copy link
Contributor

Release note

The logs stream and the logs settings pages in Observability are removed and the Discover application, which now offers a contextual experience for logs, can be used for log exploration.

@mdbirnstiehl do you have any suggestions for the release notes?

Maybe something like this:

Logs Stream and the logs settings page in Observability are removed. Use the Discover application, which now offers a contextual experience for logs, to explore your logs.

@gbamparop
Copy link
Contributor Author

The logs stream panel in dashboards is removed, use Discover's saved search instead.

@mdbirnstiehl thanks, updated the release notes section in the description and mentioned the log stream panel too, please feel free to update.

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

kibana-presentation changes LGTM - functional test changes only

@gbamparop
Copy link
Contributor Author

The logs stream panel in dashboards is removed, use Discover's saved search instead.

@jughosta, based on #202217 shall we update the release notes here to mention Discover session?

@jughosta
Copy link
Contributor

Hi @gbamparop,

Yes, thanks for the ping!
"Saved search" will be renamed to "Discover session" via #202217

});

it('ensure toolbar popover closes on add', async () => {
await dashboard.navigateToApp();
await dashboard.clickNewDashboard();
await dashboard.switchToEditMode();
await dashboardAddPanel.clickEditorMenuButton();
await dashboardAddPanel.clickAddNewPanelFromUIActionLink('Log stream (deprecated)');
await dashboardAddPanel.clickAddNewPanelFromUIActionLink('Pattern analysis');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to test another panel as the log stream panel is removed

@tonyghiani
Copy link
Contributor

A huge number of translations were removed in one of the latest commits, including security translations, is that intentional?

@gbamparop
Copy link
Contributor Author

A huge number of translations were removed in one of the latest commits, including security translations, is that intentional?

Not really, and the i18n_check.js script locally verifies all the translation files

@gbamparop
Copy link
Contributor Author

@elasticmachine merge upstream

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 on UI Settings LGTM

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

@rmyz rmyz left a comment

Choose a reason for hiding this comment

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

LGTM, only codeowners review

Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

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

LGTM.

Does it make sense to merge this #203620, now that we're removing the logs stream for v9.0?

Copy link
Contributor

@miloszmarcinkowski miloszmarcinkowski left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -412,12 +367,9 @@ const getLogsNavigationEntries = ({
}

// Display Stream nav entry when Logs Stream is enabled
Copy link
Contributor

@miloszmarcinkowski miloszmarcinkowski Dec 18, 2024

Choose a reason for hiding this comment

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

nit: please remove comment associated with deleted line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, removed in 8c5c37d

Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

codeowner changes lgtm

@gbamparop
Copy link
Contributor Author

Does it make sense to merge this #203620, now that we're removing the logs stream for v9.0?

@crespocarlos there's no harm in merging it, the underlying code will be removed by #204115

@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 19, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 50a3971
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-203996-50a39713b4db

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1544 1298 -246

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 136 135 -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.7MB 1.3MB -478.6KB
observability 481.1KB 480.8KB -262.0B
total -478.8KB

Page load bundle

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

id before after diff
infra 57.8KB 51.1KB -6.7KB
Unknown metric groups

API count

id before after diff
@kbn/management-settings-ids 137 136 -1

async chunk count

id before after diff
infra 33 30 -3

History

@gbamparop gbamparop merged commit 90277c3 into elastic:main Dec 19, 2024
8 checks passed
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
## Release note
Logs Stream and the logs settings page in Observability are removed. Use
the [Discover](https://www.elastic.co/guide/en/kibana/current/discover.html) application,
which now offers a contextual experience for logs, to explore your logs.
The logs stream panel in dashboards is removed, use Discover sessions
instead.

## 📓 Summary
The Logs Stream app in Observability and the log stream panel available
in dashboards were hidden behind an advanced setting as part of
elastic#194519 in `8.16.0`. At the same
time, a link was added to the left navigation for the logs settings
page.

This PR which targets `9.0.0` disables these pages as well as the
dashboard panel:
- Removes the navigation item for the logs settings page and the
corresponding route
- Removes the `observability:enableLogsStream` setting and keeps the
redirects to logs explorer. The locators will be updated to point to
Discover as part of elastic#182229.

Removing the code that renders the logs stream and the settings pages
will be done in a [follow-up
issue](elastic#204005).

### Left navigation

#### Classic
<img width="238" alt="image"
src="https://github.com/user-attachments/assets/bc72c5ce-ed32-472e-91c1-8bd691dd2420"
/>

#### Solution
<img width="275" alt="image"
src="https://github.com/user-attachments/assets/3b21a2ae-5e82-478e-97bb-e12303178a24"
/>

### Navigating to /app/logs/settings
<img width="1722" alt="image"
src="https://github.com/user-attachments/assets/07b4197c-6063-4a59-8194-a97ce2fa3cd7"
/>


Closes elastic/observability-dev#4156

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:deprecation Team:obs-ux-management Observability Management User Experience Team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.