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 a summary field to alerts #15886

Merged
merged 17 commits into from Sep 19, 2023
Merged

Conversation

MrZammler
Copy link
Contributor

Summary

Adds a summary field to alerts. This is a short text field that can be used in some places where we currently use the alert name.

The field can be enriched as with the info field, with chart labels. This brings some more information in a glance in e.g. notifications subjects and our alert list in dashboard.

As an example, the same notification will change from:

image

to

image

Because we plan to remove the family entry from alerts, we need a way to present alert specific instance information to notifications.

Test Plan
Additional Information
For users: How does this change affect me?

@sashwathn
Copy link
Contributor

@hugovalente-pm : I don't see this impacting the Mobile App in anyway if it is purely enriching the Alert Information.. Once the payload includes this, the app will automatically use this - unless we set them up as a separate field.
@car12o Any comments?

@car12o
Copy link
Contributor

car12o commented Aug 31, 2023

@hugovalente-pm : I don't see this impacting the Mobile App in anyway if it is purely enriching the Alert Information.. Once the payload includes this, the app will automatically use this - unless we set them up as a separate field. @car12o Any comments?

I'm not yet fully aware of this feature but from glance I think it will be a separate field.

@hugovalente-pm
Copy link
Contributor

@car12o @sashwathn yes this is a new filed, the initial list of task (to be reviewed/confirmed) is stating that netdata/netdata-cloud#130 (comment) since we are not getting rid of the previous alert name we are inrtoducing this summary field

@MrZammler
Copy link
Contributor Author

We can think of the summary field as a prettier alert name :-)

@MrZammler MrZammler marked this pull request as ready for review September 15, 2023 08:06
@MrZammler
Copy link
Contributor Author

I've added the summary field to some alerts. More can be added later (assuming we're ok with what I've added so far).

When this is merged, we then need another PR to transmit it to the cloud and then change our own and cloud backend's endpoints to provide it to the FE.

We also need to add it to other notifications besides email.

@thiagoftsm
Copy link
Contributor

Hello @MrZammler ,

Please, take a look in Codacy warning https://app.codacy.com/gh/netdata/netdata/pullRequest?prid=12502044 .

Best regards!

@MrZammler
Copy link
Contributor Author

Thanks guys, I think last commit resolved the comments.

Copy link
Contributor

@thiagoftsm thiagoftsm left a comment

Choose a reason for hiding this comment

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

PR is working as expected, LGTM!

@MrZammler MrZammler merged commit 9f0fbff into netdata:master Sep 19, 2023
138 checks passed
@MrZammler MrZammler deleted the alerts-summary branch September 19, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants