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

[RAM] Add shareable rule status filter #130705

Merged
merged 14 commits into from
May 3, 2022

Conversation

JiaweiWu
Copy link
Contributor

@JiaweiWu JiaweiWu commented Apr 20, 2022

Summary

Resolves: #130160

Adds a shareable rule status filter, this is a new component that does not exist as a part of the rule list. Therefore, it is not being used, expect as a shareable component.

Also renames the existing rule_status_filter to rule_execution_status_filter.

Screenshot from 2022-05-03 15-56-17

@JiaweiWu JiaweiWu added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX v8.3.0 labels Apr 20, 2022
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiFilterButton, EuiPopover, EuiFilterGroup, EuiFilterSelectItem } from '@elastic/eui';

type State = 'enabled' | 'muted' | 'disabled';
Copy link
Contributor

Choose a reason for hiding this comment

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

@JiaweiWu I know this PR is still a draft, but I would like to give an input at this stage. It should be snoozed instead of muted

@@ -75,6 +77,7 @@ export interface TriggersAndActionsUIPublicPluginStart {
) => ReactElement<RuleEditProps>;
getAlertsTable: (props: AlertsTableProps) => ReactElement<AlertsTableProps>;
getRuleStatusDropdown: (props: RuleStatusDropdownProps) => ReactElement<RuleStatusDropdownProps>;
getRuleStateFilter: (props: RuleStateFilterProps) => ReactElement<RuleStateFilterProps>;
Copy link
Contributor

Choose a reason for hiding this comment

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

@JiaweiWu Do you think we should call it getRuleStatusFilter instead?

@mgiota
Copy link
Contributor

mgiota commented Apr 20, 2022

@JiaweiWu As we discussed for the status filter to work, we need some API changes as well, since we are currently using loadRules from triggers_actions_ui plugin https://github.com/elastic/kibana/blob/main/x-pack/plugins/observability/public/hooks/use_fetch_rules.ts#L10.

That being said I expect some changes in this function call https://github.com/elastic/kibana/blob/main/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/rules.ts#L40, where we need to pass the new statusFilter along with the rest filters that are currently being passed. This means that this file https://github.com/elastic/kibana/blob/main/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/map_filters_to_kql.ts needs to be changed as well.

@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream

@JiaweiWu
Copy link
Contributor Author

@mgiota I added changes to the loadRules so feel free to test it. As for renaming to status, I will have a discussion with the team because we already have a component called rule_status_filter.

@JiaweiWu
Copy link
Contributor Author

@elasticmachine merge upstream

@JiaweiWu JiaweiWu changed the title [RAM] Add shareable rule state filter [RAM] Add shareable rule status filter Apr 28, 2022
@JiaweiWu JiaweiWu marked this pull request as ready for review April 28, 2022 23:09
@JiaweiWu JiaweiWu requested a review from a team as a code owner April 28, 2022 23:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@kibana-ci
Copy link
Collaborator

kibana-ci commented Apr 28, 2022

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Default CI Group #6 / Discover alerting Search source Alert should navigate to discover via view in app link
  • [job] [logs] Default CI Group #6 / Discover alerting Search source Alert should navigate to discover via view in app link
  • [job] [logs] Jest Tests #2 / execution duration overview render last execution status
  • [job] [logs] Jest Tests #2 / execution duration overview renders average execution duration
  • [job] [logs] Jest Tests #2 / execution duration overview renders execution duration chart
  • [job] [logs] Jest Tests #2 / execution duration overview renders warning when average execution duration exceeds rule timeout
  • [job] [logs] Jest Tests #2 / rules render a hidden field with duration epoch
  • [job] [logs] Jest Tests #2 / rules render a list of rules
  • [job] [logs] Jest Tests #2 / rules render all active rules
  • [job] [logs] Jest Tests #2 / rules render all inactive rules
  • [job] [logs] Jest Tests #2 / tabbed content tabbed content renders when the event log experiment is on

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 370 373 +3

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
triggersActionsUi 310 313 +3

Async chunks

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

id before after diff
observability 428.0KB 428.0KB +9.0B
triggersActionsUi 724.4KB 727.5KB +3.2KB
total +3.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
triggersActionsUi 26 28 +2

Page load bundle

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

id before after diff
triggersActionsUi 99.7KB 100.5KB +783.0B
Unknown metric groups

API count

id before after diff
triggersActionsUi 324 327 +3

async chunk count

id before after diff
triggersActionsUi 79 80 +1

ESLint disabled in files

id before after diff
apm 15 14 -1
securitySolution 69 68 -1
synthetics 7 6 -1
total -3

ESLint disabled line counts

id before after diff
apm 92 89 -3
enterpriseSearch 9 7 -2
synthetics 53 47 -6
triggersActionsUi 170 171 +1
total -10

Total ESLint disabled count

id before after diff
apm 107 103 -4
enterpriseSearch 9 7 -2
securitySolution 511 510 -1
synthetics 60 53 -7
triggersActionsUi 176 177 +1
total -13

History

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

@mgiota
Copy link
Contributor

mgiota commented Apr 29, 2022

@JiaweiWu I just checked out your branch locally and Stack Management rules page is currently broken.
Screenshot 2022-04-29 at 10 59 24 Screenshot 2022-04-29 at 11 00 06

It is most probably related to the refactoring you did to the executionStatus.

I will try to use now the shared component in Observability and will give update you on this.

@mgiota
Copy link
Contributor

mgiota commented Apr 29, 2022

@JiaweiWu I confirm that getRuleStatusFilter works fine in Observability
Screenshot 2022-04-29 at 11 38 59

I just had to make one small change in triggers_actions_ui plugin to make it work. I will leave a comment inline to export RuleStatus

@@ -58,6 +58,7 @@ import {
RuleTableItem,
RuleType,
RuleTypeIndex,
RuleStatus,
Copy link
Contributor

@mgiota mgiota Apr 29, 2022

Choose a reason for hiding this comment

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

Could you export RuleStatus here https://github.com/elastic/kibana/blob/main/x-pack/plugins/triggers_actions_ui/public/index.ts#L15, so that we don't have to redefine the same type in observability?

@@ -163,6 +167,8 @@ export const RulesList: React.FunctionComponent = () => {
);
const [showErrors, setShowErrors] = useState(false);

const isRuleStatusFilterEnabled = getIsExperimentalFeatureEnabled('ruleStatusFilter');
Copy link
Contributor

Choose a reason for hiding this comment

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

@JiaweiWu Should we also check in observability if the experimental feature is enabled? In this case, could you export getIsExperimentalFeatureEnabled in this file, so that we can reuse it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh no, you shouldn't need to check the experiment in o11y.

I'm doing it here because this is a new filter for the rules list and we don't want to expose it to the user yet (until we get design feedback). So the experiment hides it for the triggers actions UI rules list.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I see, thanks for clarifying!

}): Promise<RuleAggregations> {
const filters = mapFiltersToKql({ typesFilter, actionTypesFilter, ruleStatusesFilter });
Copy link
Contributor

@mgiota mgiota Apr 29, 2022

Choose a reason for hiding this comment

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

@JiaweiWu Out of curiosity I was wondering why the new ruleStatusesFilter is not included here. Is it because the options of the rule status filter is a static hardcoded list (enabled, disabled, snoozed)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we want to filter the aggregate results as well? I suppose we do, I can add it

@mgiota mgiota self-requested a review April 29, 2022 18:12
Copy link
Contributor

@mgiota mgiota 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

@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! One comment about adding a test

})
).toEqual(['alert.attributes.executionStatus.status:(alert or statuses or filter)']);
});

test('should handle ruleStatusesFilter', () => {
expect(
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add a test for just disabled too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, good idea!

@JiaweiWu
Copy link
Contributor Author

JiaweiWu commented May 3, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #4 / Snapshots app Home page Repositories Tab cleanup repository

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 370 373 +3

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
triggersActionsUi 311 316 +5

Async chunks

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

id before after diff
observability 440.3KB 440.3KB +9.0B
triggersActionsUi 724.3KB 727.7KB +3.4KB
total +3.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
triggersActionsUi 26 27 +1

Page load bundle

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

id before after diff
triggersActionsUi 99.7KB 100.5KB +825.0B
Unknown metric groups

API count

id before after diff
triggersActionsUi 325 330 +5

async chunk count

id before after diff
triggersActionsUi 79 80 +1

ESLint disabled line counts

id before after diff
triggersActionsUi 170 171 +1

Total ESLint disabled count

id before after diff
triggersActionsUi 176 177 +1

History

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

@JiaweiWu JiaweiWu merged commit 07985db into elastic:main May 3, 2022
academo pushed a commit to academo/kibana that referenced this pull request May 4, 2022
* rule state filter

* turn off experiment

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Status filter API call

* Fix tests

* rename state to status, added tests

* Address comments and fix tests

* Revert experiment flag

* Remove unused translations

* Addressed comments

Co-authored-by: kibanamachine <[email protected]>
academo pushed a commit to academo/kibana that referenced this pull request May 4, 2022
* rule state filter

* turn off experiment

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Status filter API call

* Fix tests

* rename state to status, added tests

* Address comments and fix tests

* Revert experiment flag

* Remove unused translations

* Addressed comments

Co-authored-by: kibanamachine <[email protected]>
academo added a commit that referenced this pull request May 5, 2022
* Add severity field to create API and migration

* Adds integration test for severity field migration

* remove exclusive test

* Change severity levels

* Update integration tests for post case

* Add more integration tests

* Fix all cases list test

* Fix some server test

* Fix util server test

* Fix client util test

* Convert event log's duration from number to string in Kibana (keep as "long" in Elasticsearch) (#130819)

* Convert event.duration to string in TypeScript, keep as long in Elasticsearch

* Fix jest test

* Fix functional tests

* Add ecsStringOrNumber to event log schema

* Fix jest test

* Add utility functions to event log plugin

* Use new event log utility functions

* PR fixes

Co-authored-by: Kibana Machine <[email protected]>

* filter o11y rule aggregations (#131301)

* [Cloud Posture] Display and save rules per benchmark (#131412)

* Adding aria-label for discover data grid select document checkbox (#131277)

* Update API docs (#130999)

Co-authored-by: Kibana Machine <[email protected]>

* [CI] Use GCS buckets for bazel remote caching (#131345)

* [Actionable Observability] Add license modal to rules table (#131232)

* Add fix license link

* fix localization

* fix CI error

* fix more translation issues

Co-authored-by: Kibana Machine <[email protected]>

* [RAM] Add shareable rule status filter (#130705)

* rule state filter

* turn off experiment

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Status filter API call

* Fix tests

* rename state to status, added tests

* Address comments and fix tests

* Revert experiment flag

* Remove unused translations

* Addressed comments

Co-authored-by: kibanamachine <[email protected]>

* [storybook] Watch for changes in packages (#131467)

* [storybook] Watch for changes in packages

* Update default_config.ts

* Improve saved objects migrations failure errors and logs (#131359)

* [Unified observability] Add tour step to guided setup (#131149)

* [Lens] Improved interval input (#131372)

* [Vega] Adjust vega doc for usage of ems files (#130948)

* adjust vega doc

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Nick Peihl <[email protected]>

* Excess intersections

* Create severity user action

* Add severity to create_case user action

* Fix and add integration tests

* Minor improvements

Co-authored-by: Mike Côté <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: mgiota <[email protected]>
Co-authored-by: Jordan <[email protected]>
Co-authored-by: Bhavya RM <[email protected]>
Co-authored-by: Thomas Neirynck <[email protected]>
Co-authored-by: Brian Seeders <[email protected]>
Co-authored-by: Jiawei Wu <[email protected]>
Co-authored-by: Clint Andrew Hall <[email protected]>
Co-authored-by: Christiane (Tina) Heiligers <[email protected]>
Co-authored-by: Alejandro Fernández Gómez <[email protected]>
Co-authored-by: Joe Reuter <[email protected]>
Co-authored-by: Nick Peihl <[email protected]>
Co-authored-by: Christos Nasikas <[email protected]>
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
* rule state filter

* turn off experiment

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Status filter API call

* Fix tests

* rename state to status, added tests

* Address comments and fix tests

* Revert experiment flag

* Remove unused translations

* Addressed comments

Co-authored-by: kibanamachine <[email protected]>
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
* Add severity field to create API and migration

* Adds integration test for severity field migration

* remove exclusive test

* Change severity levels

* Update integration tests for post case

* Add more integration tests

* Fix all cases list test

* Fix some server test

* Fix util server test

* Fix client util test

* Convert event log's duration from number to string in Kibana (keep as "long" in Elasticsearch) (elastic#130819)

* Convert event.duration to string in TypeScript, keep as long in Elasticsearch

* Fix jest test

* Fix functional tests

* Add ecsStringOrNumber to event log schema

* Fix jest test

* Add utility functions to event log plugin

* Use new event log utility functions

* PR fixes

Co-authored-by: Kibana Machine <[email protected]>

* filter o11y rule aggregations (elastic#131301)

* [Cloud Posture] Display and save rules per benchmark (elastic#131412)

* Adding aria-label for discover data grid select document checkbox (elastic#131277)

* Update API docs (elastic#130999)

Co-authored-by: Kibana Machine <[email protected]>

* [CI] Use GCS buckets for bazel remote caching (elastic#131345)

* [Actionable Observability] Add license modal to rules table (elastic#131232)

* Add fix license link

* fix localization

* fix CI error

* fix more translation issues

Co-authored-by: Kibana Machine <[email protected]>

* [RAM] Add shareable rule status filter (elastic#130705)

* rule state filter

* turn off experiment

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Status filter API call

* Fix tests

* rename state to status, added tests

* Address comments and fix tests

* Revert experiment flag

* Remove unused translations

* Addressed comments

Co-authored-by: kibanamachine <[email protected]>

* [storybook] Watch for changes in packages (elastic#131467)

* [storybook] Watch for changes in packages

* Update default_config.ts

* Improve saved objects migrations failure errors and logs (elastic#131359)

* [Unified observability] Add tour step to guided setup (elastic#131149)

* [Lens] Improved interval input (elastic#131372)

* [Vega] Adjust vega doc for usage of ems files (elastic#130948)

* adjust vega doc

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

* Update docs/user/dashboard/vega-reference.asciidoc

Co-authored-by: Nick Peihl <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Nick Peihl <[email protected]>

* Excess intersections

* Create severity user action

* Add severity to create_case user action

* Fix and add integration tests

* Minor improvements

Co-authored-by: Mike Côté <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: mgiota <[email protected]>
Co-authored-by: Jordan <[email protected]>
Co-authored-by: Bhavya RM <[email protected]>
Co-authored-by: Thomas Neirynck <[email protected]>
Co-authored-by: Brian Seeders <[email protected]>
Co-authored-by: Jiawei Wu <[email protected]>
Co-authored-by: Clint Andrew Hall <[email protected]>
Co-authored-by: Christiane (Tina) Heiligers <[email protected]>
Co-authored-by: Alejandro Fernández Gómez <[email protected]>
Co-authored-by: Joe Reuter <[email protected]>
Co-authored-by: Nick Peihl <[email protected]>
Co-authored-by: Christos Nasikas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RAM] Add status filter to rules list and make it shareable
6 participants