-
Notifications
You must be signed in to change notification settings - Fork 303
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
Updating IRIS Alerter to use ElastAlert Alerter defaults #1532
Conversation
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? |
Hello, @bvirgilioamnh I have no objections whatsoever. |
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
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
make test-docker
with my changes.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.