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] Integrates into serverless kibana #159433

Merged
merged 47 commits into from
Jul 1, 2023

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Jun 10, 2023

  • Adds function to ML plugin server side setup contract (setFeaturesEnabled) to enable/disable features. Features being, anomaly detection, data frame analytics or natural language processing.
    • Each serverless plugin (search, observability and security) now calls setFeaturesEnabled to enable different features on setup.
  • Adds three new feature capabilities, isADEnabled, isDFAEnabled, isNLPEnabled
    • Updates the capabilities switcher to toggle these capabilities based on the shared feature API. This currently has a bug where the switcher isn't triggered for API tag checking.
  • deeplinks are now only registered based on the feature capabilities, these control what is shown in the nav menu for search and observability projects.
  • Adds the Machine Learning nav menu section to the serverless_search plugin, to match the serverless_observability plugin.
  • Does not updated the serverless_security plugin's nav menu. (update, serverless_observability nav menu also now does not contain ML)

Relates to #160891

"console",
"searchprofiler",
"dashboard",
Copy link
Member Author

Choose a reason for hiding this comment

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

adding ml but also sorting the list alphabetically

@@ -13,10 +13,11 @@
"security"
],
"requiredPlugins": [
"serverless",
"kibanaReact",
Copy link
Member Author

Choose a reason for hiding this comment

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

adding ml but also sorting the list alphabetically

@jgowdyelastic jgowdyelastic requested a review from darnautov June 26, 2023 12:54
@jgowdyelastic
Copy link
Member Author

jgowdyelastic commented Jun 28, 2023

@kpatticha That's odd. It works ok for me. This is the nav menu I see.
image

How are you running kibana?
I'm using yarn serverless-oblt

I suspect ML just isn't enabled at all in the es that you are running.

@kpatticha
Copy link
Contributor

I suspect ML just isn't enabled at all in the es that you are running

hmm, that could be. Are you running --trial ?

kpatticha added a commit that referenced this pull request Jun 28, 2023
fixes: #159681
fixes: #153777


<img width="1470" alt="image"
src="https://github.com/elastic/kibana/assets/3369346/eb810c65-c780-4597-9570-4b30cf2e1b09">


### Related
ML deep links won't show until it's merged
#159433

### Test
- e2e will be covered #160674
@qn895
Copy link
Member

qn895 commented Jun 28, 2023

On non-serverless mode, the navigation is now like this. Is this intentional?

Screen Shot 2023-06-28 at 12 28 49

@jgowdyelastic
Copy link
Member Author

On non-serverless mode, the navigation is now like this. Is this intentional?

No, not intentional, this has been fixed here

@jgowdyelastic
Copy link
Member Author

jgowdyelastic commented Jun 29, 2023

@kpatticha Apologies, it looks like you were right, the ML menu had very recently been removed from the observability menu in this PR
I had merged this PR with main via github, but not updated my local copy, hence me still seeing it in the menu.

I have updated this PR to put the ML menu back in. Once this PR is in, I believe the menus will be the responsibility of the each serverless plugin team, so the ML links can be moved about wherever needed. I've just added an ML section in this PR as an initial step.

@@ -94,6 +94,10 @@ const navigationTree: NavigationTreeDefinition = {
},
],
},
{
type: 'navGroup',
Copy link
Contributor

Choose a reason for hiding this comment

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

In the recent mockups, we won't use the ml presets for the observability side nav but instead a custom node tree. It would be great if you revert this change.

Copy link
Member Author

@jgowdyelastic jgowdyelastic Jun 29, 2023

Choose a reason for hiding this comment

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

I've removed this ML section 3a7eae1

It's worth pointing out for other reviews that until these custom ML links mentioned above are added, there will be no way to access ML pages from the side navigation menu when running the serverless Observability project.
It will still be possible to access ML pages from kibana's app search feature at the top of kibana.

Copy link
Contributor

@kpatticha kpatticha left a comment

Choose a reason for hiding this comment

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

oblt LGTM. I just left a small comment regarding the preset.

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 1762 1761 -1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 3.4MB 3.4MB +1014.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
ml 33 32 -1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 73.0KB 73.8KB +856.0B
serverlessObservability 21.8KB 21.7KB -71.0B
serverlessSearch 25.7KB 25.7KB -42.0B
total +743.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 413 417 +4
serverlessObservability 4 5 +1
total +7

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 492 496 +4
serverlessObservability 4 5 +1
total +7

History

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

cc @jgowdyelastic

@qn895
Copy link
Member

qn895 commented Jun 30, 2023

Tested latest changes and LGTM 🎉

@jgowdyelastic jgowdyelastic merged commit 055a104 into elastic:main Jul 1, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 1, 2023
@jgowdyelastic jgowdyelastic deleted the ml-serverless-compatibility branch July 1, 2023 07:08
@peteharverson peteharverson added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Aug 22, 2023
@peteharverson peteharverson changed the title [ML] Integrating into serverless kibana [ML] Integrates into serverless kibana Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting :ml release_note:skip Skip the PR/issue when compiling release notes v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.