[Security Solution] Bulk editing rule custom highlighted fields #179312

Left a couple questions and one nit but overall looks good @e40pud! I pulled down and tested all the edge cases I could think of. This is the first time I've seen the feature flag stuff used so heavily for the backend code, I think it's a pretty good way to go about it albeit adding a bit of complexity to the implementation as (correct me if I'm wrong) we aren't able to enable/disable routes themselves using the flags.

...ity_solution/server/lib/detection_engine/rule_management/api/rules/bulk_get_sources/route.ts

.../server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.test.ts

...sponse/rules_management/rule_bulk_actions/trial_license_complete_tier/perform_bulk_action.ts

e40pud · 2024-04-29T12:24:46Z

Left a couple questions and one nit but overall looks good @e40pud! I pulled down and tested all the edge cases I could think of. This is the first time I've seen the feature flag stuff used so heavily for the backend code, I think it's a pretty good way to go about it albeit adding a bit of complexity to the implementation as (correct me if I'm wrong) we aren't able to enable/disable routes themselves using the flags.

Yes, I had to add extra complexity in order to be able to block API behind feature flag. I think we can benefit from ti in future with other features that require adding public API and needs to be hidden for some time.

vitaliidm · 2024-04-29T14:06:20Z

...ity_solution/server/lib/detection_engine/rule_management/api/rules/bulk_get_sources/route.ts

+          const rules = fetchRulesOutcome.results.map(({ result }) => result);
+          const indexPatterns: string[] = [];
+          const dataViewIds: string[] = [];
+          rules.forEach((rule) => {


@e40pud
I only checked ES|QL rules since they do not have index.

Index for this rules is extracted from rule query
Without extracting index, it will display empty highlighted fields combobox.

Another case is custom ES|QL fields. They can be created in query, but I don't think it's feasible for bulk edit(since every query needs to be performed and custom fields identified).
So, we probably just should mention in docs, custom ES|QL fields are not supported.
Which would make bulk editing rules with aggregating query not working properly. As these rule have a small number of fields available.

More details on ES|QL highlighted fields can be found here: #177746

Similar behaviour also applicable to ML rules.
Index for this rule type is extracted from ML jobs, as this rule type does not have index or data view id

Thanks for pointing out to this one! We discussed performance and limitation issues and decided to go with this solution

dhurley14

Overall looks great! Appreciate the updates to the tests and additional tests too. A few small nits too.

I do believe we could simplify the code and have some performance improvements if we introduce the logic for fetching and gathering the data view titles into the bulk_get_sources route, update the hook useBulkGetRulesSources to reflect the new server-side functionality, and remove the useGetAllIndexPatternsFromSources. This would also have the added benefit of reducing the number of hooks we need to maintain and test.

...e/rule_management_ui/components/rules_table/bulk_actions/forms/investigation_fields_form.tsx

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts

...ution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts

dhurley14 · 2024-04-29T22:40:05Z

...e/rule_management_ui/components/rules_table/bulk_actions/forms/investigation_fields_form.tsx

+  const fieldOptions = indexPatterns.fields.map((field) => ({
+    label: field.name,
+  }));


Seeing as we are pulling every field (runtime included) from every index pattern for every rule in the rules table, this could be a lot of fields. Not sure what the limitations are on rendering all these fields but would be neat to see where the rendering starts to slow down.

We will show fields from the index patterns defined in Security Solution advanced settings instead

e40pud · 2024-05-02T08:50:26Z

Overall looks great! Appreciate the updates to the tests and additional tests too. A few small nits too.

I do believe we could simplify the code and have some performance improvements if we introduce the logic for fetching and gathering the data view titles into the bulk_get_sources route, update the hook useBulkGetRulesSources to reflect the new server-side functionality, and remove the useGetAllIndexPatternsFromSources. This would also have the added benefit of reducing the number of hooks we need to maintain and test.

Hey @dhurley14, we discussed potential performance issue and limitations with ES|QL and ML rules.

To avoid those issues, we decided to go with the next solution for the list of fields available in "custom highlighted fields" flyout: the dropdown will include fields from the index patterns defined in Security Solution advanced settings (more details here).

This means, I will remove the route to fetch all indices for selected rules.

…nced settings

...ution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts

Co-authored-by: Devin W. Hurley <[email protected]>

kibana-ci · 2024-05-02T12:53:22Z

💚 Build Succeeded

Buildkite Build
Commit: 9e0524f

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	5466	5467	+1

Async chunks

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

id	before	after	diff
`securitySolution`	13.7MB	13.7MB	+18.1KB

Page load bundle

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

id	before	after	diff
`securitySolution`	83.1KB	83.3KB	+268.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	518	519	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	596	597	+1

History

💔 Build #207443 failed 450296c
💛 Build #206612 was flaky 8ad3a67
💚 Build #206549 succeeded 611c445
💛 Build #206052 was flaky 6c3c65e
💔 Build #206030 failed d805d62
💛 Build #205865 was flaky 411e618

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

cc @e40pud

e40pud added 4 commits March 24, 2024 17:27

Add custom highlighted fields through the bulk edit feature (elastic#…

669d295

…164301)

Get all index patterns to build available custom highlighted fields

400aba4

Test coverage

16256a7

Fix the issue with removing all highlighted fields

1ad0a44

e40pud added release_note:enhancement Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Engine Security Solution Detection Engine Area labels Mar 24, 2024

e40pud self-assigned this Mar 24, 2024

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

231de6c

e40pud changed the title ~~[Security Solution] Add custom highlighted fields through the bulk edit feature (#164301)~~ [Security Solution][On Week] Add custom highlighted fields through the bulk edit feature (#164301) Apr 2, 2024

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

f734cfb

[CI] Auto-commit changed files from 'yarn openapi:generate'

21273b4

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

15411bd

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

414d2fb

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

01e6018

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

9baf5a4

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

733d5c5

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

2d89410

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

1bc4eb0

kibanamachine and others added 2 commits April 25, 2024 03:29

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

411e618

e40pud requested review from a team as code owners April 25, 2024 14:14

e40pud requested a review from dhurley14 April 25, 2024 14:14

e40pud added 2 commits April 25, 2024 16:38

Add basic telemetry

d805d62

Fix types

6c3c65e

dplumlee approved these changes Apr 26, 2024

View reviewed changes

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

611c445

Review feedback

8ad3a67

vitaliidm reviewed Apr 29, 2024

View reviewed changes

dhurley14 reviewed Apr 29, 2024

View reviewed changes

dkirchan approved these changes Apr 30, 2024

View reviewed changes

e40pud added 2 commits May 2, 2024 10:53

Merge branch 'main' into security/on-week/2024-03-highlighted-fields

46fc61a

Show fields from the index patterns defined in Security Solution adva…

448ad4f

…nced settings

e40pud commented May 2, 2024

View reviewed changes

...ution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts Outdated Show resolved Hide resolved

e40pud and others added 4 commits May 2, 2024 11:33

Apply suggestions from code review

22e1788

Co-authored-by: Devin W. Hurley <[email protected]>

Updated messages

ce924b4

[CI] Auto-commit changed files from 'yarn openapi:generate'

450296c

Fix types

9e0524f

dhurley14 approved these changes May 2, 2024

View reviewed changes

e40pud merged commit b4b316a into elastic:main May 2, 2024
44 checks passed

kibanamachine added the backport:skip This commit does not require backporting label May 2, 2024

banderror mentioned this pull request May 7, 2024

[MKI] Failing test: x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/rule_actions/bulk_actions/bulk_edit_rules.cy.ts - Investigation fields actions #182834

Closed

nastasha-solomon mentioned this pull request May 28, 2024

[UI copy]: Review copy for bulk updating custom highlighted fields for rules elastic/security-docs#5269

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Bulk editing rule custom highlighted fields #179312

[Security Solution] Bulk editing rule custom highlighted fields #179312

e40pud commented Mar 24, 2024 •

edited

Loading

e40pud commented Mar 24, 2024

e40pud commented Mar 27, 2024

e40pud commented Apr 2, 2024

e40pud commented Apr 2, 2024

e40pud commented Apr 4, 2024

e40pud commented Apr 5, 2024

e40pud commented Apr 8, 2024

e40pud commented Apr 10, 2024

e40pud commented Apr 11, 2024

e40pud commented Apr 12, 2024

e40pud commented Apr 15, 2024

dplumlee left a comment

e40pud commented Apr 29, 2024

vitaliidm Apr 29, 2024 •

edited

Loading

vitaliidm Apr 29, 2024

e40pud May 2, 2024

dhurley14 left a comment

dhurley14 Apr 29, 2024

e40pud May 2, 2024

e40pud commented May 2, 2024

kibana-ci commented May 2, 2024

ESLint disabled line counts

Total ESLint disabled count

[Security Solution] Bulk editing rule custom highlighted fields #179312

[Security Solution] Bulk editing rule custom highlighted fields #179312

Conversation

e40pud commented Mar 24, 2024 • edited Loading

Summary

Checklist

e40pud commented Mar 24, 2024

e40pud commented Mar 27, 2024

e40pud commented Apr 2, 2024

e40pud commented Apr 2, 2024

e40pud commented Apr 4, 2024

e40pud commented Apr 5, 2024

e40pud commented Apr 8, 2024

e40pud commented Apr 10, 2024

e40pud commented Apr 11, 2024

e40pud commented Apr 12, 2024

e40pud commented Apr 15, 2024

dplumlee left a comment

Choose a reason for hiding this comment

e40pud commented Apr 29, 2024

vitaliidm Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

vitaliidm Apr 29, 2024

Choose a reason for hiding this comment

e40pud May 2, 2024

Choose a reason for hiding this comment

dhurley14 left a comment

Choose a reason for hiding this comment

dhurley14 Apr 29, 2024

Choose a reason for hiding this comment

e40pud May 2, 2024

Choose a reason for hiding this comment

e40pud commented May 2, 2024

kibana-ci commented May 2, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

e40pud commented Mar 24, 2024 •

edited

Loading

vitaliidm Apr 29, 2024 •

edited

Loading