[Security Solution][Sourcerer] Maintain url sync support by lgestc · Pull Request #221737 · elastic/kibana

lgestc · 2025-05-28T08:19:47Z

Summary

Sourcerer supports url sync for its state, and we should have the same thing working for the new data view picker.
This PR maintains that and makes sure that we are not calling the update logic twice when the feature is off.
It also prevent race conditions - only the last selection will be applied to the picker in case of multiple dispatches within short time frames (eg. during app init).

One known issue: you might see a flash during app init related to data views switching after being restored from the url. This is tracked in a separate ticket.

Testing

With the feature flag on:

xpack.securitySolution.enableExperimental: ['newDataViewPickerEnabled']

Data View Manager state should be persisted and restored to/from the url the same way it works with Sourcerer.

Eg: navigate to explore pages, change the data view to 'metrics' for example. Refreshing the page should render the metrics data view selected again.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…ent picker

elasticmachine · 2025-05-28T08:20:32Z

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

x-pack/solutions/security/plugins/security_solution/public/app/home/index.tsx

michaelolo24 · 2025-05-29T03:13:41Z

x-pack/solutions/security/plugins/security_solution/public/common/hooks/constants.ts

+export const URL_PARAM_KEY = {
+  appQuery: 'query',
+  /** @deprecated */
+  eventFlyout: 'eventFlyout', // TODO remove when we assume it's been long enough that all users should use the newer `flyout` key


@PhilippeOberti,do we have telemetry?

hmmm I know we added telemetry when users open the flyout, but I don't think it logs what's in the url. Can't we just retrieve url information from Kibana telemetry? We should be able to query and see if any of the url we log contain the eventFlyout param?

Not related to this PR I think but I can remove it if you want.

Yea, unrelated, but didn't want this to stay in forever. No need to remove here, just wanted to call attention to it so we could remove it later

...ns/security/plugins/security_solution/public/data_view_manager/hooks/use_select_data_view.ts

michaelolo24 · 2025-05-29T11:39:10Z

...ions/security/plugins/security_solution/public/data_view_manager/hooks/use_sync_url_state.ts

+
+      // Select data view for specific scope, based on the UrlParam.
+      (Object.keys(initialState) as SourcererScopeName[]).forEach((scope) => {
+        // NOTE: looks like this is about skipping the init when the active page is detections


Not just detections, but the explore pages as well. If it helps, I believe this was to account for timeline as the sourcerer state for that would've been handled in the timeline slice of useUrlState.

This is how the code was: https://github.com/elastic/kibana/blob/8.2/x-pack/plugins/security_solution/public/common/components/url_state/helpers.ts

And this is an old example of how it was structured: #123617

...utions/security/plugins/security_solution/public/sourcerer/containers/use_init_sourcerer.tsx

michaelolo24 · 2025-05-29T12:26:43Z

The PR looks good for the most part, only a couple things:

It looks like the persisted dataview selection for timeline is being overridden. So if you save a timeline with a different dataview selected (i.e. metrics-*), when you go to a new tab with that same url, the timeline opens with the default dataview selected. That might not have to do with the changes you've made here though.
Less crucial, but this error is showing up in the console, but most likely due to the logic for sourcerer still running

lgestc · 2025-05-30T07:23:43Z

The PR looks good for the most part, only a couple things:

It looks like the persisted dataview selection for timeline is being overridden. So if you save a timeline with a different dataview selected (i.e. metrics-*), when you go to a new tab with that same url, the timeline opens with the default dataview selected. That might not have to do with the changes you've made here though.

Less crucial, but this error is showing up in the console, but most likely due to the logic for sourcerer still running

looks like I have fixed the first thing, as for the 2nd one, this is just a warning and we cannot do anything about it yet. will go away after we only do a single init vs two :)

lgestc · 2025-06-02T08:11:25Z

The PR looks good for the most part, only a couple things:

It looks like the persisted dataview selection for timeline is being overridden. So if you save a timeline with a different dataview selected (i.e. metrics-*), when you go to a new tab with that same url, the timeline opens with the default dataview selected. That might not have to do with the changes you've made here though.

Less crucial, but this error is showing up in the console, but most likely due to the logic for sourcerer still running

@michaelolo24 data view manager uses url sourced state first, with default selections applied as fallback. race conditions should not be possible after the changes.

michaelolo24 · 2025-06-03T01:11:10Z

...ity/plugins/security_solution/public/data_view_manager/components/data_view_picker/index.tsx

+    (id: string, indexPattern: string = '') => {
+      selectDataView({ id, scope });
+
+      if (isDefaultSourcerer) {


Not for this PR, but defaultSourcerer is vague, but I'm not sure what to rename it to or how to classify the other scopes of timeline,analyzer, and the detections page 🤔 . Anyways, just wanted to bring it up for discussion separately

michaelolo24 · 2025-06-03T01:17:06Z

...security/plugins/security_solution/public/data_view_manager/hooks/use_sync_url_state.test.ts

+    expect(initDataViewSelection).not.toHaveBeenCalled();
+  });
+
+  it('should call initDataViewSelection for each scope if newDataViewPickerEnabled is true', () => {


nit, but I would have this map over all of the available scopes, rather than just 2

michaelolo24 · 2025-06-03T01:36:24Z

...ions/security/plugins/security_solution/public/data_view_manager/hooks/use_sync_url_state.ts

+      // Select data view for specific scope, based on the UrlParam.
+      const urlBasedSelection = (Object.keys(initialState) as SourcererScopeName[])
+        .map((scope) => {
+          // NOTE: looks like this is about skipping the init when the active page is detections


michaelolo24 · 2025-06-03T01:37:54Z

...ity/plugins/security_solution/public/data_view_manager/redux/listeners/data_view_selected.ts

+      }
+
+      // Cancel effects running for the current scope to prevent race conditions
+      listenerApi.cancelActiveListeners();


👍🏾 Thanks

...ity/plugins/security_solution/public/data_view_manager/redux/listeners/data_view_selected.ts

michaelolo24 · 2025-06-03T01:42:07Z

x-pack/solutions/security/plugins/security_solution/public/data_view_manager/redux/slices.ts

      }
    },
-    init: (state) => {
+    init: (state, _action: PayloadAction<SelectDataViewAsyncPayload[]>) => {


Is the _action reference here necessary?

I think this is where the type information is sourced from, but made it shorter.

michaelolo24

The changes look good and the race conditions seem to be resolved, but will do additional testing over the next few weeks off of main. Had a few more comments here, but all in all the PR looks good. Nice work! 👍🏾

elasticmachine · 2025-06-03T10:26:14Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 21f6db8

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	7453	7455	+2

Async chunks

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

id	before	after	diff
`securitySolution`	9.2MB	9.2MB	+1.3KB

Page load bundle

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

id	before	after	diff
`securitySolution`	88.5KB	88.5KB	+2.0B

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`securitySolution`	337	339	+2

History

💔 Build #304426 failed 449fef7

) ## Summary Sourcerer supports url sync for its state, and we should have the same thing working for the new data view picker. This PR maintains that and makes sure that we are not calling the update logic twice when the feature is off. One known issue: you might see a flash during app init related to data views switching after being restored from the url. This is tracked in a separate ticket. ## Testing With the feature flag on: ``` xpack.securitySolution.enableExperimental: ['newDataViewPickerEnabled'] ``` Data View Manager state should be persisted and restored to/from the url the same way it works with Sourcerer. Eg: navigate to explore pages, change the data view to 'metrics' for example. Refreshing the page should render the metrics data view selected again. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

… Data View Picker (#210585) (#223044) # Backport This will backport the following commits from `main` to `8.19`: - [[Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker (#210585)](#210585) - [[Security Solution] Rename use_data_view to use_data_view_spec #216461](#216461) - [[Security Solution] Rename use full data view hook #216614](#216614) - [[Security Solution] Replace sourcerer in global header #216685](#216685) - [[Security Solution] Remove .title use in use_selected_patterns #216994](#216994) - [[Security Solution] Render default security solution data view with managed label #216961](#216961) - [[Security Solution] Replace sourcerer in analyzer #218183](#218183) - [[Security Solution] Replace use_sourcerer_data_view #216997](#216997) - [[Security Solution] Replace sourcerer in EQL tab with dataview picker #218897](#218897) - [[Security Solution][Sourcerer] replace use get scoped data view #220196](#220196) - [[Security Solution] renaming dataView to dataViewSpec and adding types for clarity #220718](#220718) - [[Security Solution][Sourcerer] Maintain url sync support #221737](#221737) - [[Security Solution][Data View Manager] Allow passing data view to query bar #220585](#220585) - [[Security Solution] Fix data view picker privilege #222122](#222122)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: Luke Gmys <11671118+lgestc@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Philippe Oberti <philippe.oberti@elastic.co>

) ## Summary Sourcerer supports url sync for its state, and we should have the same thing working for the new data view picker. This PR maintains that and makes sure that we are not calling the update logic twice when the feature is off. One known issue: you might see a flash during app init related to data views switching after being restored from the url. This is tracked in a separate ticket. ## Testing With the feature flag on: ``` xpack.securitySolution.enableExperimental: ['newDataViewPickerEnabled'] ``` Data View Manager state should be persisted and restored to/from the url the same way it works with Sourcerer. Eg: navigate to explore pages, change the data view to 'metrics' for example. Refreshing the page should render the metrics data view selected again. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

[Security Solution][Sourcerer] Maintain url support identical to curr…

dd3fa13

…ent picker

lgestc requested review from a team as code owners May 28, 2025 08:19

lgestc added 9.1 candidate v9.1.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team Feature:Sourcerer labels May 28, 2025

lgestc added 2 commits May 28, 2025 13:03

add tests

60f8cbe

add test

48390a2

michaelolo24 reviewed May 29, 2025

View reviewed changes

x-pack/solutions/security/plugins/security_solution/public/app/home/index.tsx Outdated Show resolved Hide resolved

michaelolo24 reviewed May 29, 2025

View reviewed changes

...ns/security/plugins/security_solution/public/data_view_manager/hooks/use_select_data_view.ts Show resolved Hide resolved

michaelolo24 reviewed May 29, 2025

View reviewed changes

...utions/security/plugins/security_solution/public/sourcerer/containers/use_init_sourcerer.tsx Show resolved Hide resolved

lgestc and others added 2 commits May 29, 2025 15:36

Merge branch 'main' into support_url_sync

ace375e

fix url sync for timelines

7bee5f8

lgestc requested a review from michaelolo24 May 30, 2025 07:23

lgestc mentioned this pull request May 30, 2025

[Security Solution][Sourcerer] Default data view init flow for the DV manager #221338

Merged

1 task

lgestc and others added 5 commits May 30, 2025 13:10

fix broken test

2acf7c3

modify init flow for the data view plugin to use url state

b79b8cf

url management improvements

0b6839b

fix tests

a23e19a

Merge branch 'main' into support_url_sync

63c47a4

fixes

274e239

lgestc added 6 commits June 2, 2025 13:58

cancellable tasks for loading data views

13e37ff

update tests

2a26aa5

add one more test

2f67cd5

update tests

15e176f

types

1e70b8e

fix tests

23621a4

elastic deleted a comment from elasticmachine Jun 2, 2025

michaelolo24 reviewed Jun 3, 2025

View reviewed changes

...ity/plugins/security_solution/public/data_view_manager/redux/listeners/data_view_selected.ts Outdated Show resolved Hide resolved

michaelolo24 reviewed Jun 3, 2025

View reviewed changes

michaelolo24 approved these changes Jun 3, 2025

View reviewed changes

lgestc added 2 commits June 3, 2025 08:38

fix duplicated condition

89fe57c

rename ref

449fef7

lgestc enabled auto-merge (squash) June 3, 2025 06:41

Merge branch 'main' into support_url_sync

21f6db8

lgestc merged commit f8ad8a3 into elastic:main Jun 3, 2025
10 checks passed

christineweng mentioned this pull request Jun 6, 2025

[8.19] [Security Solution][Sourcerer] Replace Sourcerer with Discover Data View Picker (#210585) #223044

Merged

Conversation

lgestc commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Checklist

Uh oh!

elasticmachine commented May 28, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michaelolo24 commented May 29, 2025

Uh oh!

lgestc commented May 30, 2025

Uh oh!

lgestc commented Jun 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelolo24 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Jun 3, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

Page load bundle

References to deprecated APIs

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lgestc commented May 28, 2025 •

edited

Loading