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

[Event log][7.x] Updated event log client to search across legacy IDs #109365

Merged
merged 32 commits into from
Sep 3, 2021
Merged

[Event log][7.x] Updated event log client to search across legacy IDs #109365

merged 32 commits into from
Sep 3, 2021

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Aug 20, 2021 •
edited
Loading

Summary

Resolves #108077

Checklist

@YulNaumenko YulNaumenko self-assigned this Aug 20, 2021
@YulNaumenko YulNaumenko added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:EventLog v7.15.0 release_note:skip Skip the PR/issue when compiling release notes labels Aug 20, 2021
@YulNaumenko
Copy link
Contributor Author

@elasticmachine merge upstream

@YulNaumenko YulNaumenko marked this pull request as ready for review August 22, 2021 05:25
@YulNaumenko YulNaumenko requested a review from a team as a code owner August 22, 2021 05:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

mikecote
mikecote reviewed Aug 23, 2021
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

I had a few nit/questions as I was passing by this PR.

x-pack/plugins/event_log/server/es/cluster_client_adapter.ts Outdated Show resolved Hide resolved
x-pack/plugins/event_log/server/es/cluster_client_adapter.ts Outdated Show resolved Hide resolved
x-pack/plugins/event_log/server/es/cluster_client_adapter.ts Outdated Show resolved Hide resolved
@YulNaumenko YulNaumenko mentioned this pull request Aug 23, 2021
Closed
@YulNaumenko YulNaumenko requested a review from mikecote August 24, 2021 04:57
@ymao1
Copy link
Contributor

ymao1 commented Aug 24, 2021

Should there be a v8.0.0 label on this? It's pointed at master

@mikecote mikecote added v7.16.0 and removed v7.15.0 labels Aug 25, 2021
mikecote
mikecote reviewed Aug 25, 2021
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

I did another pass through the code, it's getting closer. Below are the 3 scenarios I tested:

Rule created in what version? Has version added to event log documents? Is legacy ID? Alert table still works after upgrading to 8.0.0?
7.15.0 No Yes No
7.16.0 Yes Yes No
8.0.0 Yes No Yes

I'm thinking we may need some functional tests to ensure these three scenarios work and still work down the line. The query will become sensitive when we change in the future. I've attached a zip of my Elasticsearch data folder (use with yarn es snapshot -E path.data=...)

es (all 3 scenarios).zip

x-pack/plugins/alerting/server/rules_client/rules_client.ts Show resolved Hide resolved
x-pack/plugins/alerting/server/rules_client/rules_client.ts Outdated Show resolved Hide resolved
x-pack/plugins/event_log/server/es/cluster_client_adapter.ts Outdated Show resolved Hide resolved
x-pack/plugins/event_log/server/es/cluster_client_adapter.ts Outdated Show resolved Hide resolved
x-pack/plugins/event_log/server/es/cluster_client_adapter.ts Outdated Show resolved Hide resolved
@YulNaumenko
Copy link
Contributor Author

@elasticmachine merge upstream

@YulNaumenko YulNaumenko enabled auto-merge (squash) September 3, 2021 05:12
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
eventLog 70 67 -3

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
alerting 16 17 +1
Unknown metric groups

API count

id before after diff
eventLog 70 67 -3

History

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

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit d421c4d into elastic:master Sep 3, 2021
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 3, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into vis/migrate-r…
7fe9e60 
…eporting-to-v2

* 'master' of github.com:elastic/kibana: (65 commits)
  Move to vis_types folder part 2 (elastic#110574)
  [SOR] use initialNamespaces when checking for conflict for `create` and `bulkCreate` (elastic#111023)
  [Discover] Remove export* syntax (elastic#110934)
  [Event log][7.x] Updated event log client to search across legacy IDs (elastic#109365)
  [Security Solution][Detection Rules] Changes 'activated' text on rule details page  (elastic#111044)
  [Metrics UI] Filter out APM nodes from the inventory view (elastic#110300)
  [package testing] Update logging and pid configuration (elastic#111059)
  [Dashboard] Read App State from URL on Soft Refresh (elastic#109354)
  Add correct roles to test user for functional tests in dashboard (elastic#110880)
  [DOCS] Adds Lens Inspector and minor edits (elastic#109736)
  [DOCS] Updates Spaces page (elastic#111005)
  normalize initialNamespaces (elastic#110936)
  [Reporting] Clean up `any` usage, reorganize server route files (elastic#110740)
  [Security Solution] [CTI] Fixes bug that caused Threshold and Indicator Match rules to ignore custom rule filters if a saved query was used in the rule definition. (elastic#109253)
  skip flaky suites: elastic#111001, elastic#111022
  [Security Solution][RAC] - Update reason field text (elastic#110308)
  [RAC][Security Solution] Make analyzer work with EuiDataGrid full screen (elastic#110913)
  [Metrics UI] Add integration tests for Metric Threshold Rule and refactor to fire correctly (elastic#109971)
  [DOCS] Updates Discover docs (elastic#110346)
  [RAC] Persistent timeline fields fix (elastic#110685)
  ...
@YulNaumenko YulNaumenko mentioned this pull request Sep 3, 2021
Closed
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Sep 3, 2021
@YulNaumenko @kibanamachine @mikecote
[Event log][7.x] Updated event log client to search across legacy IDs (…
a9bc35c 
…elastic#109365)

* [Event log][7.x] Updated event log client to search across legacy IDs

* fixed tests

* extended kibana null version check

* added logic to alerting plugin

* fixed typechecks

* fixed typechecks

* Revert "fixed typechecks"

This reverts commit 6f6770f.

* removed legacyId for routes

* fixed typechecks

* fixed position

* fixed query

* fixed query

* fixed tests

* fixed types place

* fixed due to comments

* fixed due to comments

* fixed eslint

* fixed due to comments

* splitted test data

* fixed test data

* increased the delay time to await the search

* removed version for 7.9 docs

* Update x-pack/plugins/event_log/server/es/cluster_client_adapter.ts

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* fixed unit test

* fixed test data

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 7, 2021
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Sep 8, 2021
@YulNaumenko
[7.x] Added support for 7.x for the event log client to search across…
e3696b3 
… legacy IDs (elastic#109365)
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

5 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109365 or prevent reminders by adding the backport:skip label.

5 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109365 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109365 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109365 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109365 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109365 or prevent reminders by adding the backport:skip label.

@mikecote
Copy link
Contributor

@YulNaumenko regarding the pings above ^^ is there a backport missing or it's already merged as a separate PR?

@mikecote
Copy link
Contributor

I answered my own question: #111176 (comment).

@mikecote mikecote added backport:skip This commit does not require backporting and removed v7.16.0 labels Sep 23, 2021
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikecote mikecote mikecote approved these changes

@pmuellr pmuellr Awaiting requested review from pmuellr

Assignees

@YulNaumenko YulNaumenko

Labels
backport:skip This commit does not require backporting Feature:EventLog release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Event log][7.x] Update event log client to search across legacy IDs
6 participants
@YulNaumenko @elasticmachine @ymao1 @pmuellr @kibanamachine @mikecote