[SIEM] [Detection Engine] Search signals index#52661
[SIEM] [Detection Engine] Search signals index#52661dhurley14 merged 5 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/siem (Team:SIEM) |
There was a problem hiding this comment.
Awesome tests! I should back fill my to have at least the basics like this. Really appreciate it!
There was a problem hiding this comment.
I'd just drop the search_query and use { query since you already have the URL for search. Would feel more natural to the ES querying of things.
There was a problem hiding this comment.
If you dropped the search_query, you could change this to just be that query is required? Unless you were planning more than just query to be accepted.
There was a problem hiding this comment.
If you wanted aggregations too you could then do a "at least one of these is required" with the top level keys being:
query
aggregationsJust kind of asking since I have to do these type of endpoints myself for rules
There was a problem hiding this comment.
Ah yeah I forgot about aggregations. Good call.
There was a problem hiding this comment.
Yeah, ++ on aggregations -- we'll want to use this for the Signals histogram.
There was a problem hiding this comment.
Woa, I don't think you want the -v here for the example. That adds a lot of extra stuff! :-)
There was a problem hiding this comment.
very true - was debugging and forgot to take it out. Thanks 👍
There was a problem hiding this comment.
You mean signals/aggs_signals.sh on this line
There was a problem hiding this comment.
Optional, if you're not using _headers you can just do: async handler(request: SignalsQueryRequest)
There was a problem hiding this comment.
Optional, if you're not using headers you can just do: async handler(request: SignalsStatusRequest)
FrankHassanabad
left a comment
There was a problem hiding this comment.
Found one comment in a sh file that was a copy-pasta and 2 optionals.
Outside of that, I checked this out, tested it, and everything looks great!
LGTM!
…query, adds aggs to signals search api, updates route and validation tests
907115d to
7ec68f9
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* adds route for querying signals index, also updates signal status type names * first pass at happy path tests * fixes stuff after rebase with master * utilizes removes search_query from payload and replaces it with just query, adds aggs to signals search api, updates route and validation tests * removes _headers parameter from route handler and updates comment for aggs script
* adds route for querying signals index, also updates signal status type names * first pass at happy path tests * fixes stuff after rebase with master * utilizes removes search_query from payload and replaces it with just query, adds aggs to signals search api, updates route and validation tests * removes _headers parameter from route handler and updates comment for aggs script
Summary
Adds route for searching signals index and sample usage script. Also updates naming for signals status schemas.
There is a sample usage query in
scripts/signals/query_signals.shin addition to an aggs script inscripts/signals/aggs_signals.sh. Ensure you have a signal doc in the signals index. There is a sample doc that can be indexed via thesignals/put_signal_doc.shscript.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers