Skip to content
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

[ML] Adding space aware jobs #77916

Merged
merged 104 commits into from
Nov 3, 2020

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Sep 18, 2020

Adds saved objects to represent ML anomaly detection and data frame analytics jobs to allow for them to be assigned to spaces.

Related to #64172

The new saved object has the type ml-job, and is this shape:

ml-job: {
  job_id: "foo"
  datafeed_id: "datafeed-foo",
  type: "anomaly-detector"
},

datafeed_id will be null for dataframe analytics jobs and anomaly detector jobs with no datafeed.
type will be either anomaly-detector or data-frame-analytics

Job id checks and filters have been added to all appropriate kibana ml endpoints.
For example:

GET <kibana>/abc/api/ml/anomaly_detectors

will only return jobs in the current space.

GET <kibana>/abc/api/ml/anomaly_detectors/foo

Will return a 404 if foo is not in the current space.

All anomaly results searches now use a new function anomalySearch which also requires the list a job ids.
If any of the job ids listed does not exist or is not in the current space, a 404 will be returned.

Things to note when testing:

  • When creating a job in the UI, the job will only exist in the current space.
    • Future work (7.11) will add the ability to move jobs between spaces in kibana's management app.
  • When deleting a job, it will be deleted for all spaces.
    • Future work (7.11) will instead remove the job from the current space and only delete the actual job when it is assigned to only one space.
  • When using the assign_job_to_space or remove_job_from_space endpoints, the job must exist in the current space.
    • This will be fixed in a follow up PR.

When testing this PR, any pre-existing jobs will not have saved objects assigned to them and so will not appear in the UI.
These saved objects can be created by calling:

GET <kibana>/api/ml/saved_objects/repair

To move jobs between spaces:

POST <kibana>/api/ml/saved_objects/assign_job_to_space
or
POST <kibana>/api/ml/saved_objects/remove_job_from_space

Payload:
{
  "jobType": "anomaly-detector",
  "jobIds": [<jobIds>],
  "spaces": [<spaces names>]
}

jobType can be anomaly-detector and data-frame-analytics

Job spaces are also listed in the management page:
image

For APM reviewers, I had to recreate the apm_8.0.0 archive to add the ML saved objects to the snapshot .kibana index.

Follow up work for 7.11:

  • Job management UI for assigning spaces
  • Allow assigning of jobs to any space accessible by the user.
  • Manage job deletion, only remove job from space if it is assigned to multiple spaces.
  • ES upgrade, migration task should create saved objects in * space for all existing jobs.
  • Add functional API tests. We should have strong coverage for all endpoints modified.

Checklist

Delete any items that are not applicable to this PR.

@joshdover joshdover mentioned this pull request Sep 22, 2020
41 tasks
@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

2 similar comments
@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

Latest changes LGTM ⚡

@peteharverson
Copy link
Contributor

If I disable Spaces, with xpack.spaces.enabled: false, I only see jobs from the default space. Is this the correct behavior?
cc @legrego

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.

Left a question about the behaviour when spaces is disabled, but otherwise tested latest edits and LGTM.

@jgowdyelastic
Copy link
Member Author

jgowdyelastic commented Oct 30, 2020

If I disable Spaces, with xpack.spaces.enabled: false, I only see jobs from the default space. Is this the correct behavior?
cc @legrego

I'm not doing anything special here if spaces is disabled. i'm still using the saved object client and these are the jobs it's returning.
I assumed this was intended behaviour.
We would want to continue creating and searching the saved objects when creating and retrieving jobs, just in case spaces is enabled at a future point.

@legrego
Copy link
Member

legrego commented Oct 30, 2020

If I disable Spaces, with xpack.spaces.enabled: false, I only see jobs from the default space. Is this the correct behavior?
cc @legrego

I'm not doing anything special here if spaces is disabled. i'm still using the saved object client and these are the jobs it's returning.
I assumed this was intended behaviour.
We would want to continue creating and searching the saved objects when creating and retrieving jobs, just in case spaces is enabled at a future point.

Yes this is intended behavior, and consistent with the way other saved objects behave today. It's not the best experience, but that's what we have.

@legrego
Copy link
Member

legrego commented Nov 1, 2020

My plan was to finish my review on Monday, but I'm taking a sick day instead; I'll pick this back up as soon as possible.

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Code LGTM 🥳

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
ml 1234 1237 +3

async chunks size

id before after diff
ml 6.6MB 6.6MB +1.9KB

distributable file count

id before after diff
default 42620 42633 +13

page load bundle size

id before after diff
ml 63.5KB 63.5KB -41.0B

Saved Objects .kibana field count

id before after diff
ml-job - 6 +6

History

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

@jgowdyelastic jgowdyelastic merged commit a0fded5 into elastic:master Nov 3, 2020
jgowdyelastic added a commit that referenced this pull request Nov 3, 2020
* [ML] Adding space aware jobs

* adding mlClient

* switching to type includes

* adding additional job checks

* fixing conflict

* adding dfa checks

* refactoring jobs in spaces checks

* filtering calendars

* adding initial job object status and repair endpoints

* enabling repair endpoint

* fixing listed jobs in status

* adding datafeed repair

* updating shared services

* adding results job id check

* fixing conflicts

* don't remove SO on delete

* fixing non-ml plugins

* filtering job audit messages

* fixing types

* fixing tests

* adding job ids wildcard support

* removing empty migration test

* fixing tests and disabling spaces test user

* adding saved objects all permission

* fixing calendars

* updating job 404

* updating job wildcard search

* renaming services

* fixing conflicts

* fixing log tests

* disabling apm test

* skipping more apm tests

* optimzing repair

* fixing types

* updating apm test archive to include ML saved objects

* enabling disabled test

* removing comment

* adding space assigning endpoints

* adding saved object default permissions

* removing commented code

* loading all jobs for all spaces for status check

* adding spaces list endpoint

* adding job spaces to management page

* adding trained model fltering

* fixing trained model id check and job wildcard check

* fixing types

* fixing bug when adding new job to calendar

* changes based on review

* updating schema

* changes based on review

* fixing types

* rolling back http service injection

* fixing http service injection

* adding errrors to repair endpoint response

* updating api doc

* improving types

* disabling id check on ad get endpoints

* fixing tests

* fixing group requests

* adding comments

* using filter in saved object search

* fixing fake request issue

* removing console log

* making job saved object hidden

* removing acccidentally included file

* renaming saved object client

* updating apidoc

* unhiding ml saved objects

* moving route guard

* improving error when SOC is null

* fixing types after merge with master

* fixing tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@jgowdyelastic jgowdyelastic deleted the adding-space-aware-jobs branch November 3, 2020 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Anomaly Detection ML anomaly detection Feature:Data Frame Analytics ML data frame analytics features Feature:New Feature New feature not correlating to an existing feature label :ml release_note:enhancement review Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.