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.

Sign up for GitHub

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

Jump to bottom

[RAM] Adds revision to event-log #153716

Merged
merged 25 commits into from
Apr 22, 2023
Merged

[RAM] Adds revision to event-log #153716

merged 25 commits into from
Apr 22, 2023

Conversation

spong
Copy link
Member

@spong spong commented Mar 24, 2023 •
edited
Loading

Summary

Follow on from #151388 & #147398, which includes the rule's current revision when writing to the kibana event-log.

Note: Added as kibana.alert.rule.revision instead of as ECS field rule.version as the ECS docs conflate version & revision and figured it was best to be explicit. If we do indeed want to use rule.version I'll make the change.

Checklist

Delete any items that are not applicable to this PR.

@banderror banderror mentioned this pull request Mar 31, 2023
Closed
4 tasks
@spong spong added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:EventLog Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Apr 20, 2023
@spong spong added Team:Detection Rule Management Security Detection Rule Management Team v8.8.0 labels Apr 20, 2023
@spong spong marked this pull request as ready for review April 20, 2023 01:05
@spong spong requested review from a team as code owners April 20, 2023 01:05
@spong spong requested a review from banderror April 20, 2023 01:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@pmuellr
Copy link
Member

pmuellr commented Apr 20, 2023

If we do indeed want to use rule.version I'll make the change.

Given our bright and shiny new / future model-version "stuff" (for zero-downtime upgrades), I'm thinking we may end up finding a use for rule.version for that, so I'm quite happy with kibana.alert.rule.revision for this.

vitaliidm
vitaliidm approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

Alerts area changes LGTM

banderror
banderror approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Changes on the Rules area side LGTM, thanks @spong 👍
Tested the PR locally and everything worked fine.
:shipit:

pmuellr
pmuellr approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM; concur with Georgii's comments

@spong spong enabled auto-merge (squash) April 21, 2023 22:33
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #3 / timeline flyout button the (+) button popover menu owns focus

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
alerting 584 585 +1
triggersActionsUi 512 513 +1
total +2
Unknown metric groups

API count

id before after diff
alerting 605 606 +1
triggersActionsUi 541 542 +1
total +2

ESLint disabled line counts

id before after diff
enterpriseSearch 16 18 +2
securitySolution 395 398 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 17 19 +2
securitySolution 475 478 +3
total +5

History

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

cc @spong

@spong spong merged commit ef64acf into elastic:main Apr 22, 2023
@spong spong deleted the adds-revision-to-event-log branch April 22, 2023 00:35
@banderror banderror added the Feature:Rule Monitoring Security Solution Detection Rule Monitoring area label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmuellr pmuellr pmuellr approved these changes

@banderror banderror banderror approved these changes

@vitaliidm vitaliidm vitaliidm approved these changes

Assignees

@spong spong

Labels
backport:skip This commit does not require backporting Feature:EventLog Feature:Rule Monitoring Security Solution Detection Rule Monitoring area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@spong @elasticmachine @pmuellr @kibana-ci @banderror @vitaliidm