Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] [Platform] Adds support for data views and runtime field mappings in rule creation, exceptions, and during execution #130929
[Security Solution] [Platform] Adds support for data views and runtime field mappings in rule creation, exceptions, and during execution #130929
Changes from 62 commits
4ab6be4
195480b
c29db7d
1e5de0d
452e7ea
818e0eb
51c9db1
111e7c0
c241aa6
e3c0d21
a463132
628e7e7
1744714
603b72c
47c9baf
95bebf0
9f554de
b684a26
1c51bea
8ab0609
864c178
2e7381a
c77ec73
d68a8b9
c6088f2
bad4c7e
b152cf8
52985b2
9966b44
120e02f
9b2b7d9
113c937
e48bb65
f754090
d734c12
cab37f3
c36ccae
b57ed3a
60328cc
28de147
362a3e1
5f94d20
c2776c6
3f416bb
b2c5586
14a6c55
bb97d75
f272d37
1c1b8fa
4774d8f
6a5e490
7fd22fb
c39e57d
58b2432
3722a4a
a4c90b4
d0fbb9b
b9e8656
bdddbce
0696ce8
1bb5867
c076b76
b873b55
d91afd4
110f277
51cd358
2ce7c44
3ff1d99
34dd775
da37d23
471d9fb
39d1f3d
e8bb209
7b8e62a
a8a8d98
8974a8f
d27f613
45a6f70
1b4a9c6
f26eeae
9c85a3f
e35a168
e8e9e5a
5be1079
7754d84
c9d18ad
ebeed1f
6954036
9baef65
f5e0989
24d181a
357121e
c1bb307
b5954cc
4f69c78
b535d5c
ef10521
3b39205
2b2e531
6ef4f2a
6637e05
5d069c8
645ca2b
594b1c0
56bed04
bebc731
d2da1da
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Nit: Looks like we're starting to define these in numerous places - could probably be moved to
kbn-securitysolution-utils
.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.
We've got one over here:
kibana/x-pack/plugins/security_solution/common/machine_learning/helpers.ts
Line 27 in e15b887
and here as well too btw:
kibana/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/preview_histogram.tsx
Line 83 in 5ee7c3d
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.
this seems like a strange change cc @paul-tavares
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.
I remember copying code similar to this from/to another place in the data loaders. I had to do it in order for the documents to get picked up by the transforms (I think @joeypoon also had to do something similar). It had something to do with ensuring that we did not send a document whose timestamp had lapse the transform process windows
@joeypoon do you remember? I also know we use these data loaders from FTR and you were just making changes there recently to improve it.
Also - if there is a better way to do this, we should change it. Maybe Stop the transform, then restart it? 🤷♂️
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.
For some reason the script was loading alerts into the future? This fixed that.. Not sure if you've also seen that or if it was a design feature or me not using it properly...
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, I remember seeing that @dhurley14 . But now I wonder if your change broke the loading of the Endpoints.
@joeypoon , @pzl - can you checkout this PR and test it? Load some endpoints (ensure you use the
--fleet
flag) and then validate that they all show up on the Endpoint List.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.
I don't recall this specific piece of code but the
'@timestamp': hostMetadata['@timestamp'] + 60000
slightly below this I definitely added to get around the transform checkpoint times as you mentioned @paul-tavares. We do actually use the stop transform, insert, then start transform instead in some places now too as you mentioned. I agree that this is a better solution.Will test this out locally.
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.
To my slight confusion, loading endpoints still works. I got 30/30 loaded. I'd need to dig a bit deeper to see why it's working without the offset now. We did change the transform delay back to
4s
recently from1s
but IIRC, when this offset was initially added, we were already at a4s
delay.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 removing with this PR, even if it is a change needed for endpoint. That way we don't increase the surface level of possible bugs here.
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.
@joeypoon I undid these changes here 3b39205
I think this was the only piece of code that triggered the code owners from OLM right?
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.
I believe that's correct.