[Logs UI] Add sorting capabilities to categories page #88051

Kerry350 · 2021-01-12T17:00:28Z

Summary

This PR implements #58314.

Note

For maximum anomaly score and message count this is a simple numeric sort. For the "trend percentage" there is a little more work involved as the change factor is based off of two values, and if a category is newly detected it won't have a change factor.

If there's a mixture of new items, and items with a change factor, then depending on the order the new items are prepended (ascending sort) or appended (descending order) to the sorted items.

If the list only contains new items, there will be no visual change on sort.

Testing

Maximum anomaly score, message count, and trend should all be sortable.
Maximum anomaly score should now be the default
In the case where the trend contains "new" items this should still work, based on the implementation above.
- It can help to just recreate the job to test the above, and then wait for trend values to appear.

elasticmachine · 2021-01-12T17:00:33Z

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

weltenwort · 2021-01-14T10:31:37Z

@elasticmachine merge upstream

weltenwort

So... this sorts the 25 categories in the browser after the server already sorted them by document count and truncated them to the requested number.

Doesn't that mean that an anomaly with a very high score in a category with low document count might never show up even though the user sorts by anomaly score?

Should we delegate the sorting to ES? It'll be tricky for the trend column because these values are fetched separately. 🤔

Kerry350 · 2021-01-14T12:11:31Z

@weltenwort Yes, I think my interpretation was probably wrong here. I thought we'd be keeping the "top" nature but then allowing a sort. But in hindsight that doesn't really make sense.

Should we delegate the sorting to ES?

Yeah. It will largely mirror the sort implementation I did for the anomalies page.

It'll be tricky for the trend column because these values are fetched separately.

Yeah. I'm just looking at the way fetchTopLogEntryCategoryHistograms depends on fetchTopLogEntryCategories and so on. That is pretty tricky, I'll see if I can come up with something there 🤔

TODO: Trend sorting

weltenwort · 2021-01-14T15:03:18Z

Maybe we can reduce the scope to not make the trend sortable? (@mukeshelastic)

Kerry350 · 2021-01-14T15:44:16Z

Maybe we can reduce the scope to not make the trend sortable?

👍 Makes sense to me. The "new" aspect makes things more complicated as well. It's not a particularly good field for a primary sort.

Kerry350 · 2021-01-14T16:18:23Z

I've pushed the changes that switch this for server side sorting for maximum anomaly score and message count. I've also reversed anything to do with trend sorting.

@mukeshelastic Do you think this is a suitable compromise? It's not trivial to sort on the trend as the values are derived from different places. There might be a way to combine the queries, but it's certainly not a small change.

weltenwort

@Kerry350, thanks for revisiting the server-side sorting. It works great for the count and the anomaly score and is implemented very cleanly. 👍

While I'd still like to hear from @mukeshelastic about the trend column, I think this can stand on its own and could be merged at your convenience.

x-pack/plugins/infra/common/http_api/log_analysis/results/log_entry_categories.ts

weltenwort · 2021-01-15T11:08:08Z

...ra/public/pages/logs/log_entry_categories/sections/top_categories/top_categories_section.tsx

@@ -29,6 +30,8 @@ export const TopCategoriesSection: React.FunctionComponent<{
  sourceId: string;
  timeRange: TimeRange;
  topCategories: LogEntryCategory[];
+  sortOptions: SortOptions;
+  changeSortOptions: ChangeSortOptions;


total nit: Do we want to keep the on* naming schema for such functions?

Suggested change

changeSortOptions: ChangeSortOptions;

onChangeSortOptions: ChangeSortOptions;

weltenwort · 2021-01-15T11:17:43Z

x-pack/plugins/infra/server/lib/log_analysis/queries/top_log_entry_categories.ts

@@ -14,13 +14,33 @@ import {
  createDatasetsFilters,
 } from './common';

+import { CategorySort } from '../../../../common/http_api/log_analysis';


This import makes my slightly uncomfortable because it couples the query to the HTTP types, but I'm not sure it's worth dwelling on.

Do you think it would be worth it to duplicate that type here so they are technically independent but happen to line up without conversion (how convenient)?

Alternatively we could move that type to a location like common/log_analysis/log_entry_rate_analysis.ts so it's independent of the HTTP layer. 🤔

…entry_categories.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>

Kerry350 · 2021-01-15T16:46:42Z

@weltenwort I'm going to merge this without the further type / naming changes as I think they'll make more sense addressed alongside #79770. As part of that I've lifted a few (other) bits into common for a similar reason already.

changeSortOptions is also the naming used over on the anomalies page. So I'll just try to adjust everything in one place, as imports from http_api permeate a few places (the log_entry_anomalies query for example is also setup like this one, to import sort and pagination from http_api).

But my intention is to hoist them to common locations that aren't http_api and then have the HTTP types decorated with those common types.

weltenwort · 2021-01-15T17:27:11Z

Sounds good, thank you!

Kerry350 · 2021-01-18T11:09:55Z

@elasticmachine merge upstream

kibanamachine · 2021-01-18T13:27:18Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 92d14a9

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
`infra`	2.7MB	2.7MB	+1.3KB

History

💚 Build #99170 succeeded 911a9bf
💚 Build #98902 succeeded 69f1685
💚 Build #98870 succeeded 7801d8e
💚 Build #98795 succeeded df159f4
💚 Build #98302 succeeded 262eb11

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

* Add sorting capabilities to categories page Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>

mukeshelastic · 2021-01-18T15:50:00Z

@Kerry350 @weltenwort Thanks for looking into this. I am good with not making the trend column sortable?.

* Add sorting capabilities to categories page Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>

Add sorting capabilities to categories page

262eb11

Kerry350 added release_note:enhancement v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.12.0 labels Jan 12, 2021

Kerry350 added this to the Logs UI 7.12 milestone Jan 12, 2021

Kerry350 requested a review from a team January 12, 2021 17:00

Kerry350 self-assigned this Jan 12, 2021

weltenwort self-requested a review January 14, 2021 10:26

Merge branch 'master' into 58314-log-categories-sorting

df159f4

weltenwort reviewed Jan 14, 2021

View reviewed changes

Add server side sorting for maximum anomaly score and log entry count

7801d8e

TODO: Trend sorting

Revert changes relating to trend sorting

69f1685

weltenwort approved these changes Jan 15, 2021

View reviewed changes

Update x-pack/plugins/infra/common/http_api/log_analysis/results/log_…

911a9bf

…entry_categories.ts Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>

Merge branch 'master' into 58314-log-categories-sorting

92d14a9

Kerry350 merged commit 53f4b21 into elastic:master Jan 18, 2021

Kerry350 mentioned this pull request Jan 18, 2021

[7.x] [Logs UI] Add sorting capabilities to categories page (#88051) #88610

Merged

Kerry350 added a commit to Kerry350/kibana that referenced this pull request Jan 18, 2021

[Logs UI] Add sorting capabilities to categories page (elastic#88051)

e4b1156

* Add sorting capabilities to categories page Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>

Kerry350 added a commit that referenced this pull request Jan 18, 2021

[Logs UI] Add sorting capabilities to categories page (#88051) (#88610)

d3ee41d

* Add sorting capabilities to categories page Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com>

Kerry350 mentioned this pull request Jan 19, 2021

[Logs UI] Display category in anomalies table #88677

Merged

Kerry350 mentioned this pull request Feb 23, 2021

[Logs UI] [Test scenario] Categories page #92396

Closed

25 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Logs UI] Add sorting capabilities to categories page #88051

[Logs UI] Add sorting capabilities to categories page #88051

Kerry350 commented Jan 12, 2021

elasticmachine commented Jan 12, 2021

weltenwort commented Jan 14, 2021

weltenwort left a comment

Kerry350 commented Jan 14, 2021

weltenwort commented Jan 14, 2021

Kerry350 commented Jan 14, 2021

Kerry350 commented Jan 14, 2021 •

edited

Loading

weltenwort left a comment

weltenwort Jan 15, 2021

weltenwort Jan 15, 2021

Kerry350 commented Jan 15, 2021

weltenwort commented Jan 15, 2021

Kerry350 commented Jan 18, 2021

kibanamachine commented Jan 18, 2021

mukeshelastic commented Jan 18, 2021

	changeSortOptions: ChangeSortOptions;
	onChangeSortOptions: ChangeSortOptions;

[Logs UI] Add sorting capabilities to categories page #88051

[Logs UI] Add sorting capabilities to categories page #88051

Conversation

Kerry350 commented Jan 12, 2021

Summary

Note

Testing

elasticmachine commented Jan 12, 2021

weltenwort commented Jan 14, 2021

weltenwort left a comment

Choose a reason for hiding this comment

Kerry350 commented Jan 14, 2021

weltenwort commented Jan 14, 2021

Kerry350 commented Jan 14, 2021

Kerry350 commented Jan 14, 2021 • edited Loading

weltenwort left a comment

Choose a reason for hiding this comment

weltenwort Jan 15, 2021

Choose a reason for hiding this comment

weltenwort Jan 15, 2021

Choose a reason for hiding this comment

Kerry350 commented Jan 15, 2021

weltenwort commented Jan 15, 2021

Kerry350 commented Jan 18, 2021

kibanamachine commented Jan 18, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

mukeshelastic commented Jan 18, 2021

Kerry350 commented Jan 14, 2021 •

edited

Loading