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] Creating category validation package #161261

Merged

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Jul 5, 2023

Moves the server and client side code which performs analysis on data to see whether it is suitable for categorization.
This is currently only used by the categorization job wizard to display this callout:
image

However this analysis will be useful for the Log Pattern Analysis feature and so moving the code to a package allows easier sharing between ML and AIOPs plugins.

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@jgowdyelastic jgowdyelastic self-assigned this Jul 17, 2023
@jgowdyelastic jgowdyelastic added :ml release_note:skip Skip the PR/issue when compiling release notes Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v8.10.0 chore labels Jul 17, 2023
@jgowdyelastic jgowdyelastic marked this pull request as ready for review July 17, 2023 15:47
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner July 17, 2023 15:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra
Copy link
Contributor

Other plugins we maintain are prefixed with either @kbn/aiops-* or @kbn/ml-*. The ml one is more like a prefix as in ML the team, not ML the code or Kibana plugin. Looking back maybe even the aiops ones should be prefixed with ml. Suggest to also prefix this one like @kbn/ml-category-validator. It makes also search for imports we own easier.

@jgowdyelastic
Copy link
Member Author

jgowdyelastic commented Jul 17, 2023

Suggest to also prefix this one like @kbn/ml-category-validator.

Good spot. I missed that the package ids were different to the directory name.
Updated the package name in e397708

@kbn/category_validator -> @kbn/ml-category-validator

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, just added some minor comments.

Would be good to get the missing comments/any counts down, stats can be output like this:

node scripts/build_api_docs --plugin @kbn/ml-category-validator --stats comments
node scripts/build_api_docs --plugin @kbn/ml-category-validator --stats any

@@ -0,0 +1,3 @@
# @kbn/ml-category-validator

Empty package generated by @kbn/generate
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to have a brief description here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I missed that. Thanks
Updated in 91de94b

* 2.0.
*/

export { categorizationExamplesProvider } from './examples';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This file might not be necessary, the index.ts in the root of the package could directly import/export categorizationExamplesProvider.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in 91de94b

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #58 / AIOps POST /internal/aiops/explain_log_rate_spikes - groups only with ecommerce should return group only in chunks with streaming without compression with flushFix

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 1761 1764 +3

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 +71.0B
Unknown metric groups

API count

id before after diff
@kbn/ml-category-validator - 32 +32

History

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

cc @jgowdyelastic

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.

LGTM. Good job getting those API counts down to 0!

Added a comment on the format of comments to add a description for params for future reference.

* Retrieves the tokens for the provided examples and analyzer.
*
* @async
* @param {string} indexPatternTitle
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding this for reference - note that these will generate the docs, but there will be no description of what the param is. the syntax to use is e.g.

 @param {string} indexPatternTitle the title of the index pattern for which to obtain examples

@jgowdyelastic jgowdyelastic merged commit 219426d into elastic:main Jul 19, 2023
21 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 19, 2023
jgowdyelastic added a commit that referenced this pull request Jul 28, 2023
Uses the recently created [category validation
package](#161261) to perform
validation on the field selected for pattern analysis.

If the field is considered unsuitable for categorization, a warning
callout is displayed which lists the reasons it is unsuitable.
If the field is suitable, no callout is displayed.

Other changes:
- Adds the selected field to the URL state, so it is remembered on page
refresh.
- If no field is in the URL, it will look for a field called `message`
in the data view and auto select it.
- renames the ML route `/jobs/categorization_field_examples` to
`/jobs/categorization_field_validation` as it is a more accurate name
and it's consistent with the newly added route in AIOPs.

**Log Pattern Analysis page in ML**


![image](https://github.com/elastic/kibana/assets/22172091/c0dfda8b-bc34-48b7-9e71-8bae9e65bdf3)


**Log Pattern Analysis flyout in Discover**


![image](https://github.com/elastic/kibana/assets/22172091/b4d251f3-bae6-424f-9891-bda57ba1673d)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
Moves the server and client side code which performs analysis on data to
see whether it is suitable for categorization.
This is currently only used by the categorization job wizard to display
this callout:

![image](https://github.com/elastic/kibana/assets/22172091/08db5321-0c38-474d-9bfe-90b8a9ad984a)

However this analysis will be useful for the Log Pattern Analysis
feature and so moving the code to a package allows easier sharing
between ML and AIOPs plugins.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
Uses the recently created [category validation
package](elastic#161261) to perform
validation on the field selected for pattern analysis.

If the field is considered unsuitable for categorization, a warning
callout is displayed which lists the reasons it is unsuitable.
If the field is suitable, no callout is displayed.

Other changes:
- Adds the selected field to the URL state, so it is remembered on page
refresh.
- If no field is in the URL, it will look for a field called `message`
in the data view and auto select it.
- renames the ML route `/jobs/categorization_field_examples` to
`/jobs/categorization_field_validation` as it is a more accurate name
and it's consistent with the newly added route in AIOPs.

**Log Pattern Analysis page in ML**


![image](https://github.com/elastic/kibana/assets/22172091/c0dfda8b-bc34-48b7-9e71-8bae9e65bdf3)


**Log Pattern Analysis flyout in Discover**


![image](https://github.com/elastic/kibana/assets/22172091/b4d251f3-bae6-424f-9891-bda57ba1673d)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
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 chore Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis :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.

None yet

6 participants