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

Add execute streaming detectors API #850

Conversation

engechas
Copy link
Collaborator

Description

Adds new API for executing streaming detectors and the logic needed to handle these requests

Check List

  • New functionality includes testing.
    • All tests pass
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
…etector is invalid for streaming

Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
.collect(Collectors.toList());
}

// TODO - this logic is consuming ~40% of the CPU. Is there a more efficient way to filter the docs?
Copy link
Member

Choose a reason for hiding this comment

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

can we do this in the OSI pipeline itself and save this work

In the request to restExecuteStreamingDetector pass a list of docs to be ingested in _bulk and a corresponding list of filtered docs also and avoid this CPU and latency on node

@eirsep
Copy link
Member

eirsep commented Feb 14, 2024

Can we add a concept of “post-processors” - something that works on the response of the sink call
Use case in mind: I am ingesting data from s3 as source to opensearch as sink, so I make bulk api call to ingest into opensearch. If ingestion succeeds I want to propogate the same events and the success/failure of the api to another processor (after sink) and perform some streaming rule engine process or aggregations etc.
this way we can avoid forking code logic in SAP, Alerting plugins.
Instead invest in normalizing data seen by OSI processor by adding API for mapping OCSF field names to field names in rules from SAP which OSI can leverage

@eirsep
Copy link
Member

eirsep commented Feb 14, 2024

Can we create detectors with aggregation rules on a cluster with these changes?

@@ -248,7 +251,8 @@ public List<Setting<?>> getSettings() {
SecurityAnalyticsSettings.CORRELATION_TIME_WINDOW,
SecurityAnalyticsSettings.ENABLE_AUTO_CORRELATIONS,
SecurityAnalyticsSettings.DEFAULT_MAPPING_SCHEMA,
SecurityAnalyticsSettings.ENABLE_WORKFLOW_USAGE
SecurityAnalyticsSettings.ENABLE_WORKFLOW_USAGE,
SecurityAnalyticsSettings.ENABLE_STREAMING_DETECTORS
Copy link
Member

Choose a reason for hiding this comment

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

if i enable the setting > create detector > turn off setting will that detector run normally?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. Something has to nudge the Workflow entry in .opendistro-alerting-config to get the workflow marked as non-streaming.

Or the Workflow entry can be updated manually.

@@ -115,13 +113,16 @@ public class Detector implements Writeable, ToXContentObject {

private String findingsIndexPattern;

private Boolean streamingDetector;
Copy link
Member

Choose a reason for hiding this comment

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

if streaming detector is true, why are we not creating workflow with enabled=false

Copy link
Member

Choose a reason for hiding this comment

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

execute workflow will work independent of enabled=true/false anyway

@eirsep
Copy link
Member

eirsep commented Feb 14, 2024

can you run ./gradlew integTest on ensuring security analytics sanity is maintained and there is no regression in correctness because of the new changes

Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
@eirsep
Copy link
Member

eirsep commented Feb 14, 2024

How will pause detector work?

Signed-off-by: Chase Engelbrecht <engechas@amazon.com>
riysaxen-amzn pushed a commit to riysaxen-amzn/security-analytics that referenced this pull request Mar 25, 2024
…) (opensearch-project#938)

(cherry picked from commit e0b7a5a7905b977e58d80e3b9134b14893d122b0)

* remove unneeded import

---------






* Stashed user together with it's roles



---------







* Added workflow execution logic (opensearch-project#850)

* Added workflow execution logic



* Adjusted code according to comments



* Updated version of the findings json



* Updating the workflow metadata in the case of updating flag set to false while the metadata alerady exist



* Added logging for workflow metadata update



* Added Rest Execute Workflow action



* Extended workflow context with workflowMetadataId. Adjusted the doc level monitor findings



* Updated conditions for unstashing the context when indexing and deleting the workflow



---------



* Added fix when executing the workflow and when chained findings index… (opensearch-project#890)



* Fixed deleting monitor workflow metadata (#882)

* Fixed deleting monitor metadata and workflow metadata.




* fix monitor metadata error from conflict resolution



* remove unused import



* remove rest execute workflow action



* increment schema version for findings mapping json



---------

Signed-off-by: Stevan Buzejic <buzejic.stevan@gmail.com>
Signed-off-by: Angie Zhang <langelzh@amazon.com>
Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>
Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
Co-authored-by: Stevan Buzejic <buzejic.stevan@gmail.com>
Co-authored-by: Angie Zhang <langelzh@amazon.com>
Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>
Co-authored-by: Petar Dzepina <petar.dzepina@gmail.com>
Co-authored-by: Ashish Agrawal <ashisagr@amazon.com>
@engechas engechas closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants