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

[Security Solution] Trend Bar Graph Incorrect Sorting #159867

Closed
MakoWish opened this issue Jun 16, 2023 · 12 comments
Closed

[Security Solution] Trend Bar Graph Incorrect Sorting #159867

MakoWish opened this issue Jun 16, 2023 · 12 comments
Labels
enhancement New value added to drive a business result Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@MakoWish
Copy link

Kibana version: 8.8.1

Elasticsearch version: 8.8.1

Server OS version: RHEL 8

Browser version: Edge Chromium 113.0.1774.35

Browser OS version: Windows 10

Original install method (e.g. download page, yum, from source, etc.): Yum

Describe the bug: Security Alerts bar graph on Trend tab is sorted ascending instead of descending as it has always previously been. This is counterintuitive.

Steps to reproduce:

  1. Enable several Detection Rules, and trigger each a different number of times
  2. View the Security --> Alerts page
  3. Select the "Trend" tab
  4. View the sort order of the graph's index

Expected behavior: This graph's index has always previously been sorted descending and should remain that way.

Screenshots (if relevant):
Although the text and count are truncated, the graph's index is shown ascending by number of hits:
security_alerts_trend_graph

If you open this graph in Lens, you can see this more clearly. The rank direction is already Descending, but the values are actually shown ascending:
descending_selected_but_shown_ascending

If I change the sort order to Ascending, the values are then shown descending:
ascending_selected_but_shown_descending

@MakoWish MakoWish added the bug Fixes for quality problems that affect the customer experience label Jun 16, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 16, 2023
@mbondyra mbondyra added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Jun 19, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Jun 19, 2023
@markov00 markov00 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Jun 19, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

@stratoula stratoula added enhancement New value added to drive a business result and removed bug Fixes for quality problems that affect the customer experience labels Jun 20, 2023
@stratoula
Copy link
Contributor

I am changing this to enhancement as this is an expected behavior. It seems to me that this can be imrpved when we implement this #86184

@MakoWish
Copy link
Author

MakoWish commented Jun 20, 2023

Hi @stratoula,

I respectfully disagree and stand firm that this is in fact a bug. The sort order is backwards, as can be seen in my screenshots, and it was never like this before we upgraded to 8.8.1. The definition of descending says the order should be from highest on the top, to lowest on the bottom, or for alpha, it would be "Z" on the top to "A" on the bottom. As you can see in my screenshots, that is not what is happening. In my 40+ years in computing, this is the first time I have seen descending put the lowest number on top. It is great to see you are working on a feature to allow changing the sort order, but that does not fix the issue of the sorting being backward.

Ascending order means the smallest or first or earliest in the order will appear at the top of the list:
For numbers or amounts, the sort is smallest to largest. Lower numbers or amounts will be at the top of the list.
For letters/words, the sort is alphabetical from A to Z.
For data with numbers and letters/words, such as address lines, the sort is most likely alphanumeric meaning 0-9 is sorted first then followed by A-Z.
For dates, the sort will be oldest/earliest dates to most recent. The oldest dates will be at the top of the list.
 
Descending order means the largest or last in the order will appear at the top of the list:
For numbers or amounts, the sort is largest to smallest. Higher numbers or amounts will be at the top of the list.
For letters/words, the sort is alphabetical from Z to A.
For data with numbers and letters/words, such as address lines, the sort is most likely alphanumeric meaning Z to A is sorted first then followed by 9-0.
For dates, the sort will be most recent dates to the oldest/earliest dates. The most recent/latest dates will be at the top of the list.

@stratoula
Copy link
Contributor

stratoula commented Jun 20, 2023

I changed this to enhancement as this is how our chart library works. I understand that this is confusing but the idea is that the highest value is rendered first touching the baseline and everything else in order stacked on top of it.

The legend follows the same visual order. I am not aware how this specific graph was implemented before tbh. @elastic/security-solution maybe you can give us some insights here.

So from the visualizations library perspective and only, this is how the stacked bars work. So if we want to change this, it is an enhancement for us.

cc @markov00

@dej611
Copy link
Contributor

dej611 commented Jun 20, 2023

The ranking parameter is telling ES how the values should be picked and returned, that's it.
The order of the legend purely based on the order of the data from ES, the visualization library does not perform any ordering task on top of it.

The fact that the legend number is ascending is a pure coincidence as the legend is rendering by default the last bucket value when no hovering happens, and in that bucket there are "ordered" values. But it is purely a coincidence - will render descending values if the last bucket returned so.

@stratoula proposed to change this to enhancement in order to provide more control at visualization level over that, linking the general plan we have for client-side sorting.

@stratoula
Copy link
Contributor

I should be more specific about it, I understand the confusion. This is the documentation about ES order https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-terms-aggregation.html#search-aggregations-bucket-terms-aggregation-order. In Lens we are using the word ranking to avoid confusion but it might not be super clear even with that.

@MakoWish
Copy link
Author

I am sorry, but the logic or reason behind why it is sorting this way is irrelevant. The fact it is sorting backward is the issue. We have been using Elastic for more than four years now, and it has never sorted backward like this, so something changed. The highest value was always on top in every previous version we have used. If something is going to be sorted by ascending or descending at all, it should actually be ascending or descending. As shown in my screenshots, the sort order is now backwards from the rest of the world's definition of ascending and descending.

@angorayc
Copy link
Contributor

Looks similar to this issue I wrote #154533

@markov00
Copy link
Member

Hi @MakoWish thanks for reaching out, I've discussed this with the visualization team internally and we decide to fix this behavior in our charts.
You are right, users read charts and legends in different reading directions (a list like a legend, from top to bottom, a stacked bar chart from the bottom bar aligned with the baseline to the one on top, etc) and we should not change this reading direction in favor of a visual alignment of the stacked colors.
This was the principle used in our charts for years now: what you see at the bottom of the chart is aligned with what you see at the bottom of the legend and you can follow the stacked colors visually in the same direction. That is the opposite of following the reading direction of the chart and legend.
This principle causes noticeable reading difficulties, especially in your case, where the legend list is longer than the available vertical space, causing the topmost (and most important) items to be hidden from the screen.

As said, we discussed that internally and we are going to release a fix that sorts the legend by the correct reading direction.

Even if this behavior never changed in our charting library for years now, the Security page replaced their visualization with Lens visualization (introducing this behavior). their previous custom implementation before that change (~4/5 months ago) was using a custom legend with the expected legend sorting order.

I have taken a look at the code and, except for manipulating a hardcoded flag in the code you can't switch back to the previous implementation. The @elastic/security-solution team is aware of that, but probably the solution here is to wait for the sorting fix directly in the charting library.
I will keep you updated on this.

@MakoWish
Copy link
Author

... the Security page replaced their visualization with Lens visualization (introducing this behavior). their previous custom implementation before that change (~4/5 months ago) was using a custom legend with the expected legend sorting order.

That would explain why we just saw this change. We upgraded from 8.6.2 to 8.8.1, and I believe 8.6.2 is quite older than ~4/5 months.

Thank you for letting me know this will be changed. I agree it makes sense on the actual bar graph to have the larger value/bar on the bottom, with the smaller bars stacked on top, but the legend should definitely have the larger value on top.

@stratoula
Copy link
Contributor

I am closing this as this was addressed in 8.9 #160883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests

7 participants