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

[AO] - Add rule-specific fields, threshold, and reuse alert details package components #154569

Merged
merged 20 commits into from
Apr 13, 2023

Conversation

fkanout
Copy link
Contributor

@fkanout fkanout commented Apr 6, 2023

Summary

It fixes #153675, fixes #153567, fixes #153946, fixes #154845

Screenshot 2023-04-11 at 17 40 38

@fkanout fkanout added Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.8.0 labels Apr 6, 2023
@fkanout fkanout self-assigned this Apr 6, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@fkanout fkanout changed the title [AO] - Add rule-specific fields to alert summary for the Logs Alert Details page [AO] - Add rule-specific fields, and threshold Apr 11, 2023
@fkanout fkanout added the release_note:skip Skip the PR/issue when compiling release notes label Apr 11, 2023
@fkanout fkanout marked this pull request as ready for review April 12, 2023 16:24
@fkanout fkanout requested a review from a team as a code owner April 12, 2023 16:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

@fkanout fkanout changed the title [AO] - Add rule-specific fields, and threshold [AO] - Add rule-specific fields, threshold, and optim Apr 12, 2023
@fkanout fkanout changed the title [AO] - Add rule-specific fields, threshold, and optim [AO] - Add rule-specific fields, threshold, and reuse alert details package components Apr 12, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically, I applied what was suggested here #153930 (comment)
And in another PR will move it to the Observability

Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

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

LGTM, I left some non-blocking nits, thanks for the changes on the custom hook!

@fkanout fkanout enabled auto-merge (squash) April 13, 2023 11:52
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #4 / cases security and spaces enabled: basic Common bulk_create_attachments creation should bulk create multiple attachments
  • [job] [logs] FTR Configs #23 / cases security and spaces enabled: trial Common bulk_create_attachments alert filtering should create a new attachment without alerts attached to the case on the same request
  • [job] [logs] FTR Configs #23 / cases security and spaces enabled: trial Common bulk_create_attachments creation should bulk create multiple attachments

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1328 1327 -1

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.0MB 2.0MB +3.0KB

Page load bundle

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

id before after diff
infra 89.7KB 90.0KB +277.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 512 515 +3

History

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

cc @fkanout

@fkanout fkanout merged commit 89e8d03 into elastic:main Apr 13, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 13, 2023
<EuiSpacer size="s" />
{chartCriterion.comparator && (
<Threshold
title={`Threshold breached`}
Copy link
Member

Choose a reason for hiding this comment

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

@fkanout Shouldn't we translate this text?

@@ -19,7 +19,7 @@ export interface ChartProps {

export interface Props {
chartProps: ChartProps;
comparator: Comparator;
comparator: Comparator | string;
Copy link
Member

Choose a reason for hiding this comment

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

@fkanout Why do we need to add string type here?

{chartCriterion.comparator && (
<Threshold
title={`Threshold breached`}
chartProps={{ theme, baseTheme: LIGHT_THEME }}
Copy link
Member

@maryam-saeidi maryam-saeidi Apr 20, 2023

Choose a reason for hiding this comment

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

How does it work with only LIGHT_THEME as a base theme?

Previously, I provided it like this: https://github.com/elastic/kibana/blob/main/x-pack/plugins/observability/public/pages/rule_details/index.tsx#L97

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 release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.8.0
Projects
None yet
7 participants