[Metric threshold] Persist group by information and apply it in the alert details page #181689

maryam-saeidi · 2024-04-25T10:22:26Z

Summary

This PR

Persists group by information and apply it in the alert details page
Adds source and tags to the alert summary field
Fixes annotation issue on the chart by adding a margin-top

Note
I showed the chart title temporarily in the screenshots below for verification: (You can do the same by removing hideTitle)

State	Screenshot
Before
After

How to test

Create a metric threshold rule
- make sure to enable the related feature flag
```
xpack.observability.unsafe.alertDetails.observability.enabled: true
```
Go to the alert details page and verify the charts show data related to the selected group
- either remove hideTitle
- or make sure the data in the chart matches expectations for that specific group
- or check the metrics_explorer
Create an APM Latency threshold rule and check the active alert annotation to have the right color.

…lert details page; Add source and tags to the alert summary field

apmmachine · 2024-04-25T10:22:42Z

🤖 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!)

maryam-saeidi · 2024-04-25T10:27:11Z

/ci

maryam-saeidi · 2024-04-25T10:54:59Z

.../plugins/observability_solution/infra/public/alerting/metric_threshold/components/groups.tsx

+
+import React from 'react';
+
+export function Groups({ groups }: { groups: Array<{ field: string; value: string }> }) {


I've added this Groups component temporarily until we share the Source component between different rules: #181692

maryam-saeidi · 2024-04-25T16:18:26Z

/ci

x-pack/plugins/metrics_data_access/server/lib/metrics/index.ts

Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>

benakansara · 2024-05-07T14:32:45Z

I see that we are introducing kibana.alert.group array in metric threshold rule. I think before we introduce this field, we need to discuss this ticket and make a decision on how we want to streamline saving of group by fields across rules.

We already have context.group and context.groupByKeys as context variables in Metric threshold rule which have string and object formats respectively and customers are familiar with those. With kibana.alert.group, we will have third field for the same purpose with different format.

I think we should also rethink about point Brandon raised here and which Soren mentioned here. I understand that this might be an edge case, but I wander why would we want to do it anyway if we know there is a limitation. Instead groupByKeys which is an object gives better UX where users can directly query kibana.alert.groupByKeys.host.name and it will always give accurate results.

maryam-saeidi · 2024-05-08T08:02:20Z

We already have context.group and context.groupByKeys as context variables in Metric threshold rule which have string and object formats respectively and customers are familiar with those. With kibana.alert.group, we will have third field for the same purpose with different format.

Those fields are in the action variables and when I was checking AAD on the alert details page, we didn't have those fields available.

I think we should also rethink about point Brandon raised here and which Soren mentioned here. I understand that this might be an edge case, but I wander why would we want to do it anyway if we know there is a limitation.

We had this discussion in this RFC and as you mentioned this is an edge case that we don't know it might even happen for our users and there is a proposal in this document about how to solve it if we saw this turns into a challenge for our users.

Instead groupByKeys which is an object gives better UX where users can directly query kibana.alert.groupByKeys.host.name and it will always give accurate results.

Adding kibana.alert.groupByKeys.host.name is similar to the second approach mentioned in RFC which without dynamic mapping does not provide the auto-suggestion which was a requirement and ResponseOps wasn't onboard with adding dynamic mapping due to its impact on number of fields in mapping limit.

elasticmachine · 2024-05-10T08:11:15Z

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

maryam-saeidi · 2024-05-10T08:13:20Z

...ion/apm/public/components/alerting/ui_components/alert_details_app_section/latency_chart.tsx

@@ -142,7 +142,7 @@ function LatencyChart({
        <AlertActiveTimeRangeAnnotation
          alertStart={alert.start}
          alertEnd={alertEnd}
-          color={chroma(transparentize('#F04E981A', 0.2)).hex().toUpperCase()}
+          color={euiTheme.colors.danger}


Opacity of this color is now handled in x-pack/packages/observability/alert_details/src/components/alert_active_time_range_annotation.tsx.

Before this change, annotation for Metric threshold had the following issue:

When I was working on fixing recovered alert annotation in APM Latency chart, euiTheme.colors.danger with opacity 0.1 was not showing right color as I expected, so I used chroma(transparentize('#F04E981A', 0.2)).hex().toUpperCase() with opacity 1. But now it looks fine with euiTheme.colors.danger and opacity 0.1. Not sure what was the issue before. 🤔

...ics_data_access/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.ts

cauemarcondes

LGTM

benakansara

I see that we have a line chart as a main chart. In custom threshold rule which is similar to metric threshold, we are showing bar chart. The preview chart in rule form also has bar chart. Ideally I would have expected to work it similar to preview chart or custom threshold alert page. Was it a design decision to use line chart in metric threshold alert page? If so, could you please add the design link to the PR.

Should we remove "Technical preview"? Once we have alerts history PR merged, I think we are good with first version of the alert page. Wdyt?

benakansara · 2024-05-14T09:22:24Z

Can we adjust the interval of buckets to match with rule interval? For some alerts, it shows no data in chart because the chart isn't able to show the bucket with data.

Custom threshold rule for reference

benakansara · 2024-05-14T09:32:08Z

In case of only "warning", we might need to adjust the "Threshold breached" component and "Alert started" annotation.
Is it possible to -

change "Threshold breached" to have "Warning when > 1"
change color of alert annotation to match with "warning" color and change text to "Warning started"?

maryam-saeidi · 2024-05-14T13:21:22Z

@benakansara Thanks for reviewing this PR thoroughly.

I will try to address the comments as much as possible, but if I see there is a challenge in implementing those, I will create a separate ticket to move forward with this PR. The main goal of this PR was adding group information and the rest was developed a long time ago, so I need to check about the reasons for some decisions (in which the direction might have been changed over time).

About your comments, do you see any of them need to be addressed in this PR or are you fine with having separate tickets for all of them in case I see fixing them is not straightforward?

benakansara · 2024-05-14T13:57:51Z

@maryam-saeidi Let's create separate issues for them and handle outside of this PR. I can create tickets if you agree with the findings.

I will do one more round of testing.

maryam-saeidi · 2024-05-14T14:00:05Z

@maryam-saeidi Let's create separate issues for them and handle outside of this PR. I can create tickets if you agree with the findings.

I will do one more round of testing.

Sure, let me give them a try, and I will let you know which one will not be covered.

benakansara

LGTM. Tested locally, and worked as expected. 🎉
I have left some comments/questions.

...bservability_solution/infra/public/alerting/metric_threshold/components/expression_chart.tsx

...ck/plugins/observability_solution/infra/public/alerting/metric_threshold/components/tags.tsx

...servability_solution/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts

x-pack/plugins/observability_solution/metrics_data_access/common/http_api/metrics_explorer.ts

x-pack/plugins/observability_solution/metrics_data_access/server/lib/metrics/index.ts

benakansara · 2024-05-14T17:58:10Z

...ion/apm/public/components/alerting/ui_components/alert_details_app_section/latency_chart.tsx

@@ -142,7 +142,7 @@ function LatencyChart({
        <AlertActiveTimeRangeAnnotation
          alertStart={alert.start}
          alertEnd={alertEnd}
-          color={chroma(transparentize('#F04E981A', 0.2)).hex().toUpperCase()}
+          color={euiTheme.colors.danger}


When I was working on fixing recovered alert annotation in APM Latency chart, euiTheme.colors.danger with opacity 0.1 was not showing right color as I expected, so I used chroma(transparentize('#F04E981A', 0.2)).hex().toUpperCase() with opacity 1. But now it looks fine with euiTheme.colors.danger and opacity 0.1. Not sure what was the issue before. 🤔

maryam-saeidi · 2024-05-15T11:02:45Z

@benakansara Regarding your comments:

Changing to a line chart is easy, but then we will have the challenge of aligning annotations, so let's have a separate ticket for it.
No data chart

Can we adjust the interval of buckets to match with rule interval? For some alerts, it shows no data in chart because the chart isn't able to show the bucket with data.

The data is correctly bucketed based on the look-back window but I think the issue lies somewhere else related to how charts aggregate data. Can you please create a ticket for it and assign it to me? I will investigate it separately.

Warning state
Good catch, can you please create a ticket for it?
I think we need to discuss more about how we want to show warning information on this page, keeping in mind that we didn't want to spend much time on this alert details page, but wanted to have a simple base ready.

change "Threshold breached" to have "Warning when > 1"

We can either do this or always show both thresholds and indicate which one is breached in the title.

change color of alert annotation to match with "warning" color and change text to "Warning started"?

I think we can keep it as is since it is an alert even if it is because of breaching the warning threshold. (Just to avoid extra work for a rule that will be deprecated in the future)

I created a ticket to deprecate the feature flag and I included the removal of the technical preview badge there, can you please list the above tickets in the ticket that I shared?

x-pack/plugins/observability_solution/metrics_data_access/server/lib/metrics/index.ts

kibana-ci · 2024-05-16T08:36:10Z

💚 Build Succeeded

Buildkite Build
Commit: 6c00433
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-181689-6c0043310bc1
Observability Deployment

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1555	1557	+2

Async chunks

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

id	before	after	diff
`apm`	3.3MB	3.3MB	-222.0B
`infra`	1.5MB	1.5MB	+1.7KB
total			+1.5KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	-	5405	+5405
`total size`	-	8.8MB	+8.8MB

Page load bundle

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

id	before	after	diff
`infra`	105.4KB	105.5KB	+90.0B

History

💚 Build #210113 succeeded 9540fac
💛 Build #209392 was flaky 4f42618
💚 Build #209163 succeeded b59403a
💚 Build #209135 succeeded 87440f5

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

cc @fkanout @maryam-saeidi

Metric threshold rule: persist group by information and apply it in a…

649d6f5

…lert details page; Add source and tags to the alert summary field

maryam-saeidi added release_note:skip Skip the PR/issue when compiling release notes Feature:Alert Details Page Observability ux management team labels Apr 25, 2024

maryam-saeidi self-assigned this Apr 25, 2024

Add test

84cb3ad

maryam-saeidi commented Apr 25, 2024

View reviewed changes

Fix test

2ea5071

maryam-saeidi marked this pull request as ready for review April 25, 2024 18:20

maryam-saeidi requested review from a team as code owners April 25, 2024 18:20

maryam-saeidi added 2 commits April 25, 2024 20:20

Merge branch 'main' into 178998-apply-group-to-metric-threshold

ab29b6c

Merge branch 'main' into 178998-apply-group-to-metric-threshold

43d3ec0

fkanout self-assigned this Apr 26, 2024

smith added the apm:review label Apr 26, 2024

cauemarcondes reviewed Apr 29, 2024

View reviewed changes

x-pack/plugins/metrics_data_access/server/lib/metrics/index.ts Outdated Show resolved Hide resolved

botelastic bot added the ci:project-deploy-observability Create an Observability project label Apr 29, 2024

Update x-pack/plugins/metrics_data_access/server/lib/metrics/index.ts

9a19f67

Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>

maryam-saeidi added 2 commits May 8, 2024 09:17

Revert 9a19f67

0468202

Merge branch 'main' into 178998-apply-group-to-metric-threshold

e29dd94

Fix alert annotation color

a5a75da

botelastic bot added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label May 10, 2024

maryam-saeidi commented May 10, 2024

View reviewed changes

maryam-saeidi requested a review from cauemarcondes May 10, 2024 08:15

Merge branch 'main' into 178998-apply-group-to-metric-threshold

87440f5

cauemarcondes reviewed May 10, 2024

View reviewed changes

...ics_data_access/server/routes/metrics_explorer/lib/convert_request_to_metrics_api_options.ts Show resolved Hide resolved

cauemarcondes approved these changes May 10, 2024

View reviewed changes

maryam-saeidi added 3 commits May 10, 2024 12:59

Use reduce instead of map

6c7af97

Merge branch 'main' into 178998-apply-group-to-metric-threshold

b59403a

Merge branch 'main' into 178998-apply-group-to-metric-threshold

4f42618

benakansara self-requested a review May 14, 2024 08:30

benakansara reviewed May 14, 2024

View reviewed changes

benakansara approved these changes May 14, 2024

View reviewed changes

Merge branch 'main' into 178998-apply-group-to-metric-threshold

9540fac

sorenlouv reviewed May 15, 2024

View reviewed changes

x-pack/plugins/observability_solution/metrics_data_access/server/lib/metrics/index.ts Outdated Show resolved Hide resolved

maryam-saeidi added 2 commits May 16, 2024 09:44

Update type

5ff89e3

Merge branch 'main' into 178998-apply-group-to-metric-threshold

6c00433

maryam-saeidi enabled auto-merge (squash) May 16, 2024 08:44

maryam-saeidi merged commit 62a0ce9 into elastic:main May 16, 2024
21 checks passed

kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels May 16, 2024

This was referenced May 16, 2024

[Observability] [Alert Details] Move common alert summary fields to AlertSummary component #183624

Closed

[Metric threshold] [Alert details page] Chart interval does not match with rule interval #183625

Closed

maryam-saeidi deleted the 178998-apply-group-to-metric-threshold branch May 23, 2024 07:32

This was referenced May 28, 2024

[Metric threshold] [Alert details page] Adjust components with warning threshold information #183628

Closed

[Metric threshold] [Alert details page] Change line chart to bar chart #183627

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Metric threshold] Persist group by information and apply it in the alert details page #181689

[Metric threshold] Persist group by information and apply it in the alert details page #181689

maryam-saeidi commented Apr 25, 2024 •

edited

Loading

apmmachine commented Apr 25, 2024

maryam-saeidi commented Apr 25, 2024

maryam-saeidi Apr 25, 2024

maryam-saeidi commented Apr 25, 2024

benakansara commented May 7, 2024

maryam-saeidi commented May 8, 2024

elasticmachine commented May 10, 2024

maryam-saeidi May 10, 2024

benakansara May 14, 2024

cauemarcondes left a comment

benakansara left a comment

benakansara commented May 14, 2024

benakansara commented May 14, 2024

maryam-saeidi commented May 14, 2024

benakansara commented May 14, 2024

maryam-saeidi commented May 14, 2024

benakansara left a comment

benakansara May 14, 2024

maryam-saeidi commented May 15, 2024 •

edited

Loading

kibana-ci commented May 16, 2024 •

edited

Loading


		import React from 'react';

		export function Groups({ groups }: { groups: Array<{ field: string; value: string }> }) {

[Metric threshold] Persist group by information and apply it in the alert details page #181689

[Metric threshold] Persist group by information and apply it in the alert details page #181689

Conversation

maryam-saeidi commented Apr 25, 2024 • edited Loading

Summary

How to test

apmmachine commented Apr 25, 2024

🤖 GitHub comments

maryam-saeidi commented Apr 25, 2024

maryam-saeidi Apr 25, 2024

Choose a reason for hiding this comment

maryam-saeidi commented Apr 25, 2024

benakansara commented May 7, 2024

maryam-saeidi commented May 8, 2024

elasticmachine commented May 10, 2024

maryam-saeidi May 10, 2024

Choose a reason for hiding this comment

benakansara May 14, 2024

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

benakansara left a comment

Choose a reason for hiding this comment

benakansara commented May 14, 2024

benakansara commented May 14, 2024

maryam-saeidi commented May 14, 2024

benakansara commented May 14, 2024

maryam-saeidi commented May 14, 2024

benakansara left a comment

Choose a reason for hiding this comment

benakansara May 14, 2024

Choose a reason for hiding this comment

maryam-saeidi commented May 15, 2024 • edited Loading

kibana-ci commented May 16, 2024 • edited Loading

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Canvas Sharable Runtime

Page load bundle

History

maryam-saeidi commented Apr 25, 2024 •

edited

Loading

maryam-saeidi commented May 15, 2024 •

edited

Loading

kibana-ci commented May 16, 2024 •

edited

Loading