-
Notifications
You must be signed in to change notification settings - Fork 117
Security machine learning rule documentation improvements #2240
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
Conversation
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.
Left one small suggestion, otherwise, everything lgtm!
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.
Hi @susan-shu-c 👋. That variable is {{ml-app}} now. Line 100 in c7d2e6f
edit: hmm, we also have ml-cap Line 185 in c7d2e6f
edit edit: preview looks fine to me: ![]() |
solutions/security/advanced-entity-analytics/anomaly-detection.md
Outdated
Show resolved
Hide resolved
@@ -14,9 +14,16 @@ products: | |||
# Anomaly detection |
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.
Whoever reviews this from my team: should we link to this page from:
- https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/2240/solutions/security/explore/hosts-page,
- https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/2240/solutions/security/explore/network-page
- https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/2240/solutions/security/explore/users-page
?
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 would make sense to link to it from the Data tables sections on those pages.
solutions/security/advanced-entity-analytics/anomaly-detection.md
Outdated
Show resolved
Hide resolved
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.
Thanks for making these updates, @jmcarlock!
solutions/security/advanced-entity-analytics/anomaly-detection.md
Outdated
Show resolved
Hide resolved
solutions/security/advanced-entity-analytics/anomaly-detection.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Brandon Morelli <[email protected]>
Co-authored-by: natasha-moore-elastic <[email protected]>
Co-authored-by: natasha-moore-elastic <[email protected]>
Co-authored-by: Brandon Morelli <[email protected]>
Co-authored-by: natasha-moore-elastic <[email protected]>
Co-authored-by: natasha-moore-elastic <[email protected]>
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.
Looks good, ++ to Kirti's suggestion.
Thanks @bmorelli25 @natasha-moore-elastic @susan-shu-c @sodhikirti07 for the review! I made additions/changes based on the feedback. @natasha-moore-elastic can you take one more look? |
@jmcarlock, I pushed some minor tweaks – should be good to merge now! 🚀 |
Related issue
https://github.com/elastic/security-ml/issues/776