[Security Solution][Attacks/Alerts][Attacks page] KQL section (#232592)

[e40pud]

Summary

Epic: #232569
Closes #232592

These changes add a KQL search bar to the new Attacks page.

NOTE: at the moment the seqch bar connectoed to the SourcererScopeName.detections data view and will be updated to SourcererScopeName.attacks in a separate PR #242475.

Feature Flag

Note

The feature is hidden behind the feature flag (in kibana.dev.yml):

feature_flags.overrides:
  securitySolution.attacksAlertsAlignment: true

Other changes

• SearchBarSection is now a shared component placed in the detections/components/common folder. It is used on both Alerts and Attacks pages.
• DetectionsWrapper a new shared component to wrap page loading, error handling and data view fetching logic. It is used on both Alerts and Attacks pages.
• useDetectionsDataView a new hook to encapsulate data view and runtime fields retrieval logic by the scope name. It is used on both Alerts and Attacks pages via the DetectionsWrapper component.

Testing

Steps:

1. Navigate to the "Attacks" page /app/security/attacks
2. Write kibana.alert.attack_discovery in the search bar

Expected: You will see bunch of Attack discovery fields.

[elasticmachine]

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

[elasticmachine]

Pinging @elastic/security-solution (Team: SecuritySolution)

[elasticmachine]

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

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 2de86dd

Failed CI Steps

• FTR Configs #85

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
securitySolution	8335	8337	+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	11.0MB	11.0MB	+343.0B

Page load bundle

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

id	before	after	diff
securitySolution	107.1KB	107.1KB	-1.0B

cc @e40pud

[PhilippeOberti]

I feel like this PR could have been the right now to do the dataview scope changes you did in #242475.

I left a few comments, but they are all subjective so I'm not blocking the PR, as the functionality works as expected.
I'll let you discuss with @NicholasPeretti and @agusruidiazgd and you guys can make a decision on which route to take!

[PhilippeOberti]

I have a couple of small problems with this hook:

• while it's nice to extract this logic into a hook, it will make the cleanup later on a bit more complex, in the sense that when we remove all the old sourcerer code (see [Security Solution] remove newDataViewPickerEnabled feature flag #238549) we will then have to put back some of the code in the actual components , as it won't make much sense to have a hook to to fetch the dataview from another hook.
• You don't actually need to extract this into a separate hook as you only care about the new dataview picker in the attacks page.

I'm not fully opposed to this. All my arguments are very subjective, but it feels like to me like it's a too early extraction. This is a scenario where we don't have to follow a DRY pattern, and it's fine to have a little bit of duplication of code in the components.

This PR could have been modifying half of the files. After all, the only thing you need on the attacks page is:

const { dataView, status } = useDataView(SourcererScopeName.attacks);

const isLoading: boolean = useMemo(() => status === 'loading' || status === 'pristine', [status]);

const isDataViewInvalid: boolean = useMemo(
  () =>
    status === 'error' ||
      (status === 'ready' && !status.hasMatchedIndices()),
  [dataView, status]
);

You don't need runtimeMappings, oldSourcererDataViewSpec or even the newDataViewPickerEnabled feature flag.

[PhilippeOberti]

Why not making the sourcererDataViewSpec optional here? We don't have to worry about sourcerer on the attacks page.

[e40pud]

Since it is also reused on Alerts page and it is required there. This will be different for the separate search bar component on the Attacks page.

[PhilippeOberti]

Same for this file, while I see a small advantage in making this component reusable, I don't think it makes sense as part of this PR. Why using this new component on the alerts and attacks pages, but not in any of the other pages in Security Solution (like the entity analytics details, hosts, network, users, rule details...) which are also using the same code.

I think this should be done in a separate PR and touch a bigger scope.

[PhilippeOberti]

If you want to keep this anyway, I think we should add a new prop to be able to prefix the dataTestSubj

/**
 * Optional data test subject string
 */
'data-test-subj'?: string;

{...}

({ dataView, sourcererDataViewSpec, 'data-test-subj': dataTestSubj }: SearchBarSectionProps)

{...}

<SiemSearchBar
  dataTestSubj={`${dataTestSubj}-${SEARCH_BAR_TEST_ID}`}

That way we're guaranteed to have unique ids in Security Solution.

[PhilippeOberti]

I feel like using these kind of wrappers are always more confusing to me. They're a black box and it's hard to understand what's happening in the components where they're used.

Also the name Detections_Wrapper doesn't really mean much... I had a similar issue when I refactored the alerts page and ended up creating a wrapper component but I wasn't happy about it. And now we'd have multiple wrapper components?

What do you think about creating a component that just does the <EuiSkeletonLoading>...</EuiSkeletonLoading> instead? That way it could be used as a normal component in the alerts an attacks pages.

[e40pud]

Yes, we can discuss what component makes sense to make generic for all pages in detections.

[e40pud]

Thanks for the review @PhilippeOberti!

I see points in your concerns. Even though I see some benefits of reusing those components and hooks, I agree that need more thoughts on how to make it work well in detections.

I will add KQL bar to my another PR with the attacks data view scope and will close this one in favour of it. We can sync on possible options to restructure and reuse some of the existing alerts components.

[e40pud]

Closing this PR in favour of #242475 where I added a KQL search bar. We will extract and reuse common components in a separate PR.