Skip to content

Updates to pre-built Security ML jobs#154596

Merged
ajosh0504 merged 10 commits intoelastic:mainfrom
ajosh0504:security_job_update
Apr 24, 2023
Merged

Updates to pre-built Security ML jobs#154596
ajosh0504 merged 10 commits intoelastic:mainfrom
ajosh0504:security_job_update

Conversation

@ajosh0504
Copy link
Contributor

@ajosh0504 ajosh0504 commented Apr 6, 2023

Summary

This PR makes the following updates to the pre-built Security ML jobs:

  • Making the security-packetbeat compatible with Agent
  • Removing superfluous fields from the job configurations to make them consistent
  • Updating the detector_description field for almost all jobs
  • Adding influencers where missing and/or relevant
  • Adding a job_revision custom setting similar to the Logs jobs. Moving forward, this number will be updated each time a job is updated. We are starting with 4 since the linux and windows jobs are at v3 right now
  • Adding a managed: true tag to indicate that these jobs are pre-configured by Elastic and so users will see the warnings added in this PR if users choose to delete, or modify these jobs

@ajosh0504 ajosh0504 requested a review from peteharverson April 6, 2023 19:30
@ajosh0504 ajosh0504 self-assigned this Apr 6, 2023
@ajosh0504 ajosh0504 requested a review from a team as a code owner April 6, 2023 19:30
@ajosh0504 ajosh0504 added release_note:skip Skip the PR/issue when compiling release notes v8.8.0 labels Apr 6, 2023
@ajosh0504 ajosh0504 changed the title Updates to pre-built Secruity ML jobs Updates to pre-built Security ML jobs Apr 6, 2023
Copy link
Contributor

@pjhampton pjhampton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from a security data analytics perspective

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@peteharverson peteharverson added the Feature:Anomaly Detection ML anomaly detection label Apr 11, 2023
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few suggestions for minor edits to job and detector descriptions, but otherwise overall looks good.

Tested creation of most of these jobs in the ML UI:

  • Security: Authentication
  • Security: Cloudtrail
  • Security: Linux
  • Security: Packetbeat
  • Security: Windows

@ajosh0504
Copy link
Contributor Author

@elasticmachine merge upstream

@ajosh0504
Copy link
Contributor Author

ajosh0504 commented Apr 11, 2023

Tested the Packetbeat jobs locally to ensure that the datafeed update is working as expected:

  • Posted documents as follows into a test index

Screen Shot 2023-04-11 at 10 20 25 AM

- Noted that the datafeed for the `packetbeat_rare_dns_question` job recognized the two documents corresponding to DNS datasets:

Screen Shot 2023-04-11 at 10 15 52 AM

"time_field": "@timestamp"
},
"custom_settings": {
"job_tags": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajosh0504 as mentioned, note that the purpose of these job_tags properties was to allow searching on these properties in the ML jobs list, for example for jobs with specific versions

image

or specific euid:

image

and these tags are also displayed in a dedicated section in the expanded job row:

image

See original issue, and the support that was added for them in 7.14: #102099

Note that the Logs UI use an alternative approach for versioning jobs - https://github.com/elastic/kibana/blob/main/x-pack/plugins/ml/server/models/data_recognizer/modules/logs_ui_categories/ml/log_entry_categories_count.json#L38, but this job_revision property is not searchable in the ML job management page.

I am ok with you removing these job_tags if they are no longer providing value, but wanted to provide context on why we originally added them to many of the security jobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peteharverson Thanks for providing context. My thoughts on the above:

  • Afaik we aren't using these fields internally.
  • In terms of searching in the UI, I'm not sure how much of a value searching for v3 jobs is providing. Typically, we see our users upgrading to the latest version of jobs, rather than keeping both old as well as new jobs. Also having versions only for some jobs is a bit odd, given that other jobs (that don't have the version tags) have also undergone updates. I would like to have some versioning for all jobs, but I'd like to discuss this further outside of this PR, given that we are unofficially already several versions along across all our modules.
  • I don't understand the value provided by the euid field, specifically given that it is retained when the job is cloned. I'd understand if it was a unique identifier for a job so as to distinguish between different copies of the same job, but that not being the case, I don't see a ton of value in it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to defer to you on this one @ajosh0504 and remove these job_tags.

The thinking behind euid was that this field couldn't (easily) be changed by the user, whereas they are able to completely change the job ID when cloning, so it could be used to track how many of this type of job had been created.

Makes sense to address a way to version all the security jobs outside of this PR.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good for ML.

@ajosh0504 ajosh0504 requested a review from pheyos April 13, 2023 17:37
@ajosh0504
Copy link
Contributor Author

@pheyos Tagging you to see if anything at y'all's end needs updating given the changes in this PR.

@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 432 435 +3

Total ESLint disabled count

id before after diff
securitySolution 512 515 +3

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ajosh0504

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads-up @ajosh0504! Indeed, we'll have to do some follow-up adjustments to our tests in the ML QA framework once your PR is merged , I'll take care of that.

@ajosh0504 ajosh0504 merged commit 4dc21e5 into elastic:main Apr 24, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Anomaly Detection ML anomaly detection :ml release_note:skip Skip the PR/issue when compiling release notes v8.8.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants