Displaying alert data in alert summary in alert details page #140339

benakansara · 2022-09-08T19:52:21Z

Summary

Implements #139269

Querying alert data with http API using existing hook under x-pack/plugins/observability/public/pages/cases.

I have moved following files from x-pack/plugins/observability/public/pages/cases to x-pack/plugins/observability/public/hooks in order to reuse the existing hook.

use_fetch_alert_detail.ts
use_fetch_alert_detail.test.ts
use_fetch_alert_data.ts
use_fetch_alert_data.test.ts
use_data_fetcher.ts

The alert details page can be opened from the Alerts table "..." -> View alert details and from alerts flyout Alert details button. The appearance of the menu item/button depends on the feature flag xpack.observability.unsafe.alertDetails.enabled.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…-ref HEAD~1..HEAD --fix'

…-fix'

…/alert-summary-data

…ansara/kibana into feature/alert-summary-data

…/alert-summary-data

…-fix'

…ansara/kibana into feature/alert-summary-data

…/alert-summary-data

benakansara · 2022-09-20T16:27:12Z

@elasticmachine merge upstream

…ansara/kibana into feature/alert-summary-data

benakansara · 2022-09-21T13:31:12Z

Nice Job! 👏🏻

Tested locally for an active and recovered alert. Is this what we expect to see? (some of the fields are not present)

I approve (assuming it is OK that some of the fields are not present) but I've added some minor comments :)

Yes, actual value and expected value fields are empty for some rule types. Basically we are showing same values as shown in alert flyout at the moment. Source is empty currently because this will come from context variables which are not available right now.

kdelemme

just one or two nits but otherwise good job 👏🏻 !

kdelemme · 2022-09-21T13:34:02Z

x-pack/plugins/observability/public/pages/alert_details/mock/index.ts

+
+export const tags: string[] = ['tag1', 'tag2', 'tag3'];
+
+export const alert: TopAlert | null = {


nit: the type is not null

Suggested change

export const alert: TopAlert | null = {

export const alert: TopAlert = {

kdelemme · 2022-09-21T13:35:58Z

x-pack/plugins/observability/public/pages/alert_details/mock/index.ts

+  lastUpdated: 1630588131750,
+};
+
+export const alertWithTags: TopAlert | null = {


nit: Same as above regarding the type. And also, we could reuse the base alert defined above and only override the 'kibana.alert.rule.tags': tags to make it clearer what is changing between the two Alerts.

Suggested change

export const alertWithTags: TopAlert | null = {

export const alertWithTags: TopAlert = {

Good idea, I have modified the alert data.

…ansara/kibana into feature/alert-summary-data

…-fix'

fkanout · 2022-09-21T14:45:20Z

x-pack/plugins/observability/public/config/paths.ts

-      alertId && ruleId
-        ? `${ALERT_PAGE_LINK}/rules/${encodeURI(ruleId)}/alerts/${encodeURI(alertId)}`
-        : ALERT_PAGE_LINK,
+    alertDetails: (alertId?: string | null) =>


@benakansara, To create the alert details page, we need the ruleId. Also, we need to make these arguments NOT optional.

After investigating further, I found that there is an http API and also that it is the recommended approach to get alert data. This API doesn't require ruleId. I will make alertId not optional.

@fkanout BTW, I was thinking about the parameters. Actually they are not optional. You would still need to pass arguments while calling the function, but it could accept undefined or null. With optional arguments, you could call the function without passing anything at all.

That's great you found the API that gives us all we need about an alert without ruleId. 👏🏻
Regarding the function arguments, I don't see a use case where we call this function without the alertId. For me,
the function signature should be alertDetails: (alertId: string) =>. Am I missing something here?

No, you are right, I have already changed it to alertDetails: (alertId: string) =>.

fkanout · 2022-09-21T15:55:58Z

x-pack/plugins/observability/public/pages/alert_details/components/alert_summary.tsx

+          </EuiTitle>
+          <EuiSpacer size="s" />
+          <EuiText size="s" color="subdued">
+            {alert?.fields['kibana.alert.evaluation.value'] ?? '-'}


Instead of writing hardcode the fields' names which could be changed or mistakenly typed. You can use ParsedTechnicalFields

Do you mean to use this file packages/kbn-rule-data-utils/src/technical_field_names.ts instead of hardcoding field names? If not, can you please elaborate on how to use ParsedTechnicalFields?

I have removed hardcoded field names with fields from @kbn/rule-data-utils.

Awesome 💯 👍🏻 !!

fkanout · 2022-09-21T15:57:25Z

x-pack/plugins/observability/public/pages/alert_details/constants.ts

+ * 2.0.
+ */
+
+export const DEFAULT_DATE_FORMAT = 'MMM D, YYYY @ HH:mm:ss.SSS';


Why do we need to define this? I think the date format is config that is shared across Kibana

I saw it is defined at many places. When I tried to import from one of the plugins, it was giving error as it was not exposed to use outside of that plugin. I would then have to modify index file of this plugin and export the constant. That's why I created this file. Could you please point me to config which I can use?

I don't remember where I saw it, maybe as a helper formatter. But If you don't find it or it will take more time, keep it like this and can refactor it later one

sure, I will keep an eye on it. If I find helper formatter or config in future, I will update it.

.../plugins/observability/public/pages/alerts/components/alerts_flyout/alerts_flyout_footer.tsx

…ansara/kibana into feature/alert-summary-data

…-fix'

…ansara/kibana into feature/alert-summary-data

…-fix'

…ansara/kibana into feature/alert-summary-data

…-ref HEAD~1..HEAD --fix'

fkanout · 2022-09-23T13:52:21Z

x-pack/plugins/observability/public/pages/alert_details/index.tsx

@@ -1,47 +0,0 @@
-/*


@benakansara, I think this was the right way to organize the code. I mean, putting the main component in the index.tsx, and then pulling the components that this page is composed of.
I don't think putting the main page/container in the component folder is the correct convention.

@fkanout I refactored the code to be consistent with other refactoring efforts suggested by @maryam-saeidi - #139606 (comment). From my perspective, putting main component in index.tsx feels right way too, but I also think it is better to be consistent as much as possible.

fkanout

Tested locally and it works as expected. Awesome job, @benakansara!!
I left you some comments, but they could be handled with a follow up PR

Regarding the empty fields/not provided, we need to align the fields. e.g Rule tags don't
have - like the Actual value

benakansara · 2022-09-23T14:32:39Z

Tested locally and it works as expected. Awesome job, @benakansara!! I left you some comments, but they could be handled with a follow up PR

Regarding the empty fields/not provided, we need to align the fields. e.g Rule tags don't have - like the Actual value

I will work on the rule tags in new PR. The design of the tags is different than on rule details page, and I tried to put - for empty tags, however, it was not aligning properly. So I am going to work on both in separate PR. Thank you for the feedback.

kibana-ci · 2022-09-23T15:44:12Z

💚 Build Succeeded

Buildkite Build
Commit: 5a3891e

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`observability`	477	479	+2

Async chunks

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

id	before	after	diff
`observability`	525.4KB	525.5KB	+149.0B

History

💚 Build #74811 succeeded 6dd1d28
💔 Build #74777 failed e222443
💔 Build #74764 failed 96a9b07
💚 Build #74542 succeeded f66bb4f
💚 Build #74463 succeeded 0282605
💚 Build #74343 succeeded 47c19d5

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

benakansara and others added 9 commits September 8, 2022 20:36

displaying data in alert summary in alert details page

aa7ce77

refactoring index to include only exports

c93c14c

fixing path error

06b8106

removing tooltip as not required

5007d59

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

bf54fee

…-ref HEAD~1..HEAD --fix'

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

554bcbe

…-fix'

fixing failing tests

cc88757

Merge branch 'main' of https://github.com/elastic/kibana into feature…

8b78c3e

…/alert-summary-data

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

e4301b8

…ansara/kibana into feature/alert-summary-data

benakansara mentioned this pull request Sep 12, 2022

[Actionable Observability] - Create the header and the actions for the Alert Details page #140030

Closed

Merge branch 'main' of https://github.com/elastic/kibana into feature…

30d99c7

…/alert-summary-data

benakansara changed the title ~~Displaying alert data in alert summary in alert details page~~ Querying alert data for alert summary Sep 13, 2022

benakansara mentioned this pull request Sep 13, 2022

[Actionable Observability] Create the Alert Summary component for the Alert Landing page #139269

Closed

3 tasks

benakansara changed the title ~~Querying alert data for alert summary~~ Querying alert data for alert summary component Sep 13, 2022

benakansara and others added 12 commits September 20, 2022 13:46

Added unit tests for alert summary, refactoring

b402b3e

fix: hook was called conditionally

e2f14e2

fixed path errors

bd2ff11

removing dependency on ruleId as not required

3080d75

removing dependency on ruleId as not required

4aabb7b

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

1bc1194

…-fix'

Merge branch 'main' into feature/alert-summary-data

eabeab9

fixing route

4d5ff00

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

d89c193

…ansara/kibana into feature/alert-summary-data

Merge branch 'main' of https://github.com/elastic/kibana into feature…

bd5b960

…/alert-summary-data

removing ruleid dependency

56bc2bd

fixing error

bbb047e

kibanamachine and others added 3 commits September 20, 2022 10:27

Merge branch 'main' into feature/alert-summary-data

1f2ff53

minor changes

b8e818e

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

f35a08d

…ansara/kibana into feature/alert-summary-data

kdelemme approved these changes Sep 21, 2022

View reviewed changes

benakansara and others added 3 commits September 21, 2022 16:03

changes as per feedback

3eeedba

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

3ae539b

…ansara/kibana into feature/alert-summary-data

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

0282605

…-fix'

fkanout self-requested a review September 21, 2022 15:38

fkanout reviewed Sep 21, 2022

View reviewed changes

benakansara and others added 11 commits September 21, 2022 18:13

making alertId not optional

0ef3509

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

9032c9e

…ansara/kibana into feature/alert-summary-data

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

f66bb4f

…-fix'

removing hardcoded field names

334e4ff

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

96a9b07

…ansara/kibana into feature/alert-summary-data

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

f842a38

…-fix'

fixing type error

83a86bf

Merge branch 'feature/alert-summary-data' of https://github.com/benak…

e222443

…ansara/kibana into feature/alert-summary-data

fixing CI errors

e108898

fixing tests

e70b707

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

6dd1d28

…-ref HEAD~1..HEAD --fix'

fkanout self-requested a review September 23, 2022 12:54

fkanout reviewed Sep 23, 2022

View reviewed changes

fkanout approved these changes Sep 23, 2022

View reviewed changes

Merge branch 'main' into feature/alert-summary-data

5a3891e

benakansara enabled auto-merge (squash) September 23, 2022 14:33

benakansara merged commit 1b496c5 into elastic:main Sep 23, 2022

kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Sep 23, 2022

benakansara deleted the feature/alert-summary-data branch November 18, 2022 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Displaying alert data in alert summary in alert details page #140339

Displaying alert data in alert summary in alert details page #140339

benakansara commented Sep 8, 2022 •

edited

Loading

benakansara commented Sep 20, 2022

benakansara commented Sep 21, 2022

kdelemme left a comment

kdelemme Sep 21, 2022

benakansara Sep 21, 2022

kdelemme Sep 21, 2022

benakansara Sep 21, 2022

fkanout Sep 21, 2022

benakansara Sep 21, 2022

benakansara Sep 21, 2022 •

edited

Loading

fkanout Sep 23, 2022

benakansara Sep 23, 2022

fkanout Sep 21, 2022

benakansara Sep 21, 2022

benakansara Sep 22, 2022

fkanout Sep 23, 2022

fkanout Sep 21, 2022

benakansara Sep 21, 2022

fkanout Sep 23, 2022

benakansara Sep 23, 2022 •

edited

Loading

fkanout Sep 23, 2022

benakansara Sep 23, 2022

fkanout left a comment •

edited

Loading

benakansara commented Sep 23, 2022

kibana-ci commented Sep 23, 2022


		export const tags: string[] = ['tag1', 'tag2', 'tag3'];

		export const alert: TopAlert \| null = {

	export const alert: TopAlert \| null = {
	export const alert: TopAlert = {

	export const alertWithTags: TopAlert \| null = {
	export const alertWithTags: TopAlert = {

Displaying alert data in alert summary in alert details page #140339

Displaying alert data in alert summary in alert details page #140339

Conversation

benakansara commented Sep 8, 2022 • edited Loading

Summary

Checklist

benakansara commented Sep 20, 2022

benakansara commented Sep 21, 2022

kdelemme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benakansara Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benakansara Sep 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fkanout left a comment • edited Loading

Choose a reason for hiding this comment

benakansara commented Sep 23, 2022

kibana-ci commented Sep 23, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

benakansara commented Sep 8, 2022 •

edited

Loading

benakansara Sep 21, 2022 •

edited

Loading

benakansara Sep 23, 2022 •

edited

Loading

fkanout left a comment •

edited

Loading