[Security Solution][Detection Engine] adds preview logged requests for new terms, threshold, query, ML rule types by vitaliidm · Pull Request #203320 · elastic/kibana

vitaliidm · 2024-12-06T18:57:51Z

Summary

partially addresses [Epic][Security Solution][Detection Engine] add request logging on preview for the rest of rule types #202545 (except of IM rule type)
extends logged requests preview for:
- New terms
- Query
- ML
- Threshold
For Threshold, Query, New terms rule type introduced Page view, where each loop of rule execution is presented as a separate page
Only first 2 search queries requests of each type are logged for performance reasons(rule can have very a large and multiple requests). That's why property request was made not mandatory in rule_preview.schema.yaml

DEMO

Screen.Recording.2025-01-22.at.18.33.10.mov

…r the rest of rules

…aliidm/kibana into de_8_18/adds-logged-requests

# Conflicts: # x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_types/utils/logged_requests/log_search_request.test.ts # x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_types/utils/logged_requests/log_search_request.ts

…aliidm/kibana into de_8_18/adds-logged-requests

elasticmachine · 2025-01-23T12:14:49Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine · 2025-01-23T12:14:50Z

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

elasticmachine · 2025-01-23T12:14:52Z

Pinging @elastic/security-detection-engine (Team:Detection Engine)

marshallmain

The overall approach looks good, but we need to find a way to stringify search requests that don't use the body key.

marshallmain · 2025-01-27T17:24:59Z

..._solution/server/lib/detection_engine/rule_types/utils/logged_requests/log_search_request.ts

+
+  const url = `/${index}/_search${urlParams ? `?${urlParams}` : ''}`;
+
+  if (body) {


The body key is going away in 9.0 (#204858) so we should try to avoid relying on it.

I've seen that PR, my idea was to get this one merged first before FF.
Then help to resolve conflicts in #204858, since there are multiple places changed in both PRs

Won't we have to completely change the way we stringify the query after #204858 ? It seems like resolving the conflicts in that PR will essentially require re-implementing this PR, no?

I think it's only this function that needs to be changed.
Utilities that do ES query call have been refactored to not use body parameter.
So, I would need handle this

marshallmain · 2025-01-27T17:32:58Z

..._solution/server/lib/detection_engine/rule_types/utils/logged_requests/log_search_request.ts

+  const url = `/${index}/_search${urlParams ? `?${urlParams}` : ''}`;
+
+  if (body) {
+    return `POST ${url}\n${JSON.stringify({ ...body }, null, 2)}`;


Is there a significant difference in response size when we return stringified JSON with whitespace included vs returning a normal JSON object in the response? Including the whitespace seems like it would make the response larger but save CPU on the client since it wouldn't have to parse that large JSON query.

@marshallmain No, there is no significant difference.
I did quick tests:

for small queries like query, ML rule it's the same size

for large queries (new_terms), current implementation's response is 10% larger

Another advantages of current implementation, we don't need to pass large JSON object in response with multiple nested fields which is difficult to type.

marshallmain · 2025-01-27T17:36:27Z

...ty_solution/server/lib/detection_engine/rule_types/utils/search_after_bulk_create_factory.ts

+  isLoggedRequestsEnabled: boolean | undefined,
+  sortIds: estypes.SortResults | undefined,
+  page: number
+): LoggedRequestsEnabled | undefined => {


I think we should change the name of the type LoggedRequestsEnabled since it sounds like a boolean but it's a config object

renamed in dfeb094

elasticmachine · 2025-01-28T12:09:23Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: dfeb094

Failed CI Steps

Jest Tests #3

Test Failures

[job] [logs] Jest Tests #3 / Policy Advanced Settings section should expand and collapse section when button is clicked

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	6600	6602	+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`	21.3MB	21.3MB	+2.0KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	574	576	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	657	659	+2

History

💛 Build #270436 was flaky d1f0bdc
💛 Build #270073 was flaky ec9707d
💛 Build #269624 was flaky 154ec1f
💔 Build #269353 failed abf429c
💔 Build #268980 failed e14f30b
💔 Build #268904 failed 448f038

cc @vitaliidm

jkelas

I reviewed the changes owned by rule management team and I approve.
I also confirm the functionality works correct, the author walked me through it.

kibanamachine · 2025-01-28T16:50:15Z

Starting backport for target branches: 8.x

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

…r new terms, threshold, query, ML rule types (elastic#203320) ## Summary - partially addresses elastic#202545 (except of IM rule type) - extends logged requests preview for: - [x] New terms - [x] Query - [x] ML - [x] Threshold - For Threshold, Query, New terms rule type introduced Page view, where each loop of rule execution is presented as a separate page - Only first 2 search queries requests of each type are logged for performance reasons(rule can have very a large and multiple requests). That's why property **request** was made not mandatory in `rule_preview.schema.yaml` ### DEMO https://github.com/user-attachments/assets/abfbd3ff-d06c-4892-b805-0f05084042ed --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 0f996c3)

kibanamachine · 2025-01-28T16:55:40Z

💚 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

…sts for new terms, threshold, query, ML rule types (#203320) (#208581) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution][Detection Engine] adds preview logged requests for new terms, threshold, query, ML rule types (#203320)](#203320)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com>

[Security Solution][Detection Engine] adds preview logged requests fo…

6f38076

…r the rest of rules

vitaliidm self-assigned this Dec 6, 2024

vitaliidm added 21 commits December 9, 2024 17:03

[Security Solution][Detection Engine] refactoring and type fixing

c7eb832

[Security Solution][Detection Engine] fix FTR test

328987e

[Security Solution][Detection Engine] add unit tests

3eabaed

Merge branch 'main' into de_8_18/adds-logged-requests

6ed36ce

[Security Solution][Detection Engine] fix i18n

2f6651f

Merge branch 'de_8_18/adds-logged-requests' of https://github.com/vit…

6014a0f

…aliidm/kibana into de_8_18/adds-logged-requests

Merge branch 'main' into de_8_18/adds-logged-requests

6cfc5bd

[Security Solution][Detection Engine] add ML rule type

97c506b

Merge branch 'de_8_18/adds-logged-requests' of https://github.com/vit…

47276ee

…aliidm/kibana into de_8_18/adds-logged-requests

Merge branch 'main' into de_8_18/adds-logged-requests

a3cbc8f

[Security Solution][Detection Engine] fix types

bb32143

[Security Solution][Detection Engine] fix tests

8ae45c0

Merge branch 'main' into de_8_18/adds-logged-requests

30591cf

[Security Solution][Detection Engine] ass query

d395a29

Merge branch 'de_8_18/adds-logged-requests' of https://github.com/vit…

fb12a68

…aliidm/kibana into de_8_18/adds-logged-requests

[Security Solution][Detection Engine] add translations

677b6a5

[Security Solution][Detection Engine] fixes logger

e0a2bba

Merge branch 'main' into de_8_18/adds-logged-requests

3ac03c4

[Security Solution][Detection Engine] refactoring

084a47e

Merge branch 'de_8_18/adds-logged-requests' of https://github.com/vit…

7639413

…aliidm/kibana into de_8_18/adds-logged-requests

vitaliidm requested review from a team as code owners January 23, 2025 12:14

vitaliidm requested review from dplumlee and rylnd January 23, 2025 12:14

vitaliidm added 2 commits January 24, 2025 15:08

[Security Solution][Detection Engine]additional test assertion

ec9707d

Merge branch 'main' into de_8_18/adds-logged-requests

d1f0bdc

banderror requested review from jkelas and removed request for dplumlee January 27, 2025 16:17

marshallmain reviewed Jan 27, 2025

View reviewed changes

[Security Solution][Detection Engine] rename argument name

dfeb094

vitaliidm requested a review from marshallmain January 28, 2025 10:52

jkelas approved these changes Jan 28, 2025

View reviewed changes

marshallmain approved these changes Jan 28, 2025

View reviewed changes

vitaliidm merged commit 0f996c3 into elastic:main Jan 28, 2025

kibanamachine mentioned this pull request Jan 28, 2025

[8.x] [Security Solution][Detection Engine] adds preview logged requests for new terms, threshold, query, ML rule types (#203320) #208581

Merged

vitaliidm mentioned this pull request Jan 29, 2025

[Request][Detection Engine] Extend Detection rules preview logged queries to new terms, machine learning, query, threshold rule types elastic/security-docs#6494

Closed

vitaliidm mentioned this pull request Feb 13, 2025

[DO NOT MERGE] 9.0.0-beta release notes elastic/security-docs#6520

Closed

nastasha-solomon mentioned this pull request Apr 4, 2025

9.0 Security RN updates elastic/docs-content#1014

Merged


		const url = `/${index}/_search${urlParams ? `?${urlParams}` : ''}`;

		if (body) {

Conversation

vitaliidm commented Dec 6, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

DEMO

Uh oh!

elasticmachine commented Jan 23, 2025

Uh oh!

elasticmachine commented Jan 23, 2025

Uh oh!

elasticmachine commented Jan 23, 2025

Uh oh!

marshallmain left a comment

Choose a reason for hiding this comment

Uh oh!

marshallmain Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

vitaliidm Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

marshallmain Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

vitaliidm Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

marshallmain Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marshallmain Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

vitaliidm Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jan 28, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

jkelas left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 28, 2025

Uh oh!

kibanamachine commented Jan 28, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vitaliidm commented Dec 6, 2024 •

edited by kibanamachine

Loading

vitaliidm Jan 28, 2025 •

edited

Loading