[Discover] Add a default "All logs" temporary data view in the Observability Solution view #205991

davismcphee · 2025-01-09T04:40:18Z

Summary

This PR adds an "All logs" ad hoc (temporary) data view to the Discover Observability root profile based on the central log sources setting, allowing quick access to logs (with the most up to date log sources) without needing to first manually create a data view:

To support this, a new getDefaultAdHocDataViews extension point has been added to Discover, allowing profiles to specify an array of ad hoc data view specs would should be created by default when the profile is resolved, and automatically cleaned up when the profile changes or the user leaves Discover.

Resolves #201669.
Resolves #189166.

Notes

The "All logs" ad hoc data view should only appear when using the Observability Solution view (in any deployment type).
Data view specs returned from getDefaultAdHocDataViews must include consistent IDs across resolutions in order for Discover to manage them correctly (e.g. to find and reload the data view after a page refresh). Situations where we'd expect a change in ID (e.g. when saving to a Discover session) are handled internally by Discover.
To avoid a breaking change, the returned ad hoc data views have no impact on the default data view shown when navigating to Discover. If any persisted data views exist, one of them will be used as the default. If no persisted data views exist, the first entry of the array returned by getDefaultAdHocDataViews will be used as the default.
We still want to notify users in Discover when they have no ES data at all, and prompt them to install integrations. For this reason, the "no data" page is still shown in Discover even if there are default profile ad hoc data views (unlike if there are persisted data views, in which case we use the default and hide the "no data" page).
When saving a Discover session that uses a default profile ad hoc data view, the data view will be copied on save as {DATA_VIEW_NAME} (copy). This allows us to assign a unique ID to the version that gets saved with the Discover session, and avoids having to choose between the profile data view or the embedded data view when reopening the session, which has drawbacks:
- If choosing the profile data view, the Discover session may display incorrectly if the log sources setting changed since it was saved, and the user would no longer be able to view the session as it was intended without first modifying the setting to the expected value.
- If choosing the embedded data view, the replacement shown after opening the Discover session may not reflect the latest log sources setting until a new session is started, and there would be no way for the user to migrate the session to use the latest version of the profile data view.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

kibanamachine · 2025-01-22T22:28:37Z

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7771

[✅] test/functional/apps/discover/context_awareness/config.ts: 25/25 tests passed.

see run history

davismcphee · 2025-01-22T22:27:51Z

.../shared/discover/public/application/main/state_management/discover_saved_search_container.ts

@@ -91,7 +94,7 @@ export interface DiscoverSavedSearchContainer {
   * @param id
   * @param dataView
   */
-  load: (id: string, dataView?: DataView) => Promise<SavedSearch>;
+  load: (id: string) => Promise<SavedSearch>;


The dataView param didn't seem to be referenced anywhere in code, so I removed it.

davismcphee · 2025-01-22T22:39:03Z

...m/plugins/shared/discover/public/application/main/state_management/utils/change_data_view.ts

+    // This can happen when default profile data views are created without fields
+    // to avoid unnecessary requests on startup.
+    if (!nextDataView.isPersisted() && !nextDataView.fields.length) {
+      await dataViews.refreshFields(nextDataView);


I'm doing this in two separate places and I don't like it, but the alternative seems to be loading all fields up front when the data views are initialized, which would be worse. There are better ways to address this, but it would require refactoring how we handle InternalState.adHocDataViews, which I wanted to avoid in this PR.

davismcphee · 2025-01-22T22:44:18Z

.../plugins/shared/discover/public/application/main/state_management/utils/resolve_data_view.ts

While working in this file, I found the logic hard to follow, and there was some dead code for things that have since changed, so I refactored it some. I feel like it's a bit easier to follow now, but I can revert all but necessary changes if there are concerns.

davismcphee · 2025-01-22T22:54:38Z

...roviders/observability/observability_root_profile/accessors/get_default_ad_hoc_data_views.ts

While working on the extension point, adding an "All logs" data view was easy and helpful for testing, so I figured I might as well just include it with the PR. But the Logs UX team can make whatever changes they'd like to it (feel free to push to the branch), since I wasn't sure exactly how the implementation should work and just made some guesses.

jughosta

Thanks for the changes!

jughosta · 2025-01-27T10:40:31Z

src/platform/plugins/shared/discover/public/application/main/discover_main_route.test.tsx

  const props: MainRouteProps = {
    customizationCallbacks: [],
    customizationContext: mockCustomizationContext,
  };

  return mountWithIntl(
    <MemoryRouter>
-      <KibanaContextProvider services={getServicesMock(hasESData, hasUserDataView)}>
+      <KibanaContextProvider services={getServicesMock(hasESData, hasUserDataView, locationState)}>


If we want the Discover URL with "All logs" to be remembered in session storage for restoring when user presses Discover in the Kibana main nav, we could extend this logic:

kibana/src/platform/plugins/shared/discover/public/application/main/hooks/use_url_tracking.ts

Lines 27 to 33 in bb877cf

const trackingEnabled =

// Disable for ad-hoc data views as it can't be restored after a page refresh

Boolean(dataView.isPersisted() || savedSearch.id) ||

// Enable for ES|QL, although it uses ad-hoc data views

isOfAggregateQueryType(savedSearch.searchSource.getField('query'));

urlTracker.setTrackingEnabled(trackingEnabled);

Good catch! I think it makes sense to support that. Updated and added tests here: 991548e.

jughosta · 2025-01-27T11:07:07Z

...roviders/observability/observability_root_profile/accessors/get_default_ad_hoc_data_views.ts

+import { i18n } from '@kbn/i18n';
+import type { ObservabilityRootProfileProvider } from '../types';
+
+const ALL_LOGS_DATA_VIEW_ID = 'discover-observability-root-profile-all-logs';


Since this id is a part of Discover URL now, can we add a test that nobody renames it? Otherwise, saved bookmarks would break.

I had some tests for the profile that would catch if this changed, but a dedicated test explaining why is a good idea. Added here: 7306fc1.

jughosta · 2025-01-27T11:11:39Z

.../shared/discover/public/application/main/state_management/discover_saved_search_container.ts

+        name: i18n.translate('discover.savedSearch.defaultProfileDataViewCopyName', {
+          defaultMessage: '{name} (copy)',
+          values: { name: dataView.name ?? dataView.getIndexPattern() },
+        }),


ThomThomson

Unified Search managed data views addition LGTM! Code review only, but everything makes sense!

elasticmachine · 2025-01-28T00:49:34Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 7306fc1

Failed CI Steps

Jest Tests #20

Test Failures

[job] [logs] Jest Tests #20 / HoverableAvatar renders the tooltip when hovering

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	1070	1074	+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/discover-utils`	229	230	+1
`@kbn/es-query`	211	215	+4
total			+5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	828.0KB	831.8KB	+3.8KB
`unifiedSearch`	374.3KB	375.3KB	+1.1KB
total			+4.9KB

Page load bundle

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

id	before	after	diff
`discover`	44.6KB	44.7KB	+134.0B
`kbnUiSharedDeps-srcJs`	3.5MB	3.5MB	+150.0B
total			+284.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/discover-utils`	279	280	+1
`@kbn/es-query`	272	276	+4
`unifiedSearch`	150	151	+1
total			+6

ESLint disabled line counts

id	before	after	diff
`discover`	17	16	-1

Total ESLint disabled count

id	before	after	diff
`discover`	19	18	-1

History

💚 Build #270248 succeeded 641c013
💔 Build #270245 failed af6171f
💚 Build #269885 succeeded c4d8168
💔 Build #269883 failed ebd3cb6
💛 Build #269864 was flaky ad21e3d
💚 Build #269800 succeeded c2328cd

cc @davismcphee

thomheymann

All looking good but I've got a question on how we can link to this data view from within Observability since this temporary data view is not guaranteed to be there (only when solution view is enabled)

The issue I'm seeing is the following:

To link to this new All Logs managed data view we can do the following:

discoverAppLocator.getLocation({
  dataViewId: 'discover-observability-root-profile-all-logs'
})

This works as long as the solution view is set but in classic mode these links would break.

Alternatively we could specify both dataViewId and dataViewSpec but that would cause the dataViewId to be ignored:

discoverAppLocator.getLocation({
  dataViewId: 'discover-observability-root-profile-all-logs', // Is ignored when `dataViewSpec` is set
  dataViewSpec: { title: 'logs-*,...' }
})

And if we only specify our own dataViewSpec in the locator then we have the issue that Discover would display both the managed "All logs" data view and our own "All logs" temporary data view that was defined in the locator making for a confusing UX.

Can we either get a guarantee that when we link to one of these managed data views using dataViewId that they will be present irregardless of whether we are in a solution view or alternatively have a way of defining in fallback in the discover locator?

markov00

changes to unified search LGTM

davismcphee · 2025-01-29T00:49:27Z

@thomheymann Thanks for the review, and I get the concern. I don't think we can guarantee that passing just the ID will work across any solution view since the profile that generates it is linked to the O11y solution view. I'm also not sure the Security or Search solutions use the central log sources setting currently.

That said, there are still a couple of ways to do what you're asking:

You could toggle between the ID and a spec based on the solution view in some shared utility to generate links, but this probably isn't super convenient.

You could pass only dataViewSpec to the locator and use discover-observability-root-profile-all-logs for the id, which will overwrite the default if one exists. In this case the only difference between solutions is that it would appear as Managed for O11y solution view and Temporary for other views in the data view picker. I'd probably recommend going with this approach and potentially just extracting this spec to a common package to share:

kibana/src/platform/plugins/shared/discover/public/context_awareness/profile_providers/observability/observability_root_profile/accessors/get_default_ad_hoc_data_views.ts

Lines 25 to 32 in 7306fc1

    
           return prevDataViews.concat({ 
        
             id: ALL_LOGS_DATA_VIEW_ID, 
        
             name: i18n.translate('discover.observabilitySolution.allLogsDataViewName', { 
        
               defaultMessage: 'All logs', 
        
             }), 
        
             title: context.allLogsIndexPattern, 
        
             timeFieldName: '@timestamp', 
        
           });

thomheymann · 2025-01-29T11:18:18Z

Thanks @davismcphee! I'm not completely convinced by these options. Feedback below:

You could toggle between the ID and a spec based on the solution view in some shared utility to generate links, but this probably isn't super convenient.

The main issue with this approach is that we're not actually checking whether Discover knows about the data view we want to link to or not but are simply using the solution view setting as a proxy for that. This could cause bugs when that logic changes inside Discover without us knowing.

You could pass only dataViewSpec to the locator and use discover-observability-root-profile-all-logs for the id, which will overwrite the default if one exists. In this case the only difference between solutions is that it would appear as Managed for O11y solution view and Temporary for other views in the data view picker. I'd probably recommend going with this approach and potentially just extracting this spec to a common package to share

We could do this as a workaround but I'm not sure if this is a great user experience. It could be confusing for users when the "All Logs" data view keeps changing from managed to temporary. This might also impact the sort order in the dropdown which we should try to avoid.

@davismcphee When solution view is set to classic then Kibana shows all 3 solutions in the navigation. Doesn't that imply that Discover should also support the data views each solution defines? What's the concern with making these available in classic mode?

davismcphee · 2025-01-29T14:38:14Z

The main issue with this approach is that we're not actually checking whether Discover knows about the data view we want to link to or not but are simply using the solution view setting as a proxy for that. This could cause bugs when that logic changes inside Discover without us knowing.

This is true, but I think it's ok for other apps to know about Discover's behaviour without having to explicitly check each time. This functionality is part of Discover's public contract now and has to be maintained for users who will rely on it, so it's similar to using any other API that defines some behaviour. The logic should not change in Discover without a consumer knowing, and the way to validate that is with proper automated testing (which we've added).

We could do this as a workaround but I'm not sure if this is a great user experience. It could be confusing for users when the "All Logs" data view keeps changing from managed to temporary. This might also impact the sort order in the dropdown which we should try to avoid.

It shouldn't keep changing from managed to temporary on users, it only changes between solution views which reflects the implementation -- it's only actually managed in O11y solution and not others where it is actually a temporary data view that's been passed through the link to Discover. The sort order of data views in the picker is alphabetical by name, so this should not have an impact.

When solution view is set to classic then Kibana shows all 3 solutions in the navigation. Doesn't that imply that Discover should also support the data views each solution defines? What's the concern with making these available in classic mode?

No, it doesn't -- at least not with how this work was defined, which was specific to the O11y solution view. The concern with me unilaterally deciding to add it to the common Discover experience is that other solution views would get a default data view called "All logs" which is linked to an O11y specific setting, which they may not want.

The Discover architecture allows us to move this to a common profile in the future and make it available across all solution views + classic without a breaking change, but it's a product decision I'm not in a position to make alone (especially not on the day of FF) and needs alignment across solutions. I do not think this should be considered a blocker for now, and should be discussed separately in the One Discover sync to align if O11y thinks this should be included for all solution views + classic.

kibanamachine · 2025-01-29T15:04:51Z

Starting backport for target branches: 8.x

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

kibanamachine · 2025-01-29T15:10:32Z

💔 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 205991

Questions ?

Please refer to the Backport tool documentation

davismcphee · 2025-01-29T16:38:46Z

💚 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

davismcphee self-assigned this Jan 9, 2025

davismcphee force-pushed the discover-ad-hoc-data-views-extension branch 2 times, most recently from 913f96f to 4fbccf0 Compare January 10, 2025 21:30

davismcphee force-pushed the discover-ad-hoc-data-views-extension branch 5 times, most recently from 666f98e to 58862da Compare January 21, 2025 23:37

davismcphee changed the title ~~[Discover] Add getDefaultAdHocDataViews extension~~ [Discover] Add a default "All logs" temporary data view in the Observability Solution view Jan 22, 2025

davismcphee force-pushed the discover-ad-hoc-data-views-extension branch from 16a7682 to 8e2e630 Compare January 22, 2025 20:56

davismcphee added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Jan 22, 2025

davismcphee commented Jan 22, 2025

View reviewed changes

davismcphee added 11 commits January 22, 2025 19:18

Initial getDefaultAdHocDataViews extension

8cd7089

Finish initial implementation

c146fac

Fix types

79f0ec0

Add useDefaultAdHocDataViews hook

13db01c

Move default profile ad hoc data views to internal state

deae60b

Fix types and tests

0e8f21c

Fix broken test

3f9bdba

Fix broken test

255fff2

Update default o11y logs data view

7770cc0

Fix Jest test

f79ef92

Remove CCS special handling

f3d14f0

Fix type

c4d8168

davismcphee requested a review from a team as a code owner January 25, 2025 01:44

Add support for showing managed data views in the data view picker

751f366

davismcphee force-pushed the discover-ad-hoc-data-views-extension branch from 9d699e8 to 751f366 Compare January 25, 2025 02:54

davismcphee added 2 commits January 24, 2025 22:58

Merge branch 'main' into discover-ad-hoc-data-views-extension

af6171f

Fix Jest tests

641c013

jughosta approved these changes Jan 27, 2025

View reviewed changes

davismcphee requested a review from a team January 27, 2025 15:28

Merge branch 'main' into discover-ad-hoc-data-views-extension

ed79553

ThomThomson approved these changes Jan 27, 2025

View reviewed changes

davismcphee added 2 commits January 27, 2025 18:37

Allow sidebar URL tracking for default profile data views

991548e

Add test for getDefaultAdHocDataViews

7306fc1

thomheymann self-requested a review January 28, 2025 08:22

jcger approved these changes Jan 28, 2025

View reviewed changes

thomheymann reviewed Jan 28, 2025

View reviewed changes

gbamparop mentioned this pull request Jan 28, 2025

"All logs" option does not always resolve to the log source setting #208557

Open

efegurkan approved these changes Jan 28, 2025

View reviewed changes

markov00 approved these changes Jan 28, 2025

View reviewed changes

thomheymann approved these changes Jan 29, 2025

View reviewed changes

davismcphee merged commit bf9d344 into elastic:main Jan 29, 2025
8 checks passed

davismcphee deleted the discover-ad-hoc-data-views-extension branch January 29, 2025 15:04

kibanamachine added the v9.0.0 label Jan 29, 2025

davismcphee mentioned this pull request Jan 29, 2025

[8.x] [Discover] Add a default "All logs" temporary data view in the Observability Solution view (#205991) #208792

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Add a default "All logs" temporary data view in the Observability Solution view #205991

[Discover] Add a default "All logs" temporary data view in the Observability Solution view #205991

davismcphee commented Jan 9, 2025 •

edited by kibanamachine

Loading

kibanamachine commented Jan 22, 2025

davismcphee Jan 22, 2025

davismcphee Jan 22, 2025

davismcphee Jan 22, 2025 •

edited

Loading

davismcphee Jan 22, 2025

jughosta left a comment

jughosta Jan 27, 2025

davismcphee Jan 27, 2025

jughosta Jan 27, 2025

davismcphee Jan 27, 2025

jughosta Jan 27, 2025

ThomThomson left a comment

elasticmachine commented Jan 28, 2025

API count

ESLint disabled line counts

Total ESLint disabled count

thomheymann left a comment

markov00 left a comment

davismcphee commented Jan 29, 2025

thomheymann commented Jan 29, 2025

davismcphee commented Jan 29, 2025

kibanamachine commented Jan 29, 2025

kibanamachine commented Jan 29, 2025

davismcphee commented Jan 29, 2025

	const trackingEnabled =
	// Disable for ad-hoc data views as it can't be restored after a page refresh
	Boolean(dataView.isPersisted() \|\| savedSearch.id) \|\|
	// Enable for ES\|QL, although it uses ad-hoc data views
	isOfAggregateQueryType(savedSearch.searchSource.getField('query'));

	urlTracker.setTrackingEnabled(trackingEnabled);

[Discover] Add a default "All logs" temporary data view in the Observability Solution view #205991

[Discover] Add a default "All logs" temporary data view in the Observability Solution view #205991

Conversation

davismcphee commented Jan 9, 2025 • edited by kibanamachine Loading

Summary

Notes

Checklist

kibanamachine commented Jan 22, 2025

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7771

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davismcphee Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jughosta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomThomson left a comment

Choose a reason for hiding this comment

elasticmachine commented Jan 28, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

thomheymann left a comment

Choose a reason for hiding this comment

markov00 left a comment

Choose a reason for hiding this comment

davismcphee commented Jan 29, 2025

thomheymann commented Jan 29, 2025

davismcphee commented Jan 29, 2025

kibanamachine commented Jan 29, 2025

kibanamachine commented Jan 29, 2025

💔 All backports failed

Manual backport

Questions ?

davismcphee commented Jan 29, 2025

💚 All backports created successfully

Questions ?

davismcphee commented Jan 9, 2025 •

edited by kibanamachine

Loading

davismcphee Jan 22, 2025 •

edited

Loading