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

Add alert grouping functionality to the observability alerts page #189958

Merged
merged 24 commits into from
Aug 22, 2024

Conversation

maryam-saeidi
Copy link
Member

@maryam-saeidi maryam-saeidi commented Aug 6, 2024

Closes #190995

Summary

This PR adds grouping functionality to the alerts page alert table based on @umbopepato's implementation in this draft PR (basically, he implemented the feature and I adjusted a bit for our use case :D).

For now, we only added the rule and source as default grouping, and I will create a ticket to add tags as well. The challenge with tags is that since it is an array, the value of the alert is joined by a comma as the group, which does not match with what we want for tags.

image

Here is how we show the rules that don't have a group by field selected for them: (We used "ungrouped" similar to what we have in SLOs)

image

@maryam-saeidi maryam-saeidi added the release_note:feature Makes this part of the condensed release notes label Aug 6, 2024
@maryam-saeidi maryam-saeidi self-assigned this Aug 6, 2024
@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@maryam-saeidi
Copy link
Member Author

/ci

content={
<FormattedMessage
id="xpack.observability.alert.grouping.ungrouped.info"
defaultMessage='There is no "group by" field selected in rule definition.'
Copy link
Member Author

@maryam-saeidi maryam-saeidi Aug 6, 2024

Choose a reason for hiding this comment

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

@maryam-saeidi
Copy link
Member Author

/ci

// Alert page
const alertsPageAlertsTableConfig = getAlertsPageTableConfiguration(
// Observability table
const observabilityAlertsTableConfig = getObservabilityTableConfiguration(
Copy link
Member Author

@maryam-saeidi maryam-saeidi Aug 7, 2024

Choose a reason for hiding this comment

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

I registered a new alerts table with an observability ID, which is used on other applications as well. (Basically, this already existed)

alertTableConfigRegistry.register(observabilityAlertsTableConfig);

// Alerts page
const alertsPageAlertsTableConfig = getAlertsPageTableConfiguration(
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new table with grouping, only for the alerts page for now.

@maryam-saeidi
Copy link
Member Author

/ci

Copy link
Member

@umbopepato umbopepato left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Aug 8, 2024
@elasticmachine
Copy link
Contributor

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

Comment on lines +13 to +22
export function Tags({
tags,
color,
size = 3,
oneLine = false,
}: {
tags: string[];
color?: string;
size?: number;
oneLine?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

there is same kinda component exported from observability-shared, might be nice to use that

https:/elastic/kibana/blob/main/x-pack/plugins/observability_solution/observability_shared/public/index.ts#L38

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice! I will check it out!

@shahzad31
Copy link
Contributor

i am getting this error on visiting alerts page
image

umbopepato added a commit that referenced this pull request Aug 19, 2024
… aggregations endpoint (#190305)

## Summary

- Adds null-value bucket detection to server-side alerts aggregations
and marks those groups with a `--` key and `isNullGroup = true`.
- Improves alerts grouping types with default aggregations.
- Improves documentation

## To verify

1. Temporarily merge
[#189958](#189958) into this
branch
2. Create a rule that fires alerts in Observability > Alerts (i.e.
Custom Threshold, ES Query, ...)
3. Once you start to see some alerts in the Alerts page, toggle the
grouped alerts view using the dropdown at the top-right of the table
(`Group alerts by: ...`), selecting a custom field that doesn't have a
value in alert documents (to find one, open the alert flyout and look at
the fields table)
4. Check that the group based on the empty field shows `--` as a title
5. Check that the alerts table in the expanded group panel is filtered
correctly

### References

Refs [#189958](#189958)

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@maryam-saeidi
Copy link
Member Author

/ci

@maryam-saeidi
Copy link
Member Author

/ci

@maryam-saeidi maryam-saeidi marked this pull request as ready for review August 21, 2024 09:05
@shahzad31
Copy link
Contributor

/oblt-deploy

@maryam-saeidi
Copy link
Member Author

/oblt-deploy

@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 22, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: e0da06b
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-189958-e0da06bfbf63

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observability 1073 1116 +43

Async chunks

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

id before after diff
observability 426.9KB 459.8KB +32.9KB

Page load bundle

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

id before after diff
observability 102.0KB 102.4KB +497.0B
Unknown metric groups

async chunk count

id before after diff
observability 19 20 +1

miscellaneous assets size

id before after diff
observability 0.0B 761.8KB ⚠️ +761.8KB

History

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

cc @maryam-saeidi

Comment on lines +34 to +43
filter: [
{
query: {
match_phrase: {
[ALERT_STATUS]: ALERT_STATUS_ACTIVE,
},
},
meta: {},
},
],
Copy link
Contributor

Choose a reason for hiding this comment

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

it's already specified in query so why needed as separate filter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Alert grouping component accepts filters, not queries, but we use queries in other places. Maybe, in the future, we can refactor places that we use query and use filter instead.

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

LGTM !!

@maryam-saeidi maryam-saeidi changed the title Add alert grouping functionality to the alerts page Add alert grouping functionality to the observability alerts page Aug 22, 2024
@maryam-saeidi maryam-saeidi merged commit 34d392b into elastic:main Aug 22, 2024
25 checks passed
@maryam-saeidi maryam-saeidi deleted the grouped-alert-table branch August 22, 2024 10:20
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Aug 22, 2024
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 ci:project-deploy-observability Create an Observability project release_note:feature Makes this part of the condensed release notes Team:obs-ux-management Observability Management User Experience Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Alert grouping] Add grouping functionality to the alerts page
9 participants