-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Logs UI] Reorganise log rate anomaly table #69516
[Logs UI] Reorganise log rate anomaly table #69516
Conversation
Remove top level anomalies chart
…e table can use it
… log rate table can use it" This reverts commit f80db59.
Yeah, that's a fair point. Implementing
|
…/anomalies/expanded_row.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
- Pass href to context menu items - Fix start and end times used for example logs - Use "fewer" rather than "less"
…/kibana into 63671-reorganise-log-rate-table
@weltenwort I just wanted to circle back to this:
As I had some questions once I got to implementation. In this work we alter the table that shows log rate anomalies. In #64755 we'll add category anomalies to the table. But the chart remains linked to log rate anomalies only. If the start and / or end times of the bucket are used, that makes sense for now with log rate anomalies. But there won't be associated buckets for the category anomalies. A few questions:
I'm thinking across the two sets of work at this point. Also, the new anomaly API just returns all the anomalies, within the time range (future, this would have dataset filtering too). And whilst the time of the anomaly occurence would be there, it would need to be linked client side back into the bucketed data that deals with the chart-centric data (doable, of course, just wanted to highlight it). |
Good point. Showing the anomaly start time (which corresponds to the ML-controlled bucketing) for the anomaly rows seems more sensible as opposed to using the chart bucketing. |
I guess we're still aiming for a swimlane-like chart at some point, which could then contain all kinds of anomalies. |
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 couldn't provoke the problem with the missing log entries anymore 👍
I left a few questions and nits below. Do you plan on adding the time column we discussed earlier in this PR or a follow-up?
x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx
Outdated
Show resolved
Hide resolved
dataset: string, | ||
exampleCount: number, | ||
sourceConfiguration: InfraSource, | ||
callWithRequest: KibanaFramework['callWithRequest'] |
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.
Would there be a downside to using context.core.elasticsearch.legacy.client.callAsCurrentUser
instead of this?
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.
If I'm honest I'm not sure - I noticed that with the Kibana framework callWithRequest
we do a little bit of preprocessing to the ES params, before calling callAsCurrentUser
, things like:
const includeFrozen = await uiSettings.client.get(UI_SETTINGS.SEARCH_INCLUDE_FROZEN);
if (endpoint === 'msearch') {
const maxConcurrentShardRequests = await uiSettings.client.get(
UI_SETTINGS.COURIER_MAX_CONCURRENT_SHARD_REQUESTS
);
if (maxConcurrentShardRequests > 0) {
params = { ...params, max_concurrent_shard_requests: maxConcurrentShardRequests };
}
}
this seemed important, so I opted to use that version. Is it safe to bypass the shard settings and so on?
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.
That's true... so I'm not respecting that when fetching the category examples 😬 I guess we should make a search
function available via the context at some point that does something similar to our framework function.
So this is technically more correct right now 👍
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 guess we should make a search function available via the context at some point that does something similar to our framework function.
Yep, sounds good.
x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts
Outdated
Show resolved
Hide resolved
The time column in the row, and additional empty log example header to account for the context menu are coming in here (just weren't ready with the other commits earlier). Thanks for the new feedback 👍 |
…/anomalies/table.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…/anomalies/log_entry_example.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…/anomalies/log_entry_example.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…/anomalies/table.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…try_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…try_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…try_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
…try_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
- Fix typechecking - Add an empty log example column header to account for the context menu - Add anomaly start time to rows
@weltenwort I've responded to everything (so far) now. Other than the outstanding ML link question, and the The dataset alignment is fixed: And I've added a sortable |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
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.
The page looks good and seems to work well. This is a good basis for the inclusion of category anomalies IMHO.
Yet another piece of great work! 👏
* Remove top level chart Remove top level anomalies chart * Refactor table columns to accomodate new formatting * Tyical vs actual stats in expanded row * Format message based on actual vs typical * Start fleshing out log rate examples endpoint and lib methods * Use the real document ID for expanded rows so React doesn't re-render content * Add all data fetching resources for log entry rate examples * Move log entry example and severity indicator components to a shared location * Render examples for log rate * Add severity indicator * Styling tweaks * Move horizontal button popover menu to a shared components so log rate table can use it * Revert "Move horizontal button popover menu to a shared components so log rate table can use it" This reverts commit f80db59. * Add "view in stream" and "view in anomaly explorer" links * Hook links into the new context menu component * Add log column headers and add styling tweaks etc * Fix translations * Tweak comments * Chart tweaks * Update x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/expanded_row.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * PR amendments - Pass href to context menu items - Fix start and end times used for example logs - Use "fewer" rather than "less" * Update x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/table.tsx Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * Update x-pack/plugins/infra/server/routes/log_analysis/results/log_entry_rate_examples.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> * PR amendments - Fix typechecking - Add an empty log example column header to account for the context menu - Add anomaly start time to rows Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>
* master: [APM-UI] e2e speed up build (elastic#70704) skip flaky suite (elastic#70764) skip flaky suite (elastic#70762) [Security Solution][Endpoint] Update to new manifest format (without compression) (elastic#70752) [functional tests] test url field formatter on dashboard and discover (elastic#70736) logout from transform_poweruser user in after method of transform tests (elastic#70644) [SECURITY] Bug fix for topN on draggables (elastic#70450) [Logs UI] Reorganise log rate anomaly table (elastic#69516) Update dependency @elastic/charts to v19.7.0 (elastic#69791) Add googlecloud metricbeat module to Kibana Home (elastic#70652) [Logs UI] Logs overview queries for the observability dashboard (elastic#70413) [Lens] Fitting functions (elastic#69820)
* master: [APM-UI] e2e speed up build (elastic#70704) skip flaky suite (elastic#70764) skip flaky suite (elastic#70762) [Security Solution][Endpoint] Update to new manifest format (without compression) (elastic#70752) [functional tests] test url field formatter on dashboard and discover (elastic#70736) logout from transform_poweruser user in after method of transform tests (elastic#70644) [SECURITY] Bug fix for topN on draggables (elastic#70450) [Logs UI] Reorganise log rate anomaly table (elastic#69516) Update dependency @elastic/charts to v19.7.0 (elastic#69791) Add googlecloud metricbeat module to Kibana Home (elastic#70652) [Logs UI] Logs overview queries for the observability dashboard (elastic#70413)
* actions/feature: fixed type errors [APM-UI] e2e speed up build (elastic#70704) skip flaky suite (elastic#70764) skip flaky suite (elastic#70762) [Security Solution][Endpoint] Update to new manifest format (without compression) (elastic#70752) [functional tests] test url field formatter on dashboard and discover (elastic#70736) logout from transform_poweruser user in after method of transform tests (elastic#70644) [SECURITY] Bug fix for topN on draggables (elastic#70450) [Logs UI] Reorganise log rate anomaly table (elastic#69516) Update dependency @elastic/charts to v19.7.0 (elastic#69791) Add googlecloud metricbeat module to Kibana Home (elastic#70652) [Logs UI] Logs overview queries for the observability dashboard (elastic#70413)
Summary
This PR implements #63671, and lays a lot of the foundations for #64755.
From a high-level the following changes have been made:
A new API endpoint has been created to retrieve log entry rate log examples. This endpoint is very similar to the log category example endpoint. I didn't want to try and merge / DRY up these two endpoints too early. They do have a lot of similarities, especially at the route handler level, but the actual queries and types differ. When [Logs UI] Show category anomalies in anomaly table #64755 is implemented it may make sense to to have these converge (in places).
The loading / no results / has failed to load results portion of the log examples display has been refactored into a reusable component - this is now being used for the log entry rate examples, and the category examples on the categories page.
The severity indicator is also reusable now.
The API for results currently remains log entry rate "graph centric" and not anomaly centric, and the client side just formats the data in a way that makes sense for displaying the new anomaly table format. For [Logs UI] Show category anomalies in anomaly table #64755 the API will be altered to be anomaly centric, and will return log entry rate and category anomalies in one. (This is to say that this doesn't need to be commented on for this round of work).
There are some things "left behind" here, like the log rate graph. [Logs UI] anomaly view cleanup #65046 will handle cleaning these parts up.
Table is now paginated as we're displaying a row per anomaly and not per dataset.
Testing
For variety it can be helpful to have data from several datasets.