-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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][Detections] Modify threshold rule synthetic signal generation to use data from last hit in bucket #82444
Conversation
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
const source = { | ||
'@timestamp': new Date().toISOString(), | ||
'@timestamp': get(timestampOverride ?? '@timestamp', hit._source), |
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.
Does this set the threshold signal date time to be the one from the hit?
I ask because previously there has been some debate previously about what a signal's @timestamp
should be as some people prefer the source signal timestamp instead of the current date time that the signal is created but the decision so far has been that the @timestamp
for a signal should be the current date time stamp and then if a rule has an original_timestamp
it would be stored underneath signal.original_time
Threshold is unique in that is synthetic but the @timestamp
should still be the timestamp of when the signal is created for consistency. That doesn't mean that we can't change them all as part of
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.
@FrankHassanabad I thought that too initially, but the signal timestamp is created later. This timestamp is used to populate signal.original_time
(it will be the original time of the last event in the timespan...
kibana/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts
Line 99 in b6d661f
original_time: doc._source['@timestamp'], |
kibana/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts
Line 77 in b6d661f
'@timestamp': new Date().toISOString(), |
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.
Oh I see, thanks. Yeah, that makes sense. I get 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.
Code looks clean, checked out the branch and ran through some tests, everything looks 👍
…l generation to use data from last hit in bucket (elastic#82444) * Fix threshold rule synthetic signal generation * Use top_hits aggregation * Add timestampOverride * Account for when threshold.field is not supplied * Ensure we're getting the last event when threshold.field is not provided * Add missing import
…l generation to use data from last hit in bucket (#82444) (#83213) * Fix threshold rule synthetic signal generation * Use top_hits aggregation * Add timestampOverride * Account for when threshold.field is not supplied * Ensure we're getting the last event when threshold.field is not provided * Add missing import
Summary
Fixes #77253 by using the
top_hits
aggregation to return the most recent hit for each bucket which exceeded the given threshold (or by using the returned search results for threshold rules that are not bucketed by any field). This hit is used to populate the synthetic signal(s) that is generated.This ensures that our signal contains only valid field values, as opposed to potential wildcards or CIDR IP ranges that were used for the original search.
For example,
192.168.0.0/16
(the CIDR ranged used by the rule to search) becomes192.168.0.16
(the host IP from the most recent event captured on the interval).Checklist
For maintainers