[Logs Explorer] Change the default link for "Discover" in the serverless nav #173420

weltenwort · 2023-12-14T19:31:50Z

📓 Summary

This changes the navigation target of the "Discover" sidebar menu in the observability serverless project type to be the Logs Explorer.

cc @ruflin

apmmachine · 2023-12-14T19:32:06Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…discover-tab

weltenwort · 2023-12-18T20:36:26Z

@davismcphee I'd appreciate your help with something:

There is this test at https://github.com/weltenwort/kibana/blob/8268407ed15c64ba2f4125ca6519ccff4fc2236a/x-pack/test_serverless/functional/test_suites/common/discover/group1/_url_state.ts#L80, which explicitly tests the persistence of some Discover state in the side nav link. Since this PR changes the default target of the side nav entry, the test doesn't work anymore.

Would you say it can be dropped completely or does it cover anything about Discover's state that you'd like to preserve and move elsewhere?

weltenwort · 2023-12-19T11:23:10Z

cc @elastic/kibana-data-discovery ☝️ 😇

jughosta · 2023-12-19T11:45:42Z

cc @elastic/kibana-data-discovery ☝️ 😇

It's a test to check that filters (including pinned filters) are carried over correctly. Can it also work for Logs Explorer link? Would be great to keep the test for ES and Security projects at least.

weltenwort · 2023-12-19T11:51:12Z

Can it also work for Logs Explorer link?

No, the Logs Explorer doesn't persist its state globally that way. If we're going to implement something similar we'd probably use the LocalStorage approach as it is less entangled with the chrome service.

weltenwort · 2023-12-20T10:23:48Z

Would be great to keep the test for ES and Security projects at least.

I didn't even know there are tests that are executed for every serverless project type. Do you know how to limit them to a sub-set?

jughosta · 2023-12-20T13:40:39Z

I think we would need to move the tests from common directory to search and security.

…discover-tab

weltenwort · 2023-12-20T19:18:04Z

/ci

elasticmachine · 2023-12-20T20:29:00Z

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

jughosta

Data Discovery changes LGTM 👍

sebelga · 2023-12-21T14:14:47Z

x-pack/plugins/serverless_observability/public/components/side_navigation/index.tsx

-          link: 'discover',
+          link: 'observability-log-explorer',
+          // prevent this entry from ever becoming active, effectively falling through to the obs-log-explorer child
+          getIsActive: () => false,


Don't we want ever to mark this link as active?

The menu hierarchy here is:

Logs Explorer, which is never active and hides its breadcrumb, but needed so the Logs Explorer is the default tab

Discover, which is needed so Discover shows up in the breadcrumbs and the entry is active when in Discover

Logs Explorer, which is needed so the breadcrumbs are Discover / Logs Explorer and the entry is also active when in the Logs Explorer

It's the only way I found to get the serverless nav and breadcrumbs to behave that way. If you know of any other approach to achieve the same thing I'd gladly adopt it.

I just checked and indeed the way multiple active node paths (here you have 2 pointing to the observability-log-explorer link) are declared forces you to do this. I'll open a PR to fix it in the serverless nav so you don't need to hack it here.

That would be great. Would you prefer us to wait for your change or shall I align with it in a follow-up?

Sorry just reading now. All good in opened my PR after you merged 👍
#174184

maryam-saeidi

This changes the navigation target of the "Discover" sidebar menu in the observability serverless project type to be the Logs Explorer.

Based on the PR description, I expected to see the Logs Explorer view, but I still saw the Discover tab, and the URL path is /app/discover. Is this expected?

…discover-tab

weltenwort · 2024-01-02T11:48:59Z

@maryam-saeidi The goal is that the Discover tab is still there, but the Logs Explorer tab is the default. Are you seeing a different behavior in your tests?

sebelga

LGTM 👍 I'll open a PR for my comment about getIsActive()

sebelga · 2024-01-02T13:07:15Z

x-pack/plugins/serverless_observability/public/components/side_navigation/index.tsx

-          link: 'discover',
+          link: 'observability-log-explorer',
+          // prevent this entry from ever becoming active, effectively falling through to the obs-log-explorer child
+          getIsActive: () => false,


I just checked and indeed the way multiple active node paths (here you have 2 pointing to the observability-log-explorer link) are declared forces you to do this. I'll open a PR to fix it in the serverless nav so you don't need to hack it here.

x-pack/plugins/serverless_observability/public/components/side_navigation/index.tsx

kibana-ci · 2024-01-02T14:31:36Z

💚 Build Succeeded

Buildkite Build
Commit: 500e0c1

Metrics [docs]

Page load bundle

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

id	before	after	diff
`serverlessObservability`	63.4KB	63.5KB	+69.0B

History

💚 Build #185379 succeeded 2598f0a
💚 Build #184664 succeeded ba7c7be
💔 Build #184062 failed 8268407
💔 Build #183922 failed 932e1dc
💔 Build #183718 failed fb1459e

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

cc @weltenwort

weltenwort · 2024-01-02T16:06:02Z

run elasticsearch-ci/docs

maryam-saeidi

Worked as expected 👍🏻

* main: (4129 commits) [Logs Explorer] Change the default link for "Discover" in the serverless nav (#173420) [Fleet] fix unhandled error in agent details when components are missing (#174152) [Obs UX] Unskip transaction duration alerts test (#174069) [Fleet] Fix keep policies up to date after package install (#174093) [Profiling] Stack traces embeddable (#173905) [main] Sync bundled packages with Package Storage (#174115) [SLO Form] Refactor to use kibana data view component (#173513) [Obs UX] Unskip APM Service Inventory Journey (#173510) [Obs UX] Unskip preview_chart_error_count test (#173624) [api-docs] 2024-01-03 Daily api_docs build (#174142) Update babel runtime helpers (#171745) Handle content stream errors in report pre-deletion (#173792) [Cloud Posture] [Quick wins] Enable edit DataView on the Misconfigurations Findings Table (#173870) [ftr] abort retry on invalid webdriver session (#174092) Upgrade openai to 4.24.1 (#173934) chore(NA): bump node into v20 (#173461) [Security Solution][Endpoint] Fix index name pattern in SentinelOne dev. script (#174105) fix versions.json [Obs AI Assistant] Add guardrails (#174060) [ML] Transforms: Refactor validators and add unit tests. (#173736) ...

Change the default link for "Discover" in the serverless nav

fb1459e

weltenwort self-assigned this Dec 14, 2023

weltenwort added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team Feature:LogsExplorer Logs Explorer feature labels Dec 14, 2023

This comment was marked as outdated.

Sign in to view

weltenwort added 2 commits December 18, 2023 10:26

Merge remote-tracking branch 'upstream/main' into obs-change-default-…

aff3514

…discover-tab

Fix serverless navigation tests

932e1dc

This comment was marked as outdated.

Sign in to view

Attempt to fix the Discover breadcrumbs

fa58c8b

This comment was marked as outdated.

Sign in to view

Merge remote-tracking branch 'upstream/main' into obs-change-default-…

8268407

…discover-tab

This comment was marked as outdated.

Sign in to view

weltenwort added 2 commits December 20, 2023 18:00

Merge remote-tracking branch 'upstream/main' into obs-change-default-…

45d5b90

…discover-tab

Skip Discover side nav test on serverless-oblt

ba7c7be

weltenwort marked this pull request as ready for review December 20, 2023 20:28

weltenwort requested review from a team as code owners December 20, 2023 20:28

jughosta approved these changes Dec 21, 2023

View reviewed changes

sebelga reviewed Dec 21, 2023

View reviewed changes

maryam-saeidi reviewed Dec 27, 2023

View reviewed changes

weltenwort and others added 2 commits January 2, 2024 11:09

Merge remote-tracking branch 'upstream/main' into obs-change-default-…

eeeadc9

…discover-tab

[CI] Auto-commit changed files from 'node scripts/notice'

2598f0a

sebelga reviewed Jan 2, 2024

View reviewed changes

Remove superfluous sideNavStatus entries

500e0c1

sebelga self-requested a review January 3, 2024 08:09

sebelga approved these changes Jan 3, 2024

View reviewed changes

maryam-saeidi approved these changes Jan 3, 2024

View reviewed changes

weltenwort merged commit eb83fac into elastic:main Jan 3, 2024
20 checks passed

weltenwort deleted the obs-change-default-discover-tab branch January 3, 2024 11:52

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 3, 2024

sebelga mentioned this pull request Jan 3, 2024

[Serverless Sidenav] Order active paths by tree depth #174184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Logs Explorer] Change the default link for "Discover" in the serverless nav #173420

[Logs Explorer] Change the default link for "Discover" in the serverless nav #173420

weltenwort commented Dec 14, 2023

apmmachine commented Dec 14, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

weltenwort commented Dec 18, 2023

weltenwort commented Dec 19, 2023

jughosta commented Dec 19, 2023

weltenwort commented Dec 19, 2023

weltenwort commented Dec 20, 2023

jughosta commented Dec 20, 2023

weltenwort commented Dec 20, 2023

elasticmachine commented Dec 20, 2023

jughosta left a comment

sebelga Dec 21, 2023

weltenwort Jan 2, 2024

sebelga Jan 2, 2024

weltenwort Jan 2, 2024

sebelga Jan 3, 2024

maryam-saeidi left a comment •

edited

Loading

weltenwort commented Jan 2, 2024

sebelga left a comment

sebelga Jan 2, 2024

kibana-ci commented Jan 2, 2024

weltenwort commented Jan 2, 2024

maryam-saeidi left a comment

[Logs Explorer] Change the default link for "Discover" in the serverless nav #173420

[Logs Explorer] Change the default link for "Discover" in the serverless nav #173420

Conversation

weltenwort commented Dec 14, 2023

📓 Summary

apmmachine commented Dec 14, 2023

🤖 GitHub comments

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

weltenwort commented Dec 18, 2023

weltenwort commented Dec 19, 2023

jughosta commented Dec 19, 2023

weltenwort commented Dec 19, 2023

weltenwort commented Dec 20, 2023

jughosta commented Dec 20, 2023

weltenwort commented Dec 20, 2023

elasticmachine commented Dec 20, 2023

jughosta left a comment

Choose a reason for hiding this comment

sebelga Dec 21, 2023

Choose a reason for hiding this comment

weltenwort Jan 2, 2024

Choose a reason for hiding this comment

sebelga Jan 2, 2024

Choose a reason for hiding this comment

weltenwort Jan 2, 2024

Choose a reason for hiding this comment

sebelga Jan 3, 2024

Choose a reason for hiding this comment

maryam-saeidi left a comment • edited Loading

Choose a reason for hiding this comment

weltenwort commented Jan 2, 2024

sebelga left a comment

Choose a reason for hiding this comment

sebelga Jan 2, 2024

Choose a reason for hiding this comment

kibana-ci commented Jan 2, 2024

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

weltenwort commented Jan 2, 2024

maryam-saeidi left a comment

Choose a reason for hiding this comment

maryam-saeidi left a comment •

edited

Loading