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

Updating IRIS Alerter to use ElastAlert Alerter defaults #1532

Merged
merged 9 commits into from
Sep 17, 2024

Conversation

bvirgilioamnh
Copy link
Contributor

@bvirgilioamnh bvirgilioamnh commented Sep 9, 2024

Description

Currently the IRIS alert uses rule supplied values for the description and simply uses the rule title for the alert title. This significantly reduces the ability to dynamically create/repurpose alerts as the rule creator must tailor each individual alert with a custom description. Additionally this description field as it stands does not resolve any variable or template fields resulting in a static description for all alerts.

This PR aims to resolve this by using the built in create_alert_body() and create_title() functions within the Alerter class.

As this is possibly a breaking change I'm opening this PR as a draft for discussion.

Checklist

  • I have reviewed the contributing guidelines.
  • I have included unit tests for my changes or additions.
  • I have successfully run make test-docker with my changes.
  • I have manually tested all relevant modes of the change in this PR.
  • I have updated the documentation.
  • I have updated the changelog.

Questions or Comments

Do not merge this PR until discussed! :) I think this should be complete now. The defaults should be close enough to the current functionality and is more in line with expectations with the ElastAlert ecosystem.

@bvirgilioamnh
Copy link
Contributor Author

bvirgilioamnh commented Sep 9, 2024

Hey @malinkinsa - I'm just pinging you to see if you have any feedback for the reasoning behind statically setting the Alert Description and Alert Title.

This PR aims to standardize the title and description using the built in functions from ElastAlert, but I think it might be a possible breaking change within the IRIS alert functionality?

@bvirgilioamnh bvirgilioamnh changed the title Update iris.py to use Alerter defaults to create body and title Updating IRIS Alerter to use ElastAlert Alerter defaults Sep 9, 2024
@malinkinsa
Copy link
Contributor

Hello, @bvirgilioamnh I have no objections whatsoever.

@bvirgilioamnh bvirgilioamnh marked this pull request as ready for review September 13, 2024 18:43
@jertel
Copy link
Owner

jertel commented Sep 16, 2024

Will you please move the changelog entry up to the Breaking changes section? The PR shouldn't "break" most users, but it's possible someone could have some script programmatically checking an alert email or message for a specific string, and this change could impact that. So better to be cautious.

Moving change log entry to breaking changes
@jertel jertel merged commit 928746b into jertel:master Sep 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants