[Security Solution] Enables telemetry for endpoint rules by madirey · Pull Request #120225 · elastic/kibana

madirey · 2021-12-02T14:49:37Z

Summary

This PR passes the eventsTelemetry object through to the siem.queryRule and siem.indicatorRule rule type executors. This enables those rules to queue telemetry data when alerts are created. The data includes the _source of each alert that is written.

Additionally, the unit tests for createQueryAlertType have been fixed and a test was added to ensure that alerts are queued for telemetry appropriately. This is adequate to test telemetry for all rule types for which it is enabled, since the telemetry is queued from a common codepath in searchAfterAndBulkCreate (just after calling bulkCreate to create the alerts).

Finally, unit tests for all other rule types have been removed. For those tests, the executors were exiting early due to thrown exceptions when missing mocks were encountered. The tests are fragile, since they rely on internals of the executors to function, and they were producing passing results even though the executors were not running to completion. The tests were therefore misleading, as the test conditions were met without actually testing those conditions.

Checklist

Delete any items that are not applicable to this PR.

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

…metry

elasticmachine · 2021-12-03T15:36:38Z

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

elasticmachine · 2021-12-03T15:36:39Z

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

madirey · 2021-12-03T15:41:06Z

@stevewritescode @pjhampton @marshallmain This re-enables telemetry for query and threat match rules. As discussed, some code on the receiving end may need to be updated to handle the new alertTypeIds outlined here. Let me know if you have any questions or concerns!

Did we want to enable telemetry for any other rule types? It looked like these were the only ones we had enabled in the past.

madirey · 2021-12-03T18:11:13Z

jenkins test this

madirey · 2021-12-03T18:22:38Z

@elasticmachine merge upstream

madirey · 2021-12-06T14:47:22Z

@elasticmachine merge upstream

marshallmain

Detection engine code changes LGTM

@stevewritescode @pjhampton what's the process for testing telemetry end-to-end?

pjhampton

If I'm understanding the scope of this PR correctly is that you have enabled detection rule alert telemetry on query + threat match rules. I think that is great! it doesn't resolve the telemetry issue described here with the usage collector: #119047. What you have done is on the roadmap fwiw.

One concern I have with this approach is that it is going to go into the data stream where endpoint alerts go in the security analytic cluster. The buffer we have only holds 100 items or 10MB a minute. Could these detection rule alerts create so many alerts that it drowns out the endpoint telemetry?

rylnd · 2021-12-07T21:50:39Z

it doesn't resolve the telemetry issue described here with the usage collector: #119047.

@pjhampton there's not detail as to what's broken in the linked issue, and I mistakenly conflated that issue with this one. Would you be able to write up a ticket describing what's broken with the existing usage collectors? Or I'd be happy to do so if you can provide details. Thanks!

edit: It looks like @FrankHassanabad is working on fixing the outdated queries used by the usage collectors 👍

rylnd · 2021-12-07T21:58:48Z

One concern I have with this approach is that it is going to go into the data stream where endpoint alerts go in the security analytic cluster. The buffer we have only holds 100 items or 10MB a minute. Could these detection rule alerts create so many alerts that it drowns out the endpoint telemetry?

This sounds like a valid concern for a probable situation; do you have a recommendation for how to proceed? We could write to a new data stream, or disable this functionality until your cluster is ready, or both?

madirey · 2021-12-13T19:36:16Z

@elasticmachine merge upstream

pjhampton · 2021-12-14T14:17:53Z

It is possible this is broken on second look, based on https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lib/detection_engine/rules/prepackaged_rules/elastic_endpoint_security.json being of type 'query'. It's possible the 8.0 telemetry documents are from old clusters and test data. Please hold fire while I investigate.

madirey · 2021-12-16T22:43:14Z

@elasticmachine merge upstream

kibana-ci · 2021-12-17T00:29:42Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged