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 Solutions] Exposes the search_after and point in time (pit) from saved objects to exception lists #125182

Merged
merged 17 commits into from
Feb 15, 2022

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Feb 10, 2022

Summary

Exposes the functionality of

  • search_after
  • point in time (pit)

From saved objects to the exception lists. This DOES NOT expose these to the REST API just yet. Rather this exposes it at the API level to start with and changes code that had hard limits of 10k and other limited loops. I use the batching of 1k for this at a time as I thought that would be a decent batch guess and I see other parts of the code changed to it. It's easy to change the 1k if we find we need to throttle back more as we get feedback from others.

See this PR where PIT and search_after were first introduced: #89915
See these 2 issues where we should be using more paging and PIT (Point in Time) with search_after: #93770 #103944

The new methods added to the exception_list_client.ts client class are:

  • openPointInTime
  • closePointInTime
  • findExceptionListItemPointInTimeFinder
  • findExceptionListPointInTimeFinder
  • findExceptionListsItemPointInTimeFinder
  • findValueListExceptionListItemsPointInTimeFinder

The areas of functionality that have been changed:

  • Exception list exports
  • Deletion of lists
  • Getting exception list items when generating signals

Note that currently we use our own ways of looping over the saved objects which you can see in the codebase such as this older way below which does work but had a limitation of 10k against saved objects and did not do point in time (PIT)

Older way example (deprecated):

  let page = 1;
  let ids: string[] = [];
  let foundExceptionListItems = await findExceptionListItem({
    filter: undefined,
    listId,
    namespaceType,
    page,
    perPage: PER_PAGE,
    pit: undefined,
    savedObjectsClient,
    searchAfter: undefined,
    sortField: 'tie_breaker_id',
    sortOrder: 'desc',
  });
  while (foundExceptionListItems != null && foundExceptionListItems.data.length > 0) {
    ids = [
      ...ids,
      ...foundExceptionListItems.data.map((exceptionListItem) => exceptionListItem.id),
    ];
    page += 1;
    foundExceptionListItems = await findExceptionListItem({
      filter: undefined,
      listId,
      namespaceType,
      page,
      perPage: PER_PAGE,
      pit: undefined,
      savedObjectsClient,
      searchAfter: undefined,
      sortField: 'tie_breaker_id',
      sortOrder: 'desc',
    });
  }
  return ids;

But now that is replaced with this newer way using PIT:

  // Stream the results from the Point In Time (PIT) finder into this array
  let ids: string[] = [];
  const executeFunctionOnStream = (response: FoundExceptionListItemSchema): void => {
    const responseIds = response.data.map((exceptionListItem) => exceptionListItem.id);
    ids = [...ids, ...responseIds];
  };

  await findExceptionListItemPointInTimeFinder({
    executeFunctionOnStream,
    filter: undefined,
    listId,
    maxSize: undefined, // NOTE: This is unbounded when it is "undefined"
    namespaceType,
    perPage: 1_000,
    savedObjectsClient,
    sortField: 'tie_breaker_id',
    sortOrder: 'desc',
  });
  return ids;

We also have areas of code that has perPage listed at 10k or a constant that represents 10k which this removes in most areas (but not all areas):

      const items = await client.findExceptionListsItem({
        listId: listIds,
        namespaceType: namespaceTypes,
        page: 1,
        pit: undefined,
        perPage: MAX_EXCEPTION_LIST_SIZE, // <--- Really bad to send in 10k per page at a time
        searchAfter: undefined,
        filter: [],
        sortOrder: undefined,
        sortField: undefined,
      });

That is now:

      // Stream the results from the Point In Time (PIT) finder into this array
      let items: ExceptionListItemSchema[] = [];
      const executeFunctionOnStream = (response: FoundExceptionListItemSchema): void => {
        items = [...items, ...response.data];
      };

      await client.findExceptionListsItemPointInTimeFinder({
        executeFunctionOnStream,
        listId: listIds,
        namespaceType: namespaceTypes,
        perPage: 1_000,
        filter: [],
        maxSize: undefined, // NOTE: This is unbounded when it is "undefined"
        sortOrder: undefined,
        sortField: undefined,
      });

Left over areas will be handled in separate PR's because they are in other people's code ownership areas.

Checklist

@FrankHassanabad FrankHassanabad self-assigned this Feb 10, 2022
@FrankHassanabad FrankHassanabad added v8.2.0 v8.1.0 Team:Security Solution Platform Security Solution Platform Team release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed labels Feb 12, 2022
@FrankHassanabad FrankHassanabad marked this pull request as ready for review February 12, 2022 22:21
@FrankHassanabad FrankHassanabad requested review from a team as code owners February 12, 2022 22:21
Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

Code LGTM! It's super clean and easy to go through, especially with all the comments and tests you added that add lots of context. I really like the patterns you introduced too that will make it super easy to move the rest over to PIT.

Just fetching down now to play around with it and then will LGTM!

@FrankHassanabad FrankHassanabad removed v8.1.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Feb 15, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lists 293 296 +3
securitySolution 2847 2850 +3
total +6

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
@kbn/securitysolution-io-ts-list-types 428 442 +14
lists 155 161 +6
total +20

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lists 140.3KB 140.6KB +285.0B
securitySolution 4.7MB 4.7MB +280.0B
total +565.0B

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
lists 43 49 +6
Unknown metric groups

API count

id before after diff
@kbn/securitysolution-io-ts-list-types 439 453 +14
lists 185 197 +12
total +26

ESLint disabled in files

id before after diff
@kbn/securitysolution-io-ts-list-types 12 14 +2

Total ESLint disabled count

id before after diff
@kbn/securitysolution-io-ts-list-types 12 14 +2

History

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

cc @FrankHassanabad

@FrankHassanabad FrankHassanabad merged commit 81c5fbf into elastic:main Feb 15, 2022
@FrankHassanabad FrankHassanabad deleted the add-pit-exception-lists branch February 15, 2022 23:05
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 17, 2022
@kibanamachine
Copy link
Contributor

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

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

@marshallmain marshallmain added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Security Solution Platform Security Solution Platform Team v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants