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

[Infra UI] Add alerts to asset details flyout #160371

Closed
jennypavlova opened this issue Jun 23, 2023 · 12 comments · Fixed by #161677
Closed

[Infra UI] Add alerts to asset details flyout #160371

jennypavlova opened this issue Jun 23, 2023 · 12 comments · Fixed by #161677
Assignees
Labels
Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.10.0

Comments

@jennypavlova
Copy link
Member

jennypavlova commented Jun 23, 2023

Summary

The asset details flyout should include alerts. I should show the alerts widget with the number of alerts based on their type. There should be two actions available - navigate to see all alerts and create a rule.

Rules flyout

We will use the inventory rule flyout - same screen as the inventory button and Infrastructure -> Create Inventory Rule

Image

Difference with inventory in inventory we prefill the metric in WHEN condition based on the selection
image

On the hosts view, we don't want to prefill the metric (we should keep the filter):
image

Note: It should be flexible on what kind of WHEN condition metric we want to pass as a consumer of the flyout

Design

MVP version
Image

AC

  • Alerts should be visible inside the asset details flyout
  • Show all button should navigate to alerts
  • Create rule button should open the flyout for creating a rule
@jennypavlova jennypavlova added Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability labels Jun 23, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 23, 2023
@jennypavlova jennypavlova added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Jun 23, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Jun 23, 2023
@jennypavlova jennypavlova self-assigned this Jul 3, 2023
@jennypavlova
Copy link
Member Author

@roshan-elastic and @kkurstak I was investigating the alerts summary component provided and it includes the alerts counts we want and a chart. They provide 2 options - a full-size one we use on the host view and a compact one.

  • Fullsize
Screenshot 2023-07-04 at 13 39 35 - Compact Screenshot 2023-07-04 at 13 38 23 I don't see an option provided to disable the chart but the compact version is not taking much space even with the line chart (maybe the border is not something that matches our design) - I tried to adapt it a bit with custom styles and that's the best version I got (still alerts count + line chart) image If we don't want that chart at all maybe we can talk to the alerts team to adjust the component together to match our design needs (or if they can just provide the alerts count separately as a widget - I couldn't find something like that) I will still think about a better solution there 🤔

@roshan-elastic
Copy link

Hey @jennypavlova - thanks for the call out.

Can we try the compact one as a fallback but we can ask if they have plans for a version without a chart?

Question - is there a version with a table of the 'active' alerts we could use (sorted by duration)?

image

Not saying we want to use this but just understanding the limitations we have...

@jennypavlova
Copy link
Member Author

Hey @roshan-elastic, thanks for the feedback!

Question - is there a version with a table of the 'active' alerts we could use (sorted by duration)?

So that's the default look with only active (as example):
Screenshot 2023-07-05 at 14 11 20
Screenshot 2023-07-05 at 14 10 30

I checked what options they provide so we can filter by status and there is an option to give a config where we can customize the sort order and the columns (haven't tried it directly but I saw that security team used some of the custom config options)

@jennypavlova
Copy link
Member Author

Can we try the compact one as a fallback but we can ask if they have plans for a version without a chart?

I pinged the @elastic/response-ops team, let's see 🙂
So TLDR: Do you plan to have the alerts summary widget with an option to hide the chart ( showing only the alerts count)?

@roshan-elastic
Copy link

Hey @jennypavlova - thanks for that.

I'm thinking that we see if we can configure the alerts widget to:

  • remove the chart (if possible)
  • show the alerts like the designs (only showing the 'alert status', 'duration' and 'reason column')
  • limit that to perhaps 3-5 rows...

Maybe we can see what it looks like together if it's quick to play around with it?

I think we should keep it pretty minimal in the fly-out but in the detail view it can have more options...

@jennypavlova
Copy link
Member Author

Hi @roshan-elastic I opened a PR in the alerts widget to allow us to use only the count like the full size view but without the charts ( it's still waiting for a code review from response ops team) and after I finish the changes and merge the overview tab PR we can be on a good spot to try and see how it looks.

The flyout design doesn't have the table do we still want to make the changes for the full page view in this issue or we want to first do the flyout redesign and then adapt the full page view separately (in a new issue where we will add all needed things once the flyout is ready)?

I can still try how we can customise the table in the meantime so we will know how much we can adapt there - I am looking at their storybook and I saw that they support pagination (and columns customisation) which can be useful to show only 3-5 rows per page initially and then to have the option to still see the other alerts using the pages without the need to completely hide them (something we can try):
Screenshot 2023-07-07 at 12 29 23

@roshan-elastic
Copy link

Hey @jennypavlova nice one.

Is it easy to try showing the first 3 issues in the fly-out design? I know the design doesn't show it right now but I think if it's minimal - it could be useful.

If it's easy to try, perhaps we could have a look?

@jennypavlova
Copy link
Member Author

@roshan-elastic Thanks for explaining that! So the thing with the configuration of the alerts table is that the columns are configured in observability and we reuse their configuration so the part with changing the columns/order will be definitely more effort, regarding the limitation of the entries in the table (without pagination) and filtering by active status we can do that easily, I drafted something (still without the overview / hiding charts changes as mentioned they will be done after the other PRs )

// Here I have only 3 active
Image
// I made one without the filter to show the limitation (so there will be only 3 shown)
Image

I am still wondering If the table is really useful in a minimal setup where we have "show all" button where the user can see the "full" table 🤔 also it can be confusing for the user to only see 3 alerts when the total can be 10 for example?

@roshan-elastic
Copy link

Hey @jennypavlova , thanks for that!

Yeah, let's not customise the column order then - seems like a waste.

I think given the table's current state, it'll be hard to get something nice for the fly-out so let's go for the current design (and we can think about the table for the detail view when we get there).

OK?

@jennypavlova
Copy link
Member Author

Hey @roshan-elastic Thanks for the feedback, sounds good!

jennypavlova added a commit that referenced this issue Jul 10, 2023
…et (#161263)

Relates to #160371
## Summary

This PR adds an option to hide the chart when using the
`AlertSummaryWidgetFullSize` component - when the prop
`shouldHideCharts` is set to `true` the charts won't be visible

## Storybook 
- Added `Full Size Without Chart`
<img width="1804" alt="image"
src="https://github.com/elastic/kibana/assets/14139027/e0ee3360-2212-4c0c-943c-b1b9ece66a68">

## Testing

Set `shouldHideCharts` to `true` when using the alert summary component
and the charts should not be visible. If it's not set the chart should
be visible.
jennypavlova added a commit that referenced this issue Jul 18, 2023
Closes #160371 

## Summary

This PR adds alerts section to the overview tab inside the asset details
flyout component.

Notes: A lot of changes are extracting common components from the alerts
tab to a common folder. The flyout version is not showing the chart so
it's not exactly the same component but a big part of the logic is
reused there. The tooltip content can be found in a [Figma comment
](https://www.figma.com/file/XBVpHX6pOBaTPoGHWhEQJH?node-id=843:435665&mode=design#492130894)


<img width="1616" alt="alerts_section"
src="https://github.com/elastic/kibana/assets/14139027/399dd1ea-e1cb-4e7f-9ed5-917ced7cc490">

## Alerts summary widget changes:
After introducing the `hideChart` prop
[here](#161263) in this PR I
change the spinner type and size in case of no chart we want to have a
smaller section with a smaller spinner:


![image](https://github.com/elastic/kibana/assets/14139027/43a3c611-0404-4c21-a503-22f1a79dc1de)



![image](https://github.com/elastic/kibana/assets/14139027/a870fa9b-5367-4303-9b7d-4da9ff2eae2b)


##  Storybook
I added some changes to make the alerts widget show in the storybook
[[Workaround for
storybook](d97a2b1)]

<img width="1905" alt="image"
src="https://github.com/elastic/kibana/assets/14139027/539c9443-f977-4301-8d2b-d24f1d01b44e">
 
## Testing
- Go to Hosts view and open the single host flyout - alerts section
should be visible
- Alerts title icon should open a tooltip with links to alerts and
alerts documentation
- Alerts links:
- The Create rule link will open a flyout (on top, not closing the
existing flyout) to create an inventory rule, when closed/saved rule the
single host flyout should remain open
- The Show All link should navigate to alerts and apply time range /
host.name filter selected in the hosts view


https://github.com/elastic/kibana/assets/14139027/b362042a-b9de-460c-86ae-282154b586ff

---------

Co-authored-by: kibanamachine <[email protected]>
@roshan-elastic
Copy link

Wow this looks great @jennypavlova @kkurstak - thanks for figuring this out!

Link
Image

@katrin-freihofner - FYI

ThomThomson pushed a commit to ThomThomson/kibana that referenced this issue Aug 1, 2023
Closes elastic#160371 

## Summary

This PR adds alerts section to the overview tab inside the asset details
flyout component.

Notes: A lot of changes are extracting common components from the alerts
tab to a common folder. The flyout version is not showing the chart so
it's not exactly the same component but a big part of the logic is
reused there. The tooltip content can be found in a [Figma comment
](https://www.figma.com/file/XBVpHX6pOBaTPoGHWhEQJH?node-id=843:435665&mode=design#492130894)


<img width="1616" alt="alerts_section"
src="https://github.com/elastic/kibana/assets/14139027/399dd1ea-e1cb-4e7f-9ed5-917ced7cc490">

## Alerts summary widget changes:
After introducing the `hideChart` prop
[here](elastic#161263) in this PR I
change the spinner type and size in case of no chart we want to have a
smaller section with a smaller spinner:


![image](https://github.com/elastic/kibana/assets/14139027/43a3c611-0404-4c21-a503-22f1a79dc1de)



![image](https://github.com/elastic/kibana/assets/14139027/a870fa9b-5367-4303-9b7d-4da9ff2eae2b)


##  Storybook
I added some changes to make the alerts widget show in the storybook
[[Workaround for
storybook](elastic@d97a2b1)]

<img width="1905" alt="image"
src="https://github.com/elastic/kibana/assets/14139027/539c9443-f977-4301-8d2b-d24f1d01b44e">
 
## Testing
- Go to Hosts view and open the single host flyout - alerts section
should be visible
- Alerts title icon should open a tooltip with links to alerts and
alerts documentation
- Alerts links:
- The Create rule link will open a flyout (on top, not closing the
existing flyout) to create an inventory rule, when closed/saved rule the
single host flyout should remain open
- The Show All link should navigate to alerts and apply time range /
host.name filter selected in the hosts view


https://github.com/elastic/kibana/assets/14139027/b362042a-b9de-460c-86ae-282154b586ff

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.10.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants