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] adds log info level for logging in cloud #89941

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

dhurley14
Copy link
Contributor

Summary

Adds an info log level statement to execute at the end of a successful rule run whether any signals were found and what date ranges the search executed over.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dhurley14 dhurley14 requested review from a team as code owners February 1, 2021 22:36
@dhurley14 dhurley14 self-assigned this Feb 1, 2021
@dhurley14 dhurley14 added release_note:skip Skip the PR/issue when compiling release notes review Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detections and Resp Security Detection Response Team v7.11.0 v7.12.0 v8.0.0 labels Feb 1, 2021
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@@ -70,6 +70,7 @@ export const searchAfterAndBulkCreate = async ({
interval,
buildRuleMessage,
});
const tuplesToBeLogged = [...totalToFromTuples];
logger.debug(buildRuleMessage(`totalToFromTuples: ${totalToFromTuples.length}`));
Copy link
Member

@spong spong Feb 1, 2021

Choose a reason for hiding this comment

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

Should we remove the debug logline since it's now covered by the info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the execution of the big loop throws an error then the new info line won't get hit so I don't think removing the debug statement would be beneficial in that case.

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out, tested locally and verified new info logger upon rule execution for both successful and failed rule executions. LGTM! This is going to be critical for identifying issues, so thanks for taking the time to get this in for 7.11 @dhurley14! 🎉 🙂

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@dhurley14 dhurley14 merged commit 0283ca0 into elastic:master Feb 2, 2021
@dhurley14 dhurley14 deleted the detections-add-log-info branch February 2, 2021 03:50
dhurley14 added a commit to dhurley14/kibana that referenced this pull request Feb 2, 2021
dhurley14 added a commit to dhurley14/kibana that referenced this pull request Feb 2, 2021
gmmorris added a commit to lizozom/kibana that referenced this pull request Feb 2, 2021
…om/kibana into pr/89570

* 'sessions/save-all-sessions' of https://github.com/lizozom/kibana: (44 commits)
  [ML] Functional tests - skip DFA clone tests
  [Uptime] Fix synthetics detail step count (elastic#89940)
  Fixes the permissions to require cluster.manage in order to create an index and in order to update an index (elastic#89947)
  [Security Solution] [Detections] adds log info level for logging in cloud (elastic#89941)
  [Time to Visualize] Dashboard By Value Testing Lens (elastic#89581)
  [Uptime] Expand synthetic journey step thumbnail on hover (elastic#89179)
  TS project refs: Migrates snapshot_restore to a TS Project (elastic#89653)
  docs: APM 7.11 updates (elastic#89789)
  move skip to higher level (elastic#86952)
  Revert "Migrations v2: don't auto-create indices + FTR/esArchiver support (elastic#85778)"
  Revert "Revert "Enable v2 so migrations, disable in FTR tests (elastic#89297)""
  Revert "Enable v2 so migrations, disable in FTR tests (elastic#89297)"
  [data.search] Allow search response to follow new hits format (elastic#88115)
  [Maps] Change 'create multi-layer map' title to be use-case focused (elastic#89520)
  skip flaky suite (elastic#86952)
  [Security Solution] Remove focustrap (elastic#89905)
  [Workplace Search] Add remaining i18n support for the Content Sources tree (elastic#89910)
  [esArchiver] log when migrations complete and we're done loading data (elastic#89938)
  Add --ssl flag to make resolver generator use ssl with kbn and elasticsearch clients (elastic#89873)
  TS project refs: Migrates grokdebugger (elastic#89652)
  ...
dhurley14 added a commit that referenced this pull request Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.11.0 v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants