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

[Alerting] Showing last execution duration on Rule Management view #113935

Merged
merged 32 commits into from
Oct 12, 2021

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Oct 5, 2021

Relates to #111805

This addresses the rule management view portion of the issue. Will have separate PR for rule details view when those mocks become available.

Summary

Added last execution duration as a field inside the executionStatus of the rule saved object.
Adding last execution duration as a column on the Rules Management view. Also tweaked the table to reflect mockups.

UI Changes:

  • moved rule type as a second line the rule name column instead of its own column
  • changed tags list into a button showing # of tags with popover
  • added "Last run" column
  • added "Duration" column
  • removed # Actions column

Screen Shot 2021-10-06 at 8 30 23 AM

Tooltip describing duration column
Screen Shot 2021-10-06 at 8 30 34 AM

Warning indicator when rule duration exceeds the configured rule timeout value
Screen Shot 2021-10-06 at 8 30 40 AM

Tags popover
Screen Shot 2021-10-06 at 8 30 50 AM

Checklist

@ymao1 ymao1 changed the title [Alerting] Showing last execution duration on Rule Management view [Alerting][skip-ci] Showing last execution duration on Rule Management view Oct 5, 2021
@ymao1 ymao1 self-assigned this Oct 6, 2021
@ymao1 ymao1 added Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) UX v7.16.0 v8.0.0 labels Oct 6, 2021
@@ -99,10 +99,10 @@ export function getHealthColor(status: AlertExecutionStatuses) {
case 'error':
return 'danger';
case 'ok':
return 'subdued';
return 'primary';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed these colors to reflect mockup.

@ymao1 ymao1 marked this pull request as ready for review October 6, 2021 14:09
@ymao1 ymao1 requested a review from a team as a code owner October 6, 2021 14:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@ymao1 ymao1 requested review from mdefazio and gchaps October 6, 2021 14:10
@ymao1
Copy link
Contributor Author

ymao1 commented Oct 6, 2021

Tagging @gchaps to review copy on tooltips (can be seen in screenshots in the PR summary)

@ymao1
Copy link
Contributor Author

ymao1 commented Oct 7, 2021

Suggestions for tooltip #1:

  • The time it took for the rule to run to completion.
  • The length of time it took for the rule to run.

Tooltip #2:

  • Duration exceeds the rule's expected run time.

@gchaps Thanks! Updated in this commit: cccf8d1

@brianseeders
Copy link
Contributor

Hey @ymao1, just noticed you were running in buildkite, then went back to Jenkins. Was that just because of the test failures?

@ymao1
Copy link
Contributor Author

ymao1 commented Oct 7, 2021

@brianseeders i was trying to see if one was faster than the other :) but I actually forgot to check the total runtime

@brianseeders
Copy link
Contributor

Ah, okay. They should be about the same at the moment. I have a PR open though that reduces the overall time by 40 minutes, but we have to wait until we've fully moved over to merge it

@ymao1
Copy link
Contributor Author

ymao1 commented Oct 11, 2021

@elasticmachine merge upstream

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tooltip text looks good to me

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ymao1
Copy link
Contributor Author

ymao1 commented Oct 11, 2021

@elasticmachine merge upstream

@ymao1 ymao1 added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 11, 2021
@ymao1 ymao1 enabled auto-merge (squash) October 11, 2021 23:04
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
alerting 245 249 +4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
triggersActionsUi 760.3KB 762.7KB +2.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
alerting 36.5KB 36.9KB +369.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
alert 34 35 +1
Unknown metric groups

API count

id before after diff
alerting 253 257 +4

History

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

cc @ymao1

@ymao1 ymao1 merged commit c926b14 into elastic:master Oct 12, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Oct 12, 2021
…lastic#113935)

* Adding last duration to execution status and returning in alerting routes

* Fixing types

* Adding helper function to format duration

* Returning rule timeout value in list rules API

* Updating rules table to add duration column and tweaks to match mockup

* Updating rules table to add duration column and tweaks to match mockup

* i18n fix

* Only showing duration warning if duration is long

* Unit tests

* i18n fix

* Fixing functional test

* Aligning warning icon

* Reset last duration when rule is disabled then reenabled

* Fixing functional test

* Fixing functional test

* Restoring muted badge. Fixing scss

* Dont show muted badge if rule is disabled

* Moving disabled icontip to right of rule name

* Updating tooltips

* Updating last run

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 12, 2021
…113935) (#114584)

* Adding last duration to execution status and returning in alerting routes

* Fixing types

* Adding helper function to format duration

* Returning rule timeout value in list rules API

* Updating rules table to add duration column and tweaks to match mockup

* Updating rules table to add duration column and tweaks to match mockup

* i18n fix

* Only showing duration warning if duration is long

* Unit tests

* i18n fix

* Fixing functional test

* Aligning warning icon

* Reset last duration when rule is disabled then reenabled

* Fixing functional test

* Fixing functional test

* Restoring muted badge. Fixing scss

* Dont show muted badge if rule is disabled

* Moving disabled icontip to right of rule name

* Updating tooltips

* Updating last run

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: ymao1 <ying.mao@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) UX v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants