-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] [AIOps] Log Rate Analysis: Shows top N results when no docs in baseline or deviation #171924
[ML] [AIOps] Log Rate Analysis: Shows top N results when no docs in baseline or deviation #171924
Conversation
Pinging @elastic/ml-ui (:ml) |
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.
Tested and LGTM. Just a couple of typos need correcting.
x-pack/plugins/aiops/public/components/log_rate_analysis/log_rate_analysis_type_callout.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/aiops/public/components/log_rate_analysis/log_rate_analysis_type_callout.tsx
Outdated
Show resolved
Hide resolved
); | ||
} | ||
|
||
if (version === '2') { |
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.
Version 1 and 2 look pretty much identical here - is this duplication purely for type reasons?
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.
Just a small difference, version 1 is accessing significantTerms
whereas version 2 uses significantItems
.
import { getQueryWithParams } from './get_query_with_params'; | ||
import { getRequestBase } from './get_request_base'; | ||
|
||
// TODO Consolidate with duplicate `fetchDurationFieldCandidates` in |
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.
Is this comment still needed?
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.
Yes, this is an older comment I copied over, it's related to this issue where we track potential cleanups of duplicated code: #136182
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.
Left a couple of small comments but overall LGTM ⚡
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @walterra |
Summary
Resolves #169767.
This PR adds a fallback analysis when either the baseline or deviation time range contains no documents. Instead of trying to fetch significant items (which would return no results), this will just fetch the top terms/categories for the time range that has some documents.
Analysis type: Top items for deviation time range when no docs in baseline
Analysis type: Top items for baseline time range when no docs in deviation
Checklist