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: Separate overlapping legacy and UA alerting routes #76517

Merged
merged 4 commits into from
Jan 4, 2024

Conversation

JacobsonMT
Copy link
Member

@JacobsonMT JacobsonMT commented Oct 13, 2023

To facilitate upcoming feature flag and migration/upgrade page improvements, it's necessary to separate the shared legacy alerting and unified alerting routes / api endpoints.

This renames the legacy routes/api's and keeps the UA ones the same, this should limit impact of unintentional problems. Changes:

  • api/alert-notifiers -> api/alert-notifiers-legacy
    • The purpose of this api endpoint is to return the list of available integration types for notification channels.
  • alerting/list -> alering-legacy/list
    • The is the route for the the main alert rules list view.
  • alerting/notifications and various subpaths such as /alerting/notifications/receivers/:id/edit -> alerting-legacy/...
    • This is the route and sub-routes related to viewing and editing notification channels.

Special notes for your reviewer:

I don't think we reference the legacy routes and apis in the docs anymore, but if you know of a spot I can update it.

Extracted from: #76074

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@JacobsonMT JacobsonMT added area/alerting Grafana Alerting area/backend area/frontend add to changelog area/alerting-legacy Legacy dashboard alerts, deprecated since 9.0 no-backport Skip backport of PR labels Oct 13, 2023
@JacobsonMT JacobsonMT added this to the 10.2.x milestone Oct 13, 2023
@JacobsonMT JacobsonMT requested review from a team as code owners October 13, 2023 09:28
@JacobsonMT
Copy link
Member Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with jacobsonmt/separate_legacy_UA_routes oss branch and main enterprise branch. How to choose a branch

@JacobsonMT JacobsonMT changed the title Alertnig: Separate overlapping legacy and UA alerting routes Alerting: Separate overlapping legacy and UA alerting routes Oct 13, 2023
Copy link
Contributor

@konrad147 konrad147 left a comment

Choose a reason for hiding this comment

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

Does legacy alerting generate any links in notification messages that might require updates?
I mean e.g. links in emails?

@ephemeral-instances-bot
Copy link

@JacobsonMT
Copy link
Member Author

Does legacy alerting generate any links in notification messages that might require updates? I mean e.g. links in emails?

Good question, only notification channels have deep links that would change with this PR, alert rules would link to the dashboard (which doesn't change in this PR). I wouldn't expect us to embed a deep link to the channel, but I'll fish around. Of course, if they manually add a link in the notification message it will break as well.

Copy link
Member

@soniaAguilarPeiron soniaAguilarPeiron left a comment

Choose a reason for hiding this comment

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

FE changes LGTM!🚀

@JacobsonMT JacobsonMT force-pushed the jacobsonmt/separate_legacy_UA_routes branch from 67535a6 to 0d3889c Compare October 19, 2023 17:29
@JacobsonMT
Copy link
Member Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with jacobsonmt/separate_legacy_UA_routes oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

Copy link
Contributor

@konrad147 konrad147 left a comment

Choose a reason for hiding this comment

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

FE LGTM!

Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale Issue with no recent activity label Nov 24, 2023
Copy link
Contributor

github-actions bot commented Dec 8, 2023

This pull request has been automatically closed because it has not had activity in the last 2 weeks. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Dec 8, 2023
@aangelisc aangelisc modified the milestones: 10.2.x, 10.2.3 Dec 18, 2023
@JacobsonMT JacobsonMT reopened this Jan 4, 2024
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.3.x milestone Jan 4, 2024
api/alert-notifiers, alerting/list, and alerting/notifications existed in both
legacy and UA.
Rename legacy route paths and nav ids to be independent of UA ones.
@JacobsonMT JacobsonMT force-pushed the jacobsonmt/separate_legacy_UA_routes branch from 11f503f to e31c7e9 Compare January 4, 2024 15:04
@JacobsonMT
Copy link
Member Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with jacobsonmt/separate_legacy_UA_routes oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@JacobsonMT JacobsonMT added area/alerting/migration Issues relating to legacy alerting migration and removed stale Issue with no recent activity labels Jan 4, 2024
@JacobsonMT JacobsonMT merged commit c18da48 into main Jan 4, 2024
21 checks passed
@JacobsonMT JacobsonMT deleted the jacobsonmt/separate_legacy_UA_routes branch January 4, 2024 23:01
zserge pushed a commit that referenced this pull request Jan 22, 2024
* Separate overlapping legacy and UA alerting routes

api/alert-notifiers, alerting/list, and alerting/notifications existed in both
legacy and UA.
Rename legacy route paths and nav ids to be independent of UA ones.
@summerwollin summerwollin modified the milestones: 10.3.x, 10.3.0 Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/alerting/migration Issues relating to legacy alerting migration area/alerting Grafana Alerting area/alerting-legacy Legacy dashboard alerts, deprecated since 9.0 area/backend area/frontend no-backport Skip backport of PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants