fix(alerts): Fix alert link unfurling URL#111033
Conversation
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| metric_alerts_link_regex = re.compile( | ||
| r"^https?\://(?#url_prefix)[^/]+/organizations/(?P<org_slug>[^/]+)/alerts/rules/details/(?P<alert_rule_id>\d+)" | ||
| r"^https?\://(?#url_prefix)[^/]+/organizations/(?P<org_slug>[^/]+)/issues/alerts/rules/details/(?P<alert_rule_id>\d+)" | ||
| ) | ||
|
|
||
| customer_domain_metric_alerts_link_regex = re.compile( | ||
| r"^https?\://(?P<org_slug>[^/]+?)\.(?#url_prefix)[^/]+/alerts/rules/details/(?P<alert_rule_id>\d+)" | ||
| r"^https?\://(?P<org_slug>[^/]+?)\.(?#url_prefix)[^/]+/issues/alerts/rules/details/(?P<alert_rule_id>\d+)" | ||
| ) |
There was a problem hiding this comment.
Bug: The URL generation for metric alerts produces the old format (/alerts/rules/details/), but the updated Slack unfurling regex only matches the new format (/issues/alerts/rules/details/), causing unfurling to fail.
Severity: HIGH
Suggested Fix
Update the Django URL pattern for sentry-metric-alert-details in src/sentry/web/urls.py to use the new path /issues/alerts/rules/details/. This will ensure that generated URLs match the format expected by the new unfurling regex.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/integrations/slack/unfurl/metric_alerts.py#L184-L190
Potential issue: The pull request updated the regex in
`src/sentry/integrations/slack/unfurl/metric_alerts.py` to match metric alert URLs with
the path `/issues/alerts/rules/details/`. However, the backend code that generates these
URLs was not updated. The `build_title_link` function still uses the Django URL pattern
`sentry-metric-alert-details` from `src/sentry/web/urls.py`, which resolves to the old
path `/alerts/rules/details/`. As a result, URLs sent in Slack notifications will have
the old format, which the new unfurling logic will not recognize. This will cause metric
alert link unfurling in Slack to silently fail.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
This is referring to the routes in web/urls.py which only route requests to the react frontend and don't actually define which routes are valid. We still have the old /alerts/ route in there for redirecting purposes.
3 participants
Ref https://linear.app/getsentry/issue/ISWF-769/slack-metric-alert-link-unfurls-not-working
Looks like we're still using the URLs from the old navigation (
/alerts/) and never updated to the new pathname/issues/alerts.I don't think this will actually fix the problem completely, but it will at least get us closer to getting working unfurl links.