Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Detections] Implementation of RuleExecutionLogClient #103463

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

xcrzx
Copy link
Contributor

@xcrzx xcrzx commented Jun 28, 2021

Addresses: #106461, #106466

Summary

This PR adds rule execution log implementation build on top of rule_registry.

How to test this implementation

  1. Enable experimental FF echo "xpack.securitySolution.enableExperimental: ['ruleRegistryEnabled']" >> ./config/kibana.dev.yml
  2. Enable rule registry write echo "xpack.ruleRegistry.write.enabled: true" >> ./config/kibana.dev.yml
  3. Create a reference rule by running ./x-pack/plugins/security_solution/server/lib/detection_engine/reference_rules/scripts/create_reference_rule_query.sh

It creates a rule that generates 10 alerts every minute or so. The created rule is not visible in our UI and is not accessible through most of our API endpoints. But, we can fetch rule execution logs using the setection_engine/rules/_find_statuses endpoint.

cd ./x-pack/plugins/security_solution/server/lib/detection_engine/scripts/
./find_rules_statuses_by_ids.sh [\"rule-id-here\"]

The output could look like this:

{
  "93ef7e00-d996-11eb-8993-0f3373136c03": {
    "current_status": {
      "alert_id": "93ef7e00-d996-11eb-8993-0f3373136c03",
      "status_date": "2021-07-01T15:39:58.020Z",
      "last_failure_at": "2021-07-01T15:39:58.020Z",
      "last_failure_message": "Response Error",
      "status": "failed",
      "gap": null,
      "bulk_create_time_durations": null,
      "search_after_time_durations": [
        "1385.66"
      ]
    },
    "failures": [
      {
        "alert_id": "93ef7e00-d996-11eb-8993-0f3373136c03",
        "status_date": "2021-07-01T15:39:58.020Z",
        "last_failure_at": "2021-07-01T15:39:58.020Z",
        "last_failure_message": "Response Error",
        "status": "failed",
        "gap": null,
        "bulk_create_time_durations": null,
        "search_after_time_durations": null
      }
    ]
  }
}

These are logs written by the RuleExecutionLogClient during the rule execution.

Design

RuleExecutionLogClient

  • RuleExecutionLogClient - contains feature switch logic and "decides" which concrete client to use to read/write logs. It is supposed to be the only publicly available class, and all API users should interact with logs exclusively through it.
  • Adapters - glue code needed for backward compatibility with the existing usage patterns. They are supposed to be sole integration points for the new log clients.
  • RuleExecutionLogClient - turned-down implementation of the execution log, left here only for reference.

Integration Phase 1

Elastic-11

Integration Phase 2

Elastic-12

Integration Phase 3

Elastic-13

TODO

  • Test if the implementation could work on top of Kibana system indices
  • Add a feature flag to disable write to the rule execution log
  • Measure rule execution log performance in comparison to sidecar SOs

@xcrzx xcrzx added release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Theme: rac label obsolete Feature:RAC label obsolete labels Jun 28, 2021
@xcrzx xcrzx self-assigned this Jun 28, 2021
@xcrzx xcrzx force-pushed the rule-execution-log branch 7 times, most recently from 7c33cde to 34882c0 Compare July 1, 2021 15:45
@xcrzx xcrzx force-pushed the rule-execution-log branch 9 times, most recently from 1387e22 to 1f0f182 Compare July 15, 2021 13:47
@xcrzx xcrzx force-pushed the rule-execution-log branch 3 times, most recently from d6cd759 to 4fc4706 Compare July 21, 2021 10:42
@banderror banderror changed the title [RAC] Rule execution log implementation [Security Solution][Detections] Implementation of RuleExecutionLogClient Jul 21, 2021
@banderror banderror added Feature:Detection Rules Security Solution rules and Detection Engine and removed Feature:RAC label obsolete Theme: rac label obsolete labels Jul 21, 2021
@xcrzx xcrzx force-pushed the rule-execution-log branch from 4fc4706 to 92ab22f Compare July 30, 2021 08:57
@xcrzx xcrzx force-pushed the rule-execution-log branch 2 times, most recently from 9d097cb to cb401fa Compare August 1, 2021 20:16
@elastic elastic deleted a comment from kibanamachine Aug 1, 2021
@elastic elastic deleted a comment from kibanamachine Aug 1, 2021
@xcrzx xcrzx marked this pull request as ready for review August 1, 2021 20:18
@xcrzx xcrzx requested a review from a team as a code owner August 1, 2021 20:18
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@xcrzx xcrzx requested a review from banderror August 2, 2021 09:26
@xcrzx xcrzx force-pushed the rule-execution-log branch 3 times, most recently from 9e53e3b to 2e5ea8c Compare August 2, 2021 12:41
@xcrzx xcrzx force-pushed the rule-execution-log branch from 2e5ea8c to d319d63 Compare August 2, 2021 15:31
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lists 229 230 +1
securitySolution 2337 2338 +1
total +2

Async chunks

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

id before after diff
lists 276.1KB 276.6KB +508.0B
securitySolution 6.4MB 6.4MB -3.0B
total +505.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 208.2KB 209.2KB +1.1KB
Unknown metric groups

API count

id before after diff
ruleRegistry 60 82 +22
securitySolution 1298 1300 +2
total +24

API count missing comments

id before after diff
ruleRegistry 60 82 +22
securitySolution 1247 1249 +2
total +24

Non-exported public API item count

id before after diff
ruleRegistry 9 11 +2
securitySolution 27 28 +1
total +3

History

  • 💔 Build #142275 failed 2e5ea8c84ba24549c23327474017f083f33339ba
  • 💚 Build #142153 succeeded cb401fa185b0ae30c13af4c7ce140d7bd3048de0
  • 💛 Build #142138 was flaky 9d097cb3c7933a8c1e381eb8d8d92afe521c33bf
  • 💔 Build #141940 failed 645cad08db51a41fd366473a33eabb557c16aa19
  • 💔 Build #141931 failed e3eb4e3612ee55312fc2534b9d433eae8ff09558

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

cc @xcrzx

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to be merged!

I left some comments, but none of them are anything significant to block this PR. Still, let's quickly go through them and decide, which of them could be addressed in a follow-up PR.

🚀 🎉

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

xcrzx added a commit that referenced this pull request Aug 3, 2021
kibanamachine added a commit that referenced this pull request Aug 3, 2021
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Detection Rules Security Solution rules and Detection Engine release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants