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][Endpoint] Refactor resolver generator for ancestry array #70129

Merged

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Jun 26, 2020

This PR refactors how the generator passes in options for building the tree. This makes assigning options like how large of an ancestry array to use easier to use throughout the generator.

I also added a levels field in the tree response to make it easier to access the levels of the tree when using the generator in tests.

These are changes I've made while implementing msearch for resolver's backend and needed extra functionality for the tests.

@jonathan-buttner jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver Feature:Endpoint Elastic Endpoint feature v7.9.0 labels Jun 26, 2020
@jonathan-buttner jonathan-buttner requested review from a team as code owners June 26, 2020 21:54
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@@ -492,6 +523,11 @@ export class EndpointDocGenerator {
* @param options - Allows event field values to be specified
*/
public generateEvent(options: EventOptions = {}): EndpointEvent {
let ancestry: string[] | undefined;
if (options?.useAncestryArray === true || options?.ancestry !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need a useAncestryArray option separate from the ancestry option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good point. I'll remove it. We can accomplish the same thing with ancestry = 0 👍

@jonathan-buttner
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@jonathan-buttner jonathan-buttner merged commit 37d7d78 into elastic:master Jul 1, 2020
@jonathan-buttner jonathan-buttner deleted the generator-refactor branch July 1, 2020 19:13
jonathan-buttner added a commit that referenced this pull request Jul 2, 2020
…y array (#70129) (#70518)

* Refactor generator for ancestry support

* Adding optional ancestry array

* Fixing tests and type errors

* Removing useAncestry field

* Fixing test

* An empty array will be returned because that's how ES will do it too

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

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Endpoint Elastic Endpoint feature release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants