add joins, insertion & search to correlation engine #7771

sbcd90 · 2023-05-26T02:14:37Z

Description

This PR focusses on the Join handler, Insertion handler & the Search handler components of the Events Correlation Engine. The component level architecture diagram can be found here

Join Handler

The Join handler component determines immediate neighbors of a particular event using the Correlation Rules defined by the user for the log indices(or datastreams) they wish to correlate.

Here is a brief schematic class diagram of the Join Handler layer.

Insertion Handler

In the Insertion handler layer events are converted to k-dimensional vectors & are stored in the vector storage layer mentioned above along with their correlations.

Here is a brief schematic class diagram of the Insertion Handler layer.

Search Handler

The Search Handler layer allows user to specify a particular event, & then converts it to a k-dimensional vector & then uses it to query its neighboring eventswhich are actually its correlated events within a time window.

Here is a brief schematic class diagram of the Search Handler layer.

REST apis

The following are the REST apis introduced by this PR.

Correlate an event

POST /_correlation/events

Request:
{
  "index": "app_logs",
  "event": "l2UZD4kBSz-dGVL1EZFJ",
  "store": false
}

Response:
{
  "is_orphan": false,
  "neighbor_events": {
    "windows": [
      "DmyTDI8B1GiWmmBKbkSu"
    ]
  }
}

Search Correlated Events for the input event

GET /_correlation/events?index=windows&event=FZ-9DI8BfF4uBOcpmLhq&timestamp_field=winlog.timestamp&time_window=300000&nearby_events=5

Response:
{
  "events": [
    {
      "index": "app_logs",
      "event": "Fp-9DI8BfF4uBOcpmLj1",
      "score": 0.5,
      "tags": []
    }
  ]
}

Related Issues

#6854

Check List

New functionality includes testing.
- All tests pass
[] New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

github-actions · 2023-05-26T02:19:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/16345/
CommitID: 6bc37f0
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

opensearch-trigger-bot · 2023-06-25T15:22:37Z

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

github-actions · 2023-06-26T20:24:19Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled

URL: https://build.ci.opensearch.org/job/gradle-check/18381/
CommitID: 20e81ad
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

codecov · 2023-06-26T20:27:50Z

Codecov Report

Attention: Patch coverage is 68.37325% with 383 lines in your changes missing coverage. Please review.

Project coverage is 71.83%. Comparing base (b15cb0c) to head (46dafcb).
Report is 441 commits behind head on main.

Files	Patch %	Lines
...nts/transport/TransportStoreCorrelationAction.java	66.33%	104 Missing and 32 partials ⚠️
...nts/transport/TransportIndexCorrelationAction.java	58.29%	86 Missing and 17 partials ⚠️
...ansport/TransportSearchCorrelatedEventsAction.java	64.45%	39 Missing and 20 partials ⚠️
...h/plugin/correlation/utils/CorrelationIndices.java	11.11%	40 Missing ⚠️
...h/plugin/correlation/events/model/Correlation.java	86.82%	3 Missing and 14 partials ⚠️
...ch/plugin/correlation/EventsCorrelationPlugin.java	20.00%	4 Missing ⚠️
...lation/events/action/IndexCorrelationResponse.java	80.00%	4 Missing ⚠️
...lugin/correlation/events/model/EventWithScore.java	91.66%	1 Missing and 3 partials ⚠️
...events/resthandler/RestIndexCorrelationAction.java	66.66%	4 Missing ⚠️
.../resthandler/RestSearchCorrelatedEventsAction.java	78.94%	4 Missing ⚠️
... and 5 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7771      +/-   ##
============================================
+ Coverage     71.42%   71.83%   +0.41%     
- Complexity    59978    62306    +2328     
============================================
  Files          4985     5135     +150     
  Lines        282275   293039   +10764     
  Branches      40946    42306    +1360     
============================================
+ Hits         201603   210508    +8905     
- Misses        63999    65154    +1155     
- Partials      16673    17377     +704

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

github-actions · 2024-04-23T20:14:48Z

❌ Gradle check result for 4b4a92f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

github-actions · 2024-04-23T21:54:15Z

✅ Gradle check result for 5e649c0: SUCCESS

…ject#9956) Fix is pending in opensearch-project#7771, but that PR may take some time to land in main so muting for the time being. Signed-off-by: Andrew Ross <andrross@amazon.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>

shwetathareja · 2024-06-03T16:35:58Z

...in/java/org/opensearch/plugin/correlation/events/resthandler/RestIndexCorrelationAction.java

+/**
+ * Rest action for indexing an event and its correlations
+ */
+public class RestIndexCorrelationAction extends BaseRestHandler {


do these actions support timeouts?

added search timeouts to these actions.

shwetathareja · 2024-06-03T16:39:10Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+            client.search(searchRequest, new ActionListener<>() {
+                @Override
+                public void onResponse(SearchResponse response) {
+                    if (response.isTimedOut()) {


it can be 429s as well

added check for 429

shwetathareja · 2024-06-03T16:45:21Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+                        int idx = 0;
+                        for (MultiSearchResponse.Item response : responses) {
+                            if (response.isFailure()) {
+                                log.error("error:", response.getFailure());


add more details to log message.

fixed this.

shwetathareja · 2024-06-03T16:49:26Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+ *
+ * @opensearch.internal
+ */
+public class TransportIndexCorrelationAction extends HandledTransportAction<IndexCorrelationRequest, IndexCorrelationResponse> {


we should move the core logic to EventsCorrelationManager or something of that sort.

Applicable for all transport classes

can address this in a follow-up pr.

shwetathareja · 2024-06-03T16:54:56Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+                            if (response.getStatus().equals(RestStatus.OK)) {
+                                onOperation(true, new HashMap<>());
+                            } else {
+                                onFailures(new OpenSearchStatusException("Failed to store correlations", RestStatus.INTERNAL_SERVER_ERROR));


do you need specific error handling here when it failed to index vectors? lets say why would it fail with 4xx (besides 429), Also for intermittent error in case it did index but the response failed, is that an issue?

currently, RestStatus.OK -> success, any other status -> failures. This is returned by a custom transport action.

i can enhance error handling in a follow-up pr.

shwetathareja · 2024-06-03T16:55:44Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+                List<CorrelationQuery> correlationQueries = indexQueriesEntry.getValue();
+
+                // assuming all queries belonging to an index use the same timestamp field.
+                String timestampField = correlationQueries.get(0).getTimestampField();


do you want to validate here the assumption for timestamp field

added validation.

shwetathareja · 2024-06-03T16:57:57Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+                                continue;
+                            }
+
+                            Iterator<SearchHit> searchHits = response.getResponse().getHits().iterator();


this may not be all results right as you are not setting size param

yes. pagination is missing. can address this in follow-up prs.

shwetathareja · 2024-06-03T17:02:10Z

...rg/opensearch/plugin/correlation/events/transport/TransportSearchCorrelatedEventsAction.java

+            searchRequest.indices(index);
+            searchRequest.source(searchSourceBuilder);
+
+            client.search(searchRequest, new ActionListener<>() {


multiple search requests are chained as PiT is not being used, can these results become inconsistent as they will be executed against different segments?

shwetathareja · 2024-06-06T07:49:57Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+                        onFailures(new OpenSearchStatusException(response.toString(), RestStatus.REQUEST_TIMEOUT));
+                    }
+
+                    Iterator<SearchHit> hits = response.getHits().iterator();


is there any limits on the no. of hits?

10000 is default limit.

shwetathareja · 2024-06-06T07:51:24Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+            });
+        }
+
+        private void prepRulesForCorrelatedEventsGeneration(String index, String event, List<CorrelationRule> correlationRules) {


there are very long method, can break into multiple small private methods

can address this refactoring in a follow-up pr.

shwetathareja · 2024-06-06T08:06:23Z

...java/org/opensearch/plugin/correlation/events/transport/TransportIndexCorrelationAction.java

+    @Override
+    protected void doExecute(Task task, IndexCorrelationRequest request, ActionListener<IndexCorrelationResponse> listener) {
+        AsyncIndexCorrelationAction asyncAction = new AsyncIndexCorrelationAction(request, listener);
+        asyncAction.start();


These are heavy weight transport actions chaining multiple queries as well. In which threadpool they would run. They shouldn't run on transport threadpool.

today, all of them run in transport threadpool.

shwetathareja · 2024-06-06T08:26:46Z

...rg/opensearch/plugin/correlation/events/transport/TransportSearchCorrelatedEventsAction.java

+ *
+ * @opensearch.internal
+ */
+public class TransportSearchCorrelatedEventsAction extends HandledTransportAction<


Are there any stats planned for CorrelatedEvents Actions?

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

github-actions · 2024-06-18T00:46:52Z

❕ Gradle check result for 46dafcb: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

andrross

I do think a correlation engine is useful and believe that the OpenSearch Project should deliver this feature. However, I remain unconvinced that the OpenSearch repository maintainers are the right people to take ownership of this code base. I personally do not have the bandwidth to build up the domain expertise required to properly review this code, and the fact that this PR has languished for over a year suggests that the case is similar for other maintainers. Since this feature is being architected as a plugin, I don't think there is any technical reason this code needs to live inside the OpenSearch repository. I also believe there are significant advantages to being able to iterate and evolve at its own pace, without being coupled to all the bottlenecks and pain points (flaky tests) that exist in the OpenSearch repository. Also note that a convention has been recently codified that plugins should be developed in their own repository as opposed to inside the OpenSearch repository. If another maintainer wants to make the case that this is the right place for this feature, then I will happily withdraw my objection.

/cc @reta @dblock @shwetathareja

dblock · 2024-06-25T20:36:04Z

I agree with @andrross, should we close this?

praveensameneni · 2024-06-25T21:49:42Z

@dblock, @andrross , @shwetathareja , @reta
I can understand the reservations on bringing in the plugin into core opensearch repository. As per the conventions - " .. If you think it should be there anyway, please explain why in the RFC stage." We discussed this topic last year when we first started the RFC and had a core maintainer opine and another at here.

I understand maintainers are not permanent and there will be new maintainers and ideas, however as an open source project, what is the guiding principle for these kind of changes. The convention says discuss it during RFC stage and we duly followed the process.

This would be a great precedent for a non-maintainer to contribute and discuss the feature and benefits and qualify to become a core maintainer or we will always be restricting contributions and not grow the project.

reta · 2024-06-26T11:28:08Z

I understand maintainers are not permanent and there will be new maintainers and ideas, however as an open source project, what is the guiding principle for these kind of changes. The convention says discuss it during RFC stage and we duly followed the process.

@praveensameneni the OpenSearch ecosystem evolves fast and along the way the things that do work or don't become more apparent (plus new challenges always come in). The recently published guidelines (which we did not have a year ago) are trying to touch upon where new plugins belong. I agree with @andrross point and @dblock that it should be out of core at this moment of time.

This would be a great precedent for a non-maintainer to contribute and discuss the feature and benefits and qualify to become a core maintainer or we will always be restricting contributions and not grow the project.

The contributor could become the maintainer on the plugin repo (and continue contribution to core to become a maintainer there as well), we have quite a few successful stories like that already.

praveensameneni · 2024-06-26T19:24:00Z

@reta
It's currently a plugin inside of core repository, creating an additional plugin repository will also introduce additional dependency and operational overheads.
Example: Security Analytics has several use cases around correlations, and that is where we started, iterated on and evolved the functionality. Similarly Observability and any log analytics (e.g. application logs) would also have a use case of correlations - show me correlated events when there is an increase in 5xx or response times. And if we follow the separate plugin path, each of the plugins would have to take a dependency on the new plugin (e.g. correlations).

While the guidelines were recently added, the discussion from last year when we created the RFC still holds true.
Also, the recently added guideline says " ..If you think it should be there anyway, please explain why in the RFC stage."
It appears the above sentence does not truly mean what it says unless am missing something.

I understand the guidelines around new plugins, which unfortunately is biased towards maintainers domain expertise and does not fully account for integrating dependencies in plugin ecosystem. Additionally, It would also be better to take into account the use case and work backwards from the problem it's trying to solve and the value add for the community.

andrross · 2024-06-26T19:42:40Z

And if we follow the separate plugin path, each of the plugins would have to take a dependency on the new plugin (e.g. correlations).

@praveensameneni I just want to reiterate that the need to take a dependency on a new plugin is true regardless of where this code lives. The OpenSearch Project publishes many plugins, some of which reside in this repo, some of which reside in other repos, and consumers need not know or care.

the OpenSearch ecosystem evolves fast and along the way the things that do work or don't become more apparent

What has become apparent to me in this case is that the decision for the OpenSearch maintainers to take ownership of this code has not worked, as shown by this PR being open for over a year. The current state of the correlation engine is that some basic functionality has been committed in this repo on the main branch (I don't think enough is there to be practically useful) and none of it has been backported to 2.x so it has never been released.

praveensameneni · 2024-06-26T20:21:04Z

@praveensameneni I just want to reiterate that the need to take a dependency on a new plugin is true regardless of where this code lives. The OpenSearch Project publishes many plugins, some of which reside in this repo, some of which reside in other repos, and consumers need not know or care.

If it's a separate plugin repo outside of core repo, you take a dependency through common-utils, the plugin repo jar among others, however when shipped as a plugin (e.g. ActionPlugin, ScriptPlugin, ReloadablePlugin, ExtensiblePlugin) inside of core repo, we do not have to take an additional jar, common-utils dependency.

Just to clarify, we will continue to support and maintain the code.
We are adding fundamentals of correlations and the consumers (e.g. Observability, Security Analytics) will leverage these.

reta · 2024-06-26T22:39:23Z

If it's a separate plugin repo outside of core repo, you take a dependency through common-utils, the plugin repo jar among others, however when shipped as a plugin (e.g. ActionPlugin, ScriptPlugin, ReloadablePlugin, ExtensiblePlugin) inside of core repo, we do not have to take an additional jar, common-utils dependency.

@praveensameneni the plugins are distributed as ZIP archives (bundles), not a JAR files.The OpenSearch distribution comes with some plugins preinstalled / prebundled. What is the plugin repo jar you are referring to?

praveensameneni · 2024-06-27T07:19:57Z

Thank you @reta , @andrross for your insightful comments.
We will create a new plugin repository to support correlations and iterate on the development and release. However, as and when there is more interest in the correlations, we would like to integrate into opensearch-core (not as a plugin) to provide an integrated experience with search and query capabilities leveraging some of the core Lucene constructs.

praveensameneni · 2024-06-27T07:20:46Z

We will take the PR and previously merged PR's into the new opensearch-correlations plugin repository.

andrross · 2024-06-27T22:16:31Z

If it's a separate plugin repo outside of core repo, you take a dependency through common-utils, the plugin repo jar among others, however when shipped as a plugin (e.g. ActionPlugin, ScriptPlugin, ReloadablePlugin, ExtensiblePlugin) inside of core repo, we do not have to take an additional jar, common-utils dependency.

For posterity, I want to clarify a few points here. There are a couple different types of dependencies between plugins as far as I know.

A plugin adds a capability in the core, and another plugin depends on that capability. Example: the k-NN plugin adds vector storage and search capabilities to the existing indexing and search APIs. Plugins like neural-search then build additional features on top of those vector primitives. In this particular case, neural-search also takes a compile dependency on the k-NN jar to get access to some Java classes defined by k-NN.
A plugin itself defines an extensible capability that other plugins can extend using the Java SPI mechanism. job-scheduler is an example of this.

I'm honestly not sure what category the correlation engine will fall into (thus far I haven't seen it defining an SPI interface so I would guess category 1), but the overall point stands that to a consumer plugin where the dependent plugin code lives should not be important. In both the k-NN and job-scheduler cases they are plugins defined in external repos. I'm not actually familiar with common-utils but I believe it is really just a common library for sharing code across multiple plugins.

andrross · 2024-06-27T22:16:57Z

I'm going to close this PR as it looks like we have a path forward as a separate repository. As the feature matures and the code evolves, if the plugin model is no longer the right fit we can absolutely re-evaluate integrating the feature directly into the core, but for now I believe a separate repository is the right place.

sbcd90 requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and dbwiddis as code owners May 26, 2023 02:14

sbcd90 marked this pull request as draft May 26, 2023 02:14

opensearch-trigger-bot bot added the stalled Issues that have stalled label Jun 25, 2023

sbcd90 force-pushed the test14 branch from 6bc37f0 to 20e81ad Compare June 26, 2023 19:52

sbcd90 marked this pull request as ready for review June 26, 2023 19:52

sbcd90 requested a review from sachinpkale as a code owner June 26, 2023 19:52

opensearch-trigger-bot bot removed the stalled Issues that have stalled label Mar 1, 2024

stephen-crawford mentioned this pull request Mar 29, 2024

Finish "add joins, insertion & search to correlation engine" #12983

Closed

8 tasks

add joins, insertion & search to correlation engine

4b4a92f

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

sbcd90 force-pushed the test14 branch from a17c9d1 to 4b4a92f Compare April 23, 2024 19:28

add more rest layer tests

5e649c0

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

praveensameneni mentioned this pull request May 23, 2024

[DOC]: Correlation Engine on OpenSearch Core opensearch-project/documentation-website#4643

Open

3 tasks

shwetathareja reviewed Jun 6, 2024

View reviewed changes

sbcd90 added 2 commits June 17, 2024 18:50

Merge branch 'main' into test14

bea6739

address initial review comments

46dafcb

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>

praveensameneni requested a review from shwetathareja June 18, 2024 00:11

opensearch-ci-bot mentioned this pull request Jun 18, 2024

[AUTOCUT] Gradle Check Flaky Test Report for RecoveryFromGatewayIT #14304

Open

andrross requested changes Jun 20, 2024

View reviewed changes

andrross closed this Jun 27, 2024

add joins, insertion & search to correlation engine #7771

add joins, insertion & search to correlation engine #7771

Conversation

sbcd90 commented May 26, 2023 • edited Loading

Description

Join Handler

Insertion Handler

Search Handler

REST apis

Correlate an event

Search Correlated Events for the input event

Related Issues

Check List

github-actions bot commented May 26, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Jun 25, 2023

github-actions bot commented Jun 26, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Jun 26, 2023 • edited Loading

Codecov Report

github-actions bot commented Apr 23, 2024

github-actions bot commented Apr 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 18, 2024

andrross left a comment

Choose a reason for hiding this comment

dblock commented Jun 25, 2024

praveensameneni commented Jun 25, 2024

reta commented Jun 26, 2024 • edited Loading

praveensameneni commented Jun 26, 2024

andrross commented Jun 26, 2024

praveensameneni commented Jun 26, 2024

reta commented Jun 26, 2024

praveensameneni commented Jun 27, 2024

praveensameneni commented Jun 27, 2024

andrross commented Jun 27, 2024 • edited Loading

andrross commented Jun 27, 2024

sbcd90 commented May 26, 2023 •

edited

Loading

codecov bot commented Jun 26, 2023 •

edited

Loading

reta commented Jun 26, 2024 •

edited

Loading

andrross commented Jun 27, 2024 •

edited

Loading