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] Adds validation of field selected for log pattern analysis #162319
[ML] Adds validation of field selected for log pattern analysis #162319
Conversation
import { boomify, isBoom } from '@hapi/boom'; | ||
import { ResponseError, CustomHttpResponseOptions } from '@kbn/core/server'; | ||
|
||
export function wrapError(error: any): CustomHttpResponseOptions<ResponseError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A duplicate of the function in ML.
Could possibly be moved to a package if we ever need it in another plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same is already in multiple plugins. We already have @kbn/ml-error-utils
and could move it there, but I think for now this package is only used for client side utils, so we need to refactor that to be split into server/client side code to not unnecessarily blow up client side bundle sizes.
import { i18n } from '@kbn/i18n'; | ||
import { isRuntimeField } from '@kbn/ml-runtime-field-utils'; | ||
|
||
export const runtimeMappingsSchema = schema.object( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if we should add runtimeMappingsSchema to ml-runtime-field-utils
as we do use it in several plugins 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved in 40fb370
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reverted this move. I caused a 200KB bundle increase in ML and Transforms.
x-pack/plugins/aiops/public/components/log_categorization/use_categorize_request.ts
Outdated
Show resolved
Hide resolved
@@ -217,6 +251,8 @@ export const LogCategorizationFlyout: FC<LogCategorizationPageProps> = ({ | |||
</EuiFlexGroup> | |||
</EuiFlyoutHeader> | |||
<EuiFlyoutBody data-test-subj="mlJobSelectorFlyoutBody"> | |||
<FieldValidationCallout validationResults={fieldValidationResult} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this example, the field does not produce any warnings because the data is tokenized correctly but it also does not produce any categories. I suspect this is because every doc contains the same data.
|
||
return ( | ||
<> | ||
{validationResults !== null ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess this ternary can be removed since the same check is done above and returns early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 914c43d
import { boomify, isBoom } from '@hapi/boom'; | ||
import { ResponseError, CustomHttpResponseOptions } from '@kbn/core/server'; | ||
|
||
export function wrapError(error: any): CustomHttpResponseOptions<ResponseError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same is already in multiple plugins. We already have @kbn/ml-error-utils
and could move it there, but I think for now this package is only used for client side utils, so we need to refactor that to be split into server/client side code to not unnecessarily blow up client side bundle sizes.
x-pack/plugins/aiops/tsconfig.json
Outdated
"@kbn/ml-date-picker", | ||
"@kbn/ml-error-utils", | ||
"@kbn/ml-is-defined", | ||
"@kbn/ml-is-populated-object", | ||
"@kbn/ml-kibana-theme", | ||
"@kbn/ml-kibana-theme", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate entry here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 914c43d
…elastic/kibana into pattern-analysis-field-validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest changes LGTM
LGTM 🎉 |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsAPI count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great you brought down the bundle size again, latest changes LGTM!
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>
Uses the recently created category validation package 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:
message
in the data view and auto select it./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
Log Pattern Analysis flyout in Discover