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

New events resolver #82170

Merged
merged 41 commits into from
Nov 9, 2020
Merged

Conversation

IgorGuz2000
Copy link
Contributor

@IgorGuz2000 IgorGuz2000 commented Oct 30, 2020

Summary

Added Alert Test for Resolver and fix for the APM enabled Run fail

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@IgorGuz2000 IgorGuz2000 added release_note:fix Team:Endpoint Data Visibility Team managing the endpoint resolver Feature:Resolver Security Solution Resolver feature v7.10.0 labels Oct 30, 2020
@IgorGuz2000 IgorGuz2000 requested review from a team as code owners October 30, 2020 17:31
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Resolver)

@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 30, 2020
@@ -121,6 +121,8 @@ export function SecurityHostsPageProvider({ getService, getPageObjects }: FtrPro
for (let i = 0; i < NodeSubmenuItems.length; i++) {
await (await testSubjects.findAll('resolver:map:node-submenu-item'))[i].click();
const Events = await testSubjects.findAll('resolver:map:node-submenu-item');
// this sleep is for the AMP enabled run
await pageObjects.common.sleep(300);
Copy link
Contributor

Choose a reason for hiding this comment

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

How much does this sleep add to the duration of every test? Is there a way you can convert this to a wait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I try to do the wait and different ways nothing works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it just adds 2 sec per test only

Copy link
Contributor

Choose a reason for hiding this comment

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

I support a Sleep if we need the tests passing and to unblock the build. Always good to question it and follow up more if we can improve it

Copy link
Contributor

@marshallmain marshallmain left a comment

Choose a reason for hiding this comment

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

This PR adds more than 8MB of data to the kibana repo that every kibana dev will now be pulling and storing locally. Can the es_archive size be reduced?

@@ -121,6 +121,8 @@ export function SecurityHostsPageProvider({ getService, getPageObjects }: FtrPro
for (let i = 0; i < NodeSubmenuItems.length; i++) {
await (await testSubjects.findAll('resolver:map:node-submenu-item'))[i].click();
const Events = await testSubjects.findAll('resolver:map:node-submenu-item');
// this sleep is for the AMP enabled run
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo here, can you change it to APM in a subsequent PR please

@EricDavisX
Copy link
Contributor

That's wise for us to take the time to review all of the ES-Archive datasets we have and see if we can safely combine them to reduce overall size. but I submit we can do that in a separate pr. Progress over perfection.

Copy link
Contributor

@EricDavisX EricDavisX left a comment

Choose a reason for hiding this comment

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

ok to merge, needs some follow ups in subsequent pr

@IgorGuz2000
Copy link
Contributor Author

@elasticmachine merge upstream

@marshallmain
Copy link
Contributor

marshallmain commented Nov 8, 2020

@EricDavisX I'm primarily concerned that this new archive being added into the source tree for a single test is enormous. Looking at the uncompressed data, I see over 95000 documents and 10,000,000 lines of text.

@charlie-pichette
Copy link
Contributor

charlie-pichette commented Nov 9, 2020

@IgorGuz2000 95,000 documents in the archive file seems like a lot for a test checking a single node. Since all the documents are for the same host we should remove the extraneous documents and keep only the ones we need to validate the test. This would also improve the test run time.

@IgorGuz2000
Copy link
Contributor Author

This test is testing Alert event and all the data is the related events of this alert. I use Jeff script to pull the data only of one alert and this is what I get.

@achuguy
Copy link
Contributor

achuguy commented Nov 9, 2020

Can you check what events show up in kibana and what indices you are storing? you should have ~800 events based on

const expectedAlertData = [
          '1 library',
          '157 file',
          '520 registry',
          '3 file',
          '5 library',
          '5 library',
        ];

But marshall mentioned you are storing ~95k docs

@IgorGuz2000
Copy link
Contributor Author

I am not counting all of them

@achuguy
Copy link
Contributor

achuguy commented Nov 9, 2020

The process tree you're examining has 11k documents?

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@IgorGuz2000 IgorGuz2000 merged commit bf75831 into elastic:master Nov 9, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 10, 2020
* master: (39 commits)
  Fix ilm navigation (elastic#81664)
  [Lens] Distinct icons for XY and pie chart value labels toolbar (elastic#82927)
  [data.search.aggs] Throw an error when trying to create an agg type that doesn't exist. (elastic#81509)
  Index patterns api - load field list on server (elastic#82629)
  New events resolver (elastic#82170)
  [App Search] Misc naming tech debt (elastic#82770)
  load empty_kibana in test to have clean starting point (elastic#82772)
  Remove data <--> expressions circular dependencies. (elastic#82685)
  Update 8.0 breaking change template to gather information on how to programmatically detect it. (elastic#82905)
  Add alerting as codeowners to related documentation folder (elastic#82777)
  Add captions to user and space grid pages (elastic#82713)
  add alternate path for x-pack/Cloud test for Lens (elastic#82634)
  Uses asCurrentUser in getClusterUuid (elastic#82908)
  [Alerting][Connectors] Add new executor subaction to get 3rd party case fields (elastic#82519)
  Fix test import objects (elastic#82767)
  [ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate (elastic#81662)
  Update alert type selection layout to rows instead of grid (elastic#73665)
  Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. (elastic#82817)
  Update grunt and related packages (elastic#79327)
  Allow the repository to search across all namespaces (elastic#82863)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 11, 2020
@kibanamachine
Copy link
Contributor

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

2 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 82170 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 82170 or prevent reminders by adding the backport:skip label.

IgorGuz2000 added a commit to IgorGuz2000/kibana that referenced this pull request Nov 16, 2020
* Added Test for event.library

* renamed data directry and gzip data file

* rename expectedData file

* Changes per Charlie request

* Changes for the enable_APM-ci branch

* Update resolver.ts

* Added comment per Charlie request

* Update resolver.ts

* Added Alert Test for Resolver and fix for the APM enabled Run fail

* Added Alert Test for Resolver and fix for the APM enabled Run fail

* removed commented out code

* Fixing CI fail

* Fixing CI fail

* Removed Alert Resolver test

* aAdding Alert test back

* Adding Alert test back

* Adding Alert test back

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* adding one more verification for Data

* stripedd Data file

Co-authored-by: Kibana Machine <[email protected]>
@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.

IgorGuz2000 added a commit that referenced this pull request Nov 17, 2020
* New events resolver (#82170)

* Added Test for event.library

* renamed data directry and gzip data file

* rename expectedData file

* Changes per Charlie request

* Changes for the enable_APM-ci branch

* Update resolver.ts

* Added comment per Charlie request

* Update resolver.ts

* Added Alert Test for Resolver and fix for the APM enabled Run fail

* Added Alert Test for Resolver and fix for the APM enabled Run fail

* removed commented out code

* Fixing CI fail

* Fixing CI fail

* Removed Alert Resolver test

* aAdding Alert test back

* Adding Alert test back

* Adding Alert test back

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* Adding info log for debuging

* adding one more verification for Data

* stripedd Data file

Co-authored-by: Kibana Machine <[email protected]>

* Update hosts_page.ts

* Update resolver.ts

* Update resolver.ts

* Update hosts_page.ts

* Update resolver.ts

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Resolver Security Solution Resolver feature Feature:Visual Testing release_note:fix Team:Endpoint Data Visibility Team managing the endpoint resolver Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants