Skip to content

ref(np): Dispatch provider renderers through a registry - #124857

Draft
hobzcalvin wants to merge 1 commit into
masterfrom
gp/add_renderer_registry
Draft

hobzcalvin wants to merge 1 commit into
masterfrom
gp/add_renderer_registry

Conversation

@hobzcalvin

Copy link
Copy Markdown
Contributor

Stacked on #112890 (gv/add_msteams_issue_renderer).

Summary

Each provider was hand-rolling the same category → renderer dispatch table inside its own get_renderer override. This replaces those with a NotificationRendererRegistry keyed by (provider_key, category).

  • Renderers register themselves where they are defined, e.g. @renderer_registry.register(NotificationProviderKey.SLACK, NotificationCategory.ISSUE).
  • NotificationProvider.get_renderer is now a single base implementation: look the pair up, fall back to default_renderer. The Slack, Discord, and MS Teams overrides are gone, and with them the function-local renderer imports they needed to dodge the renderer → provider circular import.
  • get_renderer takes only the notification data now; the category comes from the notification's registered template rather than the call site.
  • Dropped provider_key from the NotificationRenderer protocol — nothing read it.

Renderer modules are imported in sentry/notifications/apps.py alongside the providers so registration happens at startup.

Notes for review

  • get_renderer is kept as the override seam rather than having callers hit the registry directly; it is public API used by the notification debugger endpoint and a number of tests, and it is the one place the registry's Any is narrowed back to the provider's renderable type.
  • Dispatch is still keyed on NotificationCategory, which stays coarser than the data. SeerSlackRenderer covers seven sources and still branches internally on the data type. Keying on NotificationSource would let those split apart, but that is a larger change and not attempted here.

Test plan

  • pytest tests/sentry/notifications/platform/ tests/sentry/integrations/slack/test_integration.py — 189 passing.
  • Verified from a clean app startup that all six renderers register: Discord issue and metric alert, MS Teams issue, and Slack issue, metric alert, and Seer.

Made with Cursor

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 17, 2026
Replaces the per-provider `get_renderer` overrides with a `NotificationRendererRegistry`
keyed by provider and notification source. Renderers register themselves where they are
defined, and `NotificationProvider.get_renderer` resolves the renderer from the
notification's source, falling back to the provider's default renderer.

Keying on source rather than category lets the Seer renderers be registered directly,
replacing the nested source check the Slack provider needed to separate
`SeerAgentWriteApprovalSlackRenderer` from `SeerSlackRenderer`.

Also drops the unused `provider_key` attribute from the `NotificationRenderer` protocol.

Co-authored-by: Cursor <cursoragent@cursor.com>
@hobzcalvin
hobzcalvin force-pushed the gp/add_renderer_registry branch from 6f19cf1 to aa068fe Compare September 17, 2026 23:15
@hobzcalvin
hobzcalvin changed the base branch from gv/add_msteams_issue_renderer to master September 17, 2026 23:15
Comment on lines +144 to +145
renderer = renderer_registry.get(provider_key=cls.key, source=data.source)
return renderer or cls.default_renderer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SLACK_STAGING never resolves registered Slack renderers

get_renderer looks up by cls.key, but specialized Slack renderers only register under NotificationProviderKey.SLACK, so SlackStagingNotificationProvider always falls back to the default renderer for issue/metric/seer sources. Register those renderers for SLACK_STAGING as well, or fall back to the SLACK key when cls.key is staging.

Evidence
  • get_renderer calls renderer_registry.get(provider_key=cls.key, source=data.source) and returns cls.default_renderer when that miss returns None.
  • SlackStagingNotificationProvider sets key = NotificationProviderKey.SLACK_STAGING and inherits this base get_renderer.
  • All Slack specialized renderers register only under NotificationProviderKey.SLACK (issue, metric_alert, seer, seer_agent_write_approval).
  • Prior category-based Slack overrides were inherited by staging and returned the specialized renderers; source+provider_key lookup no longer does.

Identified by Warden · sentry-backend-bugs · R99-8QC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant