[SIEM][Detection Engine] Silence 409 errors on signal creation by rylnd · Pull Request #53859 · elastic/kibana

rylnd · 2019-12-30T23:10:35Z

Summary

Most of our rules run every five minutes on the last six minutes of data. If a signal-generating event occurs in that overlapping minute, we will attempt to create the same signal twice, and receive a 409 response from elasticsearch on the second attempt.

In my exploration of this issue, every 409 response I encountered was due to this behavior. While that does not eliminate the possibility that other circumstances could generate a 409, I was unable to reproduce/imagine such a circumstance, and I think that we can safely hide these errors from the user as it is the expected behavior.

While there's also the option to use bulk index to upsert these signals and create new versions of the same signal, the only value I see in doing that would be the elimination of said errors: the duplicated signal should have no additional information, and the upsert logic has performance (and other) concerns as well.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~[ ] This was checked for cross-browser compatibility, including a check against IE11~~
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

We already had a colon on both uses of this key, resulting in '::' on the page.

In my experience these are always due to a rule being run multiple times on the same document, generating a duplicate signal with a (correctly) duplicate id. Only if we encounter non-409 errors do we log a message to the user.

elasticmachine · 2019-12-30T23:10:37Z

Pinging @elastic/siem (Team:SIEM)

rylnd · 2020-01-02T14:57:15Z

@elasticmachine merge upstream

rylnd · 2020-01-02T15:03:08Z

x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.ts

+    const errorCountsByStatus = countBy(itemsWithErrors, item => item.create.status);
+    const hasNonDuplicateError = Object.keys(errorCountsByStatus).some(status => status !== '409');
+
+    if (hasNonDuplicateError) {


I couldn't decide whether or not to hide these 409 errors in all cases. That seems more consistent with the idea that these are expected errors and could potentially be confusing to the user, but as the logic stands we will show all errors (including 409s) if there are non-409 errors.

FYI: 1bcf978

dhurley14

LGTM Thanks for making these changes.

These are expected and potentially confusing to the user. Instead, we only show unexpected errors.

rylnd · 2020-01-02T18:10:23Z

@elasticmachine merge upstream

kibanamachine · 2020-01-02T19:30:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 41a300d

History

💔 Build #17609 failed 1bcf978
💚 Build #17579 succeeded 40dd650
💚 Build #17478 succeeded 3b9c26e

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

… (#53903) * Remove punctuation from translation We already had a colon on both uses of this key, resulting in '::' on the page. * Ignore 409 errors from our signal creation In my experience these are always due to a rule being run multiple times on the same document, generating a duplicate signal with a (correctly) duplicate id. Only if we encounter non-409 errors do we log a message to the user. * Hide 409 errors during signal creation These are expected and potentially confusing to the user. Instead, we only show unexpected errors. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886) Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899) [Reporting/Tests] consolidate functional test configs (elastic#52671) [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833) [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905) cleanup server-log action (elastic#53326) [Uptime] Delete uptime eslint rule skip (elastic#50912) [skip-ci] Expression Lifecycle Docs (elastic#51494) [Endpoint] add react router to endpoint app (elastic#53808) [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859) [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500) [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521) fixes typo on engine detection page (elastic#53877)

* master: Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886) Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899) [Reporting/Tests] consolidate functional test configs (elastic#52671) [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833) [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905) cleanup server-log action (elastic#53326) [Uptime] Delete uptime eslint rule skip (elastic#50912) [skip-ci] Expression Lifecycle Docs (elastic#51494) [Endpoint] add react router to endpoint app (elastic#53808) [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859) [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500) [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521) fixes typo on engine detection page (elastic#53877) [Maps] push mapbox value extraction from VectorStyle and into DynamicStyleProperty (elastic#53806) Fix suggested value for time_zone in range query (elastic#53841) Clean up generic hooks, use react-use instead (elastic#53822)

…ris/kibana into alerting/created_at-and-updated_at * 'alerting/created_at-and-updated_at' of github.com:gmmorris/kibana: Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886) Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899) [Reporting/Tests] consolidate functional test configs (elastic#52671) [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833) [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905) cleanup server-log action (elastic#53326) [Uptime] Delete uptime eslint rule skip (elastic#50912) [skip-ci] Expression Lifecycle Docs (elastic#51494) [Endpoint] add react router to endpoint app (elastic#53808) [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859) [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500) [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521) fixes typo on engine detection page (elastic#53877) [Maps] push mapbox value extraction from VectorStyle and into DynamicStyleProperty (elastic#53806) Fix suggested value for time_zone in range query (elastic#53841) Clean up generic hooks, use react-use instead (elastic#53822)

…t-types * alerting/created_at-and-updated_at: updatedAt should equal createdAt on creation Move index patterns: src/legacy/core_plugins/data 👉 src/plugins/data (elastic#53794) moved Task Manager server code under "server" directory (elastic#53777) Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886) Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899) [Reporting/Tests] consolidate functional test configs (elastic#52671) [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833) [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905) cleanup server-log action (elastic#53326) [Uptime] Delete uptime eslint rule skip (elastic#50912) [skip-ci] Expression Lifecycle Docs (elastic#51494) [Endpoint] add react router to endpoint app (elastic#53808) [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859) [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500) [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521) fixes typo on engine detection page (elastic#53877) [Maps] push mapbox value extraction from VectorStyle and into DynamicStyleProperty (elastic#53806) Fix suggested value for time_zone in range query (elastic#53841) Clean up generic hooks, use react-use instead (elastic#53822)

rylnd added 2 commits December 30, 2019 16:54

Remove punctuation from translation

31deb25

We already had a colon on both uses of this key, resulting in '::' on the page.

Ignore 409 errors from our signal creation

3b9c26e

In my experience these are always due to a rule being run multiple times on the same document, generating a duplicate signal with a (correctly) duplicate id. Only if we encounter non-409 errors do we log a message to the user.

rylnd added Team:SIEM release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Dec 30, 2019

rylnd requested review from FrankHassanabad and dhurley14 December 30, 2019 23:10

Merge branch 'master' into silence_duplicate_signal_errors

40dd650

rylnd commented Jan 2, 2020

View reviewed changes

dhurley14 approved these changes Jan 2, 2020

View reviewed changes

Hide 409 errors during signal creation

1bcf978

These are expected and potentially confusing to the user. Instead, we only show unexpected errors.

Merge branch 'master' into silence_duplicate_signal_errors

41a300d

rylnd merged commit d9554ff into elastic:master Jan 2, 2020

rylnd deleted the silence_duplicate_signal_errors branch January 2, 2020 19:35

rylnd mentioned this pull request Jan 2, 2020

[7.x] [SIEM][Detection Engine] Silence 409 errors on signal creation (#53859) #53903

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIEM][Detection Engine] Silence 409 errors on signal creation#53859

[SIEM][Detection Engine] Silence 409 errors on signal creation#53859
rylnd merged 5 commits intoelastic:masterfrom
rylnd:silence_duplicate_signal_errors

rylnd commented Dec 30, 2019 •

edited

Loading

Uh oh!

elasticmachine commented Dec 30, 2019

Uh oh!

rylnd commented Jan 2, 2020

Uh oh!

rylnd Jan 2, 2020

Uh oh!

rylnd Jan 2, 2020

Uh oh!

dhurley14 left a comment

Uh oh!

rylnd commented Jan 2, 2020

Uh oh!

kibanamachine commented Jan 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

rylnd commented Dec 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Dec 30, 2019

Uh oh!

rylnd commented Jan 2, 2020

Uh oh!

rylnd Jan 2, 2020

Choose a reason for hiding this comment

Uh oh!

rylnd Jan 2, 2020

Choose a reason for hiding this comment

Uh oh!

dhurley14 left a comment

Choose a reason for hiding this comment

Uh oh!

rylnd commented Jan 2, 2020

Uh oh!

kibanamachine commented Jan 2, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rylnd commented Dec 30, 2019 •

edited

Loading