[ASM] - Expandr 4735 #27624
Merged
[ASM] - Expandr 4735 #27624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, we score and return all owners in ${alert.asmserviceowner} in sorted owners; instead, we want ${alert.asmserviceowner} to contain a smaller, high-confidence set of owners that we would be comfortable notifying via email.
Test plan: pytest + manual testing in tenant
johnnywilkes
approved these changes
Jun 22, 2023
There was a problem hiding this comment.
@kball-pa , Approved. Nice work!
@michal-dagan , please merge when possible
ptoman-pa
reviewed
Jun 22, 2023
Comment on lines
+148
to
+152
| scores: Iterable[float], | ||
| target_k: int = 5, | ||
| k_tol: int = 2, | ||
| a_tol: float = 1.0, | ||
| min_score_proportion: float = 0.75 |
There was a problem hiding this comment.
can you defensively verify that these values are within expected bounds? (for instance, that k_tol isn't -3.)
ptoman-pa
reviewed
Jun 22, 2023
| def test_get_k(): | ||
| """ | ||
| These cases are designed to specify the intuition we are trying to implement with the algorithm. | ||
| They are specific to a target value of 5; if the target_k changes, these tests should update to reflect that. |
There was a problem hiding this comment.
The docstring should be generic, so mentioning 5 is not appropriate here unless you're actually setting that parameter in _get_k -- instead you might mention that this test verifies the default case. (The place to note "5" is in the docstring for _get_k -- there it is worth flagging that all the default values are tuned for k=5.)
ptoman-pa
approved these changes
Jun 22, 2023
|
Hi @kball-pa,
Please feel free to reach out to me with any questions - I'm available here or on slack :) |
michal-dagan
added
the
pending-contributor
kball-pa
requested review from
dansterenson,
Ni-Knight,
mmhw,
moishce,
altmannyarden,
amshamah419,
thefrieddan1,
ilappe,
ilaner,
Shellyber,
GuyAfik,
DeanArbel and
ostolero
as code owners
|
|
michal-dagan
removed request for
DeanArbel,
Ni-Knight,
dansterenson,
amshamah419,
Shellyber,
GuyAfik,
altmannyarden,
ilappe,
ostolero,
ilaner,
mmhw and
moishce
|
@michal-dagan I increased the test coverage and verified that all checks were passing on a previous commit 5d6c938, but now the |
michal-dagan
added
the
ready-for-instance-test
michal-dagan
added a commit
that referenced
this pull request
Jul 6, 2023
* Update ranking algorithm for Service Ownership
Currently, we score and return all owners in ${alert.asmserviceowner} in sorted owners; instead, we want ${alert.asmserviceowner} to contain a smaller, high-confidence set of owners that we would be comfortable notifying via email.
Test plan: pytest + manual testing in tenant
* Add release notes
* Verify hyperparameters and update docs
* Add unit test for value-checking in _get_k
* Update release notes
* Manually apply diff in output of pre-commit check: use built-ins for type hints, set generators
---------
Co-authored-by: kball-pa <131012047+kball-pa@users.noreply.github.com>
Co-authored-by: michal-dagan <109464765+michal-dagan@users.noreply.github.com>
xsoar-bot
pushed a commit
to xsoar-contrib/content
that referenced
this pull request
Jul 26, 2023
* Update ranking algorithm for Service Ownership
Currently, we score and return all owners in ${alert.asmserviceowner} in sorted owners; instead, we want ${alert.asmserviceowner} to contain a smaller, high-confidence set of owners that we would be comfortable notifying via email.
Test plan: pytest + manual testing in tenant
* Add release notes
* Verify hyperparameters and update docs
* Add unit test for value-checking in _get_k
* Update release notes
* Manually apply diff in output of pre-commit check: use built-ins for type hints, set generators
---------
Co-authored-by: kball-pa <131012047+kball-pa@users.noreply.github.com>
Co-authored-by: michal-dagan <109464765+michal-dagan@users.noreply.github.com>
xsoar-bot
pushed a commit
to xsoar-contrib/content
that referenced
this pull request
Aug 2, 2023
* Update ranking algorithm for Service Ownership
Currently, we score and return all owners in ${alert.asmserviceowner} in sorted owners; instead, we want ${alert.asmserviceowner} to contain a smaller, high-confidence set of owners that we would be comfortable notifying via email.
Test plan: pytest + manual testing in tenant
* Add release notes
* Verify hyperparameters and update docs
* Add unit test for value-checking in _get_k
* Update release notes
* Manually apply diff in output of pre-commit check: use built-ins for type hints, set generators
---------
Co-authored-by: kball-pa <131012047+kball-pa@users.noreply.github.com>
Co-authored-by: michal-dagan <109464765+michal-dagan@users.noreply.github.com>
xsoar-bot
pushed a commit
to xsoar-contrib/content
that referenced
this pull request
Aug 2, 2023
* Update ranking algorithm for Service Ownership
Currently, we score and return all owners in ${alert.asmserviceowner} in sorted owners; instead, we want ${alert.asmserviceowner} to contain a smaller, high-confidence set of owners that we would be comfortable notifying via email.
Test plan: pytest + manual testing in tenant
* Add release notes
* Verify hyperparameters and update docs
* Add unit test for value-checking in _get_k
* Update release notes
* Manually apply diff in output of pre-commit check: use built-ins for type hints, set generators
---------
Co-authored-by: kball-pa <131012047+kball-pa@users.noreply.github.com>
Co-authored-by: michal-dagan <109464765+michal-dagan@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Contributing to Cortex XSOAR Content
Make sure to register your contribution by filling the contribution registration form
The Pull Request will be reviewed only after the contribution registration form is filled.
Status
Related Issues
https://jira-hq.paloaltonetworks.local/browse/EXPANDR-4735
Description
Currently, we score and return all owners in ${alert.asmserviceowner} in sorted order. Some of these may be service accounts or other low-confidence users/owners that we don't want to notify.
This PR implements a ranking algorithm for Service Ownership that tries to find a smaller (targeting roughly ~5), high-confidence set of owners that we would be comfortable notifying via email. After the Service Ownership playbook runs, ${alert.asmserviceowner} will contain this smaller, high-confidence set, while ${alert.asmserviceownerunrankedraw} will contain the full set of (deduplicated) owners pulled during enrichment.
See unit tests for detailed specification for how the ranking algorithm works.
Test plan: pytest + manual testing in tenant (see JIRA ticket)
Minimum version of Cortex XSOAR
Does it break backward compatibility?
Must have