-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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][Resolver] Allow a configurable entity_id field #81679
[Security Solution][Resolver] Allow a configurable entity_id field #81679
Conversation
...gins/security_solution/server/endpoint/routes/resolver/new_tree/queries/aggregation_utils.ts
Outdated
Show resolved
Hide resolved
...gins/security_solution/server/endpoint/routes/resolver/new_tree/queries/aggregation_utils.ts
Outdated
Show resolved
Hide resolved
...ck/plugins/security_solution/server/endpoint/routes/resolver/new_tree/queries/descendants.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
let startTime = new Date(timestampSafeVersion(tree.allEvents[0]) ?? startOfEpoch); | ||
expect(startTime).not.toEqual(startOfEpoch); | ||
let endTime = new Date(timestampSafeVersion(tree.allEvents[0]) ?? startOfEpoch); | ||
expect(startTime).not.toEqual(startOfEpoch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be endTime
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, thanks, copy/paste issue 😆
startTime = currentEventTime; | ||
} | ||
|
||
if (currentEventTime > endTime) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know how likely it is for currentEventTime
to equal endTime
or startTime
, but maybe just use an else as a catch all?
if (currentEventTime < startTime) {
startTime = currentEventTime
} else {
endTime = currentEventTime
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually they need to be separate if statements
haha. That's why I just got a random test failure (also not great, I'll try to fix that too). Using the if-else will mean that endTime
will be reset anytime currentEventTime
is greater than or equal to startTime
, which isn't what we want. We want endTime
to be set anytime currentEventTime
is greater than endTime
otherwise we could set endTime
to something less than the greatest event time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, makes sense. Ignore me, I'm bad with 🕐
if (eventTimestamp !== undefined) { | ||
if (eventTimestamp < startTime) { | ||
startTime = eventTimestamp; | ||
} else if (eventTimestamp > endTime) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here? Not sure how likely it is to happen from the generated data 🤷♂️
export const validateTree = { | ||
body: schema.object({ | ||
// if the ancestry field is specified this field will be ignored | ||
descendantLevels: schema.number({ defaultValue: 20, min: 0, max: 1000 }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remind me why the ancestry affects the descendantLevels again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it affects it because if we have the ancestry
field defined than we have a much more performant way of retrieving levels so my thought was to not limit the number of levels that come back in that scenario. We could still limit it, but what we'd likely have to do is get all the levels back like we normally do with the ancestry array, bucket them together by level, and then remove the levels that exceeded the requested number which seems kind of wasteful.
I'll add this as a comment as well 👍
* "process.entity_id": [ | ||
* "6k8waczi22" | ||
* ] | ||
* }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth adding the name
to the example as well?
There was a problem hiding this comment.
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 add name
.
@elasticmachine merge upstream |
…ibana into resolver-multi-fields
This reverts commit 9e9abf6.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
…81679) (#84251) * Trying to flesh out new tree route * Working on the descendants query * Almost working descendants * Possible solution for aggs * Working aggregations extraction * Working on the ancestry array for descendants * Making changes to the unique id for ancestr * Implementing ancestry funcitonality * Deleting the multiple edges * Fleshing out the descendants loop for levels * Writing tests for ancestors and descendants * Fixing type errors and writing more tests * Renaming validation variable and deprecating old tree routes * Renaming tree integration test file * Adding some integration tests * Fixing ancestry to handle multiple nodes in the request and writing more tests * Adding more tests * Renaming new tree to handler file * Renaming new tree directory * Adding more unit tests * Using doc value fields and working on types * Adding comments and more tests * Fixing timestamp test issue * Adding more comments * Fixing timestamp test issue take 2 * Adding id, parent, and name fields to the top level response * Fixing generator start and end time generation * Adding more comments * Revert "Fixing generator start and end time generation" This reverts commit 9e9abf6. * Adding test for time Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* master: (41 commits) [Maps] fix code-owners (elastic#84265) [@kbn/utils] Clean target before build (elastic#84253) [code coverage] collect for oss integration tests (elastic#83907) [APM] Use `asTransactionRate` consistently everywhere (elastic#84213) Attempt to fix incremental build error (elastic#84152) Unskip "Copy dashboards to space" (elastic#84115) Remove expressions.legacy from README (elastic#79681) Expression: Add render mode and use it for canvas interactivity (elastic#83559) [deb/rpm] Move systemd service to /usr/lib/systemd/system (elastic#83571) [Security Solution][Resolver] Allow a configurable entity_id field (elastic#81679) [ML] Space permision checks for job deletion (elastic#83871) [build] Provide ARM build of RE2 (elastic#84163) TSVB should use "histogram:maxBars" and "histogram:barTarget" settings for auto instead of a default 100 buckets (elastic#83628) [Workplace Search] Initial rendering of Org Sources (elastic#84164) update geckodriver to 0.28 (elastic#84085) Fix timelion vis escapes single quotes (elastic#84196) [Security Solution] Fix incorrect time for dns histogram (elastic#83532) [DX] Bump TS version to v4.1 (elastic#83397) [Security Solution] Add endpoint policy revision number (elastic#83982) [Fleet] Integration Policies List view (elastic#83634) ...
This PR adds a new
/tree
api that is not dependent on a single ID. Since this api will only return a single document per node in the resolver graph we'll need to update the/events
api to support requesting all the lifecycle events for multiple nodes at a time. This PR does not include the needed/events
api changes.Once the front end is transitioned over to the new api we'll be able to remove the old
/tree
route that was centered around a singleprocess.entity_id
and all the supporting code and tests.It includes a new
/tree
api that supports:Testing
Generate a tree:
Make a request:
curl command:
Example response: