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 UI] Mark Log Stream for deprecation #186510

Merged
merged 22 commits into from
Jun 27, 2024

Conversation

tonyghiani
Copy link
Contributor

@tonyghiani tonyghiani commented Jun 20, 2024

📓 Summary

Closes #184992

Due to Logs Explorer and Logs being 2 separate apps, it was more complicated to redirect the Logs default nav menu to Logs Explorer maintaining the same hierarchy on both the sidebar and the global search.
The implemented workaround redirects from the logs app to logs explorer once the main app route for logs UI is accessed, which should keep all the previous behaviours.

Screen.Recording.2024-06-20.at.15.15.53.mov

@tonyghiani tonyghiani added the release_note:skip Skip the PR/issue when compiling release notes label Jun 20, 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!)

@flash1293
Copy link
Contributor

I think I wasn't really precise about this, sorry - I meant the beta tag should be on the "Logs" entry in the main nav menu (as it leads to the logs explorer, which is beta), not on the "Logs" section in the Observability menu.

I think it's OK to keep it on the "Explorer" entry on there.

@tonyghiani
Copy link
Contributor Author

I think I wasn't really precise about this, sorry - I meant the beta tag should be on the "Logs" entry in the main nav menu (as it leads to the logs explorer, which is beta), not on the "Logs" section in the Observability menu.

In this case, it might not be a quick win because the sidebar entry is displayed using the app registration title, which does not support adding a beta badge or any other custom react node (the same value is used by the core application app to register the global search entry).

I can move back the Beta badge next to the Logs Explorer entry, but I don't think there is something quick we can do for a badge in the main sidenav entry, given the time constraint for 8.15.

@flash1293
Copy link
Contributor

Ah, wasn't aware we couldn't control this easily. OK, let's move it back then, I think it's OK that way

@tonyghiani
Copy link
Contributor Author

@mdbirnstiehl could you please take a look at the copies for this work? Would be great if we can merge it before the feature freeze for 8.15 of next week.

@tonyghiani tonyghiani marked this pull request as ready for review June 26, 2024 13:25
@tonyghiani tonyghiani requested review from a team as code owners June 26, 2024 13:25
@tonyghiani tonyghiani added the Team:obs-ux-logs Observability Logs User Experience Team label Jun 26, 2024
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Jun 26, 2024
@tonyghiani tonyghiani removed the request for review from a team June 26, 2024 15:31
Copy link
Contributor

@mdbirnstiehl mdbirnstiehl left a comment

Choose a reason for hiding this comment

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

I had a few suggestions. Let me know if you have any issues or questions.

tonyghiani and others added 5 commits June 27, 2024 08:20
…logs_deprecation_callout.tsx

Co-authored-by: Mike Birnstiehl <114418652+mdbirnstiehl@users.noreply.github.com>
…logs_deprecation_callout.tsx

Co-authored-by: Mike Birnstiehl <114418652+mdbirnstiehl@users.noreply.github.com>
…logs_deprecation_callout.tsx

Co-authored-by: Mike Birnstiehl <114418652+mdbirnstiehl@users.noreply.github.com>
…logs_deprecation_callout.tsx

Co-authored-by: Mike Birnstiehl <114418652+mdbirnstiehl@users.noreply.github.com>
@tonyghiani
Copy link
Contributor Author

@mdbirnstiehl thanks for the suggestions! We'll also need to update the documentation with a deprecation warning on the tail logs page, do you think we should use a different callout content for that? Could you help with the change on the docs please?

@flash1293
Copy link
Contributor

flash1293 commented Jun 27, 2024

This is how the callout looks right now:
Screenshot 2024-06-27 at 11 18 49

@mdbirnstiehl Do we need both of the paragraphs? Seems like we can shorten it to a single one. Also, it says Logs Explorer a lot, reads a bit weird to me.

Also, the bit about support could be interpreted that we don't support it anymore right now which isn't true, maybe "will be replaced by Logs Explorer in a future version" or something like that?

@mdbirnstiehl
Copy link
Contributor

@mdbirnstiehl Do we need both of the paragraphs? Seems like we can shorten it to a single one. Also, it says Logs Explorer a lot, reads a bit weird to me.

Also, the bit about support could be interpreted that we don't support it anymore right now which isn't true, maybe "will be replaced by Logs Explorer in a future version" or something like that?

@flash1293 We could reduce it like this:
The new Logs Explorer makes viewing and inspecting your logs easier with more features, better performance, and more intuitive navigation. We recommend switching to Logs Explorer, as it will replace Logs Stream in a future version.

@flash1293
Copy link
Contributor

Love it, @mdbirnstiehl !

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Callout looks good to me, why did you remove a bunch of tests @tonyghiani ? It seems like it shouldn't change anything in that area?

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jun 27, 2024

💛 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 1580 1581 +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.5MB 1.5MB +1.5KB
logsExplorer 247.2KB 247.3KB +67.0B
total +1.6KB

Page load bundle

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

id before after diff
infra 101.9KB 101.9KB +46.0B

History

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

@tonyghiani
Copy link
Contributor Author

why did you remove a bunch of tests @tonyghiani ? It seems like it shouldn't change anything in that area?

It affects those tests because they were oriented on loading the logs app landing page, which corresponds to app/logs and then redirected to app/logs/stream .

As we now have a redirect to app/observability-logs-explorer, those tests were all broken/unnecessary because we don't go through that flow anymore.

Copy link
Contributor

@flash1293 flash1293 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 merged commit 72284db into elastic:main Jun 27, 2024
20 checks passed
@tonyghiani tonyghiani deleted the 184992-log-stream-soft-deprecation branch June 27, 2024 12:33
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jun 27, 2024
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:obs-ux-logs Observability Logs User Experience Team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Log Stream] Soft-deprecate log stream app and embeddable
9 participants