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

Alerting: Disable legacy alerting for ever #83651

Merged
merged 10 commits into from
Mar 7, 2024

Conversation

yuri-tceretian
Copy link
Contributor

@yuri-tceretian yuri-tceretian commented Feb 29, 2024

What is this feature?
This PR removes the ability to keep the legacy alerting enabled. If setting [alerting].enabled is set to true, Grafana will refuse to start.
It emits the error suggesting enabling Unified Alerting and performing migration if it's not yet.

Also, it removes the section [alerting] from the default.ini, sample.ini, and documentation. These are not final changes for documentation

Why do we need this feature?
The first step is to remove legacy alerting.

Fixes #81268

Special notes for your reviewer:

Release notice breaking change

In Grafana 11 the legacy alerting reaches the end-of-life. Users cannot enable it and Grafana will refuse to start if the settings are not updated to run the new Grafana Alerting. Migration from legacy alerting is not available as well. Grafana 10.4.x is the last version that offers the migration.

  • If the setting [alerting].enable is set to true Grafana will not start and emit the log message with recommendations to change the configuration

  • Setting [alerting].max_annotation_age is replaced by [unified_alerting.state_history.annotations].max_age

  • Setting [alerting].max_annotations_to_keep is replaced by [unified_alerting.state_history.annotations].max_annotations_to_keep

  • setting [unified_alerting].execute_alerts does not fall back to the legacy [alerting].execute_alerts anymore. Instead, the default value true is used.

  • setting [unified_alerting].evaluation_timeout does not fall back to the legacy setting [alerting].evaluation_timeout_seconds in the case when it is either invalid or has the default value. Now, if the setting is invalid, it will cause Grafana to exit.

  • setting [unified_alerting].min_interval does not fall back to the legacy setting [alerting].min_interval_seconds in the case when it is either invalid or has the default value. Now, if the setting is invalid, it will cause Grafana to exit.

@yuri-tceretian yuri-tceretian added area/alerting Grafana Alerting add to changelog breaking change Relevant for changelog generation no-backport Skip backport of PR labels Feb 29, 2024
@yuri-tceretian yuri-tceretian added this to the 11.0.x milestone Feb 29, 2024
@yuri-tceretian yuri-tceretian self-assigned this Feb 29, 2024
@yuri-tceretian yuri-tceretian requested a review from a team as a code owner February 29, 2024 00:28
@yuri-tceretian yuri-tceretian requested review from undef1nd, suntala and idafurjes and removed request for a team February 29, 2024 00:28
@ryantxu
Copy link
Member

ryantxu commented Feb 29, 2024

FOREVER!

@yuri-tceretian yuri-tceretian force-pushed the yuri-tceretian/disable-legacy-alerting branch from ec98eca to 9b96d3e Compare March 1, 2024 21:31
@yuri-tceretian yuri-tceretian marked this pull request as ready for review March 1, 2024 21:48
@yuri-tceretian yuri-tceretian requested a review from a team March 1, 2024 21:48
@yuri-tceretian yuri-tceretian requested a review from a team as a code owner March 1, 2024 21:48
@yuri-tceretian yuri-tceretian requested review from rwwiv, JacobsonMT, grobinson-grafana and imatwawana and removed request for a team March 1, 2024 21:48
alerting := mg.Cfg.Raw.Section("alerting")
enabled, err := alerting.Key("enabled").Bool()
if err == nil && enabled {
mg.Logger.Error("Option [alerting].enabled cannot be true. Legacy alerting is no longer supported. Delete the option and use configuration '[unified_alerting].enabled' to enable Grafana Alerting and start from the scratch. Refer to documentation about migration to the new alerting (https://grafana.com/docs/grafana/latest/alerting/set-up/migrating-alerts)")
Copy link
Contributor

Choose a reason for hiding this comment

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

@brendamuir can you review the text here? It will be shown in the logs for users who upgrade to Grafana 11 but are still using legacy alerting in their configuration file.

pkg/setting/setting_unified_alerting.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@yuri-tceretian yuri-tceretian force-pushed the yuri-tceretian/disable-legacy-alerting branch from e1d97cc to 9b96d3e Compare March 4, 2024 22:28
if err == nil {
cfg.AlertingEnabled = &enabled
if err == nil && enabled {
cfg.Logger.Error("Option [alerting].enabled cannot be true. Legacy alerting is no longer supported. Delete the option and use configuration '[unified_alerting].enabled' to enable Grafana Alerting and start from the scratch. Refer to documentation about migration to the new alerting (https://grafana.com/docs/grafana/latest/alerting/set-up/migrating-alerts)")
Copy link
Contributor Author

@yuri-tceretian yuri-tceretian Mar 4, 2024

Choose a reason for hiding this comment

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

@brendamuir can you please check if this message is informative enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

cfg.Logger.Error("Option [alerting].enabled cannot be true. Legacy Alerting is removed. It is no longer deployed, enhanced, or supported. Delete [alerting].enabled and use '[unified_alerting].enabled' to enable Grafana Alerting. For more information, refer to the documentation on upgrading to Grafana Alerting. (https://grafana.com/docs/grafana/latest/alerting/set-up/migrating-alerts)")

Copy link
Contributor

Choose a reason for hiding this comment

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

  • not sure about the first bit starting cfg.Logger - cannot be true. is that to appear in the message on the UI? do we need that bit?
  • do we want to say delete xxx and use xxx to enable GA or would we rather say to upgrade to GA? not sure.
  • We use "upgrade" in the docs-so would prob stick to using that rather than migrating?

Copy link
Contributor

Choose a reason for hiding this comment

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

This will appear in the Grafana logs at start up, it won't be shown in the UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok reckon the above is ok then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Brenda , thanks for the recommendation! One thing I would like to change. I am not sure that the article will exist in G11 because migration from legacy to UA will not be supported. I will put link https://grafana.com/docs/grafana/v10.4/alerting/set-up/migrating-alerts/ which does not exist yet but that will be the last verstion that offers migration.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, ok. the plan is to still have a short one pager available just in case in the OSS docs - but add this one for now.

Copy link
Contributor

@rwwiv rwwiv left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@JacobsonMT JacobsonMT left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@yuri-tceretian yuri-tceretian merged commit 7147af6 into main Mar 7, 2024
13 checks passed
@yuri-tceretian yuri-tceretian deleted the yuri-tceretian/disable-legacy-alerting branch March 7, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Block the launch of Grafana if alerting migration happens and somehow fails
8 participants