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] Explain Log Rate Spikes: Fix field candidate selection. #147614

Merged
merged 7 commits into from
Dec 19, 2022

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Dec 15, 2022

Summary

Fixes #146992.

The field candidate selection for Explain Log Rate Spikes was missing a check if the supported field type is also aggregatable. For example, a keyword type field could still be non-aggregatable if it was both not indexed and doc_values set to false. Additionally, if no groups were detected, we showed a "Try to continue analysis" button in the UI even if the analysis was able to finish.

In this PR the artificial logs dataset for functional tests was extended to include a field like that. Without the fixes of this PR the outcome in the UI with it looks like this:

image

With the fix applied to not show the button we get:

image

Finally, with the additional fix for field selection the whole warning callout disappears:

image

Checklist

@walterra walterra added bug Fixes for quality problems that affect the customer experience release_note:fix :ml Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v8.6.0 v8.7.0 labels Dec 15, 2022
@walterra walterra self-assigned this Dec 15, 2022
@walterra walterra requested a review from a team as a code owner December 15, 2022 14:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

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.

Test changes LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
aiops 750.6KB 750.6KB +7.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 61 67 +6
osquery 109 115 +6
securitySolution 439 445 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 70 76 +6
osquery 110 117 +7
securitySolution 516 522 +6
total +21

History

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

cc @walterra

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.

LGTM ⚡

@walterra walterra merged commit aecad27 into elastic:main Dec 19, 2022
@walterra walterra deleted the 146992-fix-field-candidate-selection branch December 19, 2022 17:35
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 19, 2022
…#147614)

The field candidate selection for Explain Log Rate Spikes was missing a
check if the supported field type is also aggregatable. For example, a
`keyword` type field could still be non-aggregatable if it was both not
indexed and `doc_values` set to `false`. Additionally, if no groups were
detected, we showed a "Try to continue analysis" button in the UI even
if the analysis was able to finish. In this PR the artificial logs dataset for functional tests was extended
to include a field like that.

(cherry picked from commit aecad27)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 19, 2022
…147614) (#147792)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[ML] Explain Log Rate Spikes: Fix field candidate selection.
(#147614)](#147614)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Walter
Rafelsberger","email":"walter.rafelsberger@elastic.co"},"sourceCommit":{"committedDate":"2022-12-19T17:35:31Z","message":"[ML]
Explain Log Rate Spikes: Fix field candidate selection. (#147614)\n\nThe
field candidate selection for Explain Log Rate Spikes was missing
a\r\ncheck if the supported field type is also aggregatable. For
example, a\r\n`keyword` type field could still be non-aggregatable if it
was both not\r\nindexed and `doc_values` set to `false`. Additionally,
if no groups were\r\ndetected, we showed a \"Try to continue analysis\"
button in the UI even\r\nif the analysis was able to finish. In this PR
the artificial logs dataset for functional tests was extended\r\nto
include a field like
that.","sha":"aecad27159764d8ea2d0aeddc94fd03954d480e5","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix",":ml","Feature:ML/AIOps","v8.6.0","v8.7.0"],"number":147614,"url":"#147614
Explain Log Rate Spikes: Fix field candidate selection. (#147614)\n\nThe
field candidate selection for Explain Log Rate Spikes was missing
a\r\ncheck if the supported field type is also aggregatable. For
example, a\r\n`keyword` type field could still be non-aggregatable if it
was both not\r\nindexed and `doc_values` set to `false`. Additionally,
if no groups were\r\ndetected, we showed a \"Try to continue analysis\"
button in the UI even\r\nif the analysis was able to finish. In this PR
the artificial logs dataset for functional tests was extended\r\nto
include a field like
that.","sha":"aecad27159764d8ea2d0aeddc94fd03954d480e5"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"#147614
Explain Log Rate Spikes: Fix field candidate selection. (#147614)\n\nThe
field candidate selection for Explain Log Rate Spikes was missing
a\r\ncheck if the supported field type is also aggregatable. For
example, a\r\n`keyword` type field could still be non-aggregatable if it
was both not\r\nindexed and `doc_values` set to `false`. Additionally,
if no groups were\r\ndetected, we showed a \"Try to continue analysis\"
button in the UI even\r\nif the analysis was able to finish. In this PR
the artificial logs dataset for functional tests was extended\r\nto
include a field like
that.","sha":"aecad27159764d8ea2d0aeddc94fd03954d480e5"}}]}] BACKPORT-->

Co-authored-by: Walter Rafelsberger <walter.rafelsberger@elastic.co>
crespocarlos pushed a commit to crespocarlos/kibana that referenced this pull request Dec 23, 2022
…#147614)

The field candidate selection for Explain Log Rate Spikes was missing a
check if the supported field type is also aggregatable. For example, a
`keyword` type field could still be non-aggregatable if it was both not
indexed and `doc_values` set to `false`. Additionally, if no groups were
detected, we showed a "Try to continue analysis" button in the UI even
if the analysis was able to finish. In this PR the artificial logs dataset for functional tests was extended
to include a field like that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis :ml release_note:fix v8.6.0 v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Explain Log Rate Spikes: Fix field candidates detection.
6 participants