Skip to content

Commit 4331c54

Browse files
authored
feat(notif-platform): Prototype the template layer (#94369)
This PR introduces some more structure around templates, including a template registry. I've also added some tests to ensure developer interface with it correctly. For example, registering with an unknown key: ``` @template_registry.register('some-unknown-key') MyTemplate(...): ... ``` will yield: ``` tests/sentry/notifications/platform/test_registry.py:40: in test_no_unknown_registration_keys assert ( E AssertionError: Template <class 'sentry.notifications.platform.registry.DemoNotificationTemplate'> was registered with an unknown key (some-unknown-key) E E assert 'some-unknown-key' in NotificationTemplateKey ``` and forgetting a template, but adding to NotificationTemplateKey ``` tests/sentry/notifications/platform/test_registry.py:55: in test_no_unused_production_keys raise AssertionError( E AssertionError: Known NotificationTemplateKey(s) have no associated template registered: 'my_new_template' E Use @template_registry.register() to register them, or explicitly exclude them within this test. ``` We've discussed this below block offline, it's the way forward for now so collapsing it for legibility <details> <summary>Explanation of NotificationRenderedTemplate</summary> This has a few things to consider though: - For email, we have flexibility, we can just pass the existing django templates and get whatever level of customization we want by modifying the templates. - For integrations though, `body` (for example) is just a `str`. So if a notification wanted bold the text of a slack message, it'd have to be signaled within the `str` to bold somehow, likely from a shared templating language (e.g. markdown `**`s), which is idealistic. This would have to be understood by all providers; which is reasonable for MSTeams/Slack/Discord, but might not work for future providers(?) if they don't parse markdown, or do so differently (e.g notion uses `| text` for quotes, md uses `> text` even though most other syntax is md-like) - If we allow markdown in the `str` attributes, it's likely we'd get bugs as soon as someone tries a provider specific md formatting (e.g. imagine like `f"[John](@john)"` or something) - Onboarding new providers would also become a minefield if there is syntax assumed to be understood by all that is done differently for other integrations (e.g. if jira uses [img:link](...) but slack uses ![link](...)) -- If a new provider needs a new format, it means every notify call must now be updated to include the new syntax :( The only way around this I can see is restricting to top level building blocks (e.g. `subject`, `body`, `action`, `chart`) that explicitly will not allow custom formatting, and deferring to renderers to do all non-standard elements. Pros: Consistency out of the box, and devs never need to learn slack markdown vs discord markdown. Cons: If a dev wants to bold text, they need a custom renderer for each provider 🙃 </details>
1 parent a8d17a8 commit 4331c54

File tree

18 files changed

+264
-126
lines changed

18 files changed

+264
-126
lines changed

src/sentry/notifications/platform/README.md

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,56 @@
11
# Notification Platform
22

3-
TODO(ecosystem): Fill this out
3+
This module enables Sentry to send standardized notifications across different providers, with rich contents
4+
like images, calls to action, and dynamic links. It does this through a series of data models and concepts which can be heavily customized,
5+
but also provides some opinionated defaults to make it easy to get started.
46

5-
## NotificationProvider
7+
A quick glossary overview:
8+
9+
- **Notification Service** - the interface to actually send your notifications
10+
- **Notification Data** - the raw data which is necessary for your notification
11+
- **Notification Template** - the class which is responsible for processing your raw Notification Data into a user readable format
12+
- **Notification Rendered Template** - the standardized format all notifications adhere to prior to being sent
13+
- **Notification Provider** - the medium to which a notification is sent (e.g. slack, discord)
14+
- **Notification Provider Renderer** - the adapter to convert templates to provider specific formats (e.g. Slack's BlockKit, HTML for emails)
15+
- **Notification Target** - the intended recipients of a notification
16+
- **Notification Strategy** - a standardized pattern for repeatably creating targets
17+
18+
If you're developing a new feature, and wish to notify a Slack channel, Discord direct message, or just send an email,
19+
refer to the [usage](#usage) instructions ahead. If you're extending the platform, check out [development](#development) instead.
20+
21+
## Usage
22+
23+
For wholly new notifications, you'll have to set up the following:
24+
25+
1. Decide on a `NotificationCategory` (for opt-out) and assign a `NotificationSource` (for logs/metrics) in [types.py](./types.py).
26+
2. Use the above to create a dataclass (following the `NotificationData` protocol in [types.py](./types.py))
27+
3. Create a template (`NotificationTemplate` subclass) to convert your `NotificationData` to a `NotificationRenderedTemplate`.
28+
4. Create targets from the intended recipients (preferably with a `NotificationStrategy`)
29+
5. Import the `NotificationService` into your app code, and call `notify()`!
30+
31+
If you're changing the formatting of an existing notification, just update the loader from Step 3.
32+
If you're sending an existing notification from new code, just import the service from Step 5.
33+
34+
In general, the platform has sensible defaults, and standardized elements to help with consistency across providers and notifications.
35+
If you find yourself needing more customization, a [custom ProviderRenderer](#notificationproviderrenderer) might be helpful, but consider adding the change to all other providers as well.
36+
37+
### Example
38+
39+
<!-- TODO(ecosystem): Add example here -->
40+
41+
## Development
42+
43+
The following are common areas that owners of the NotificationPlatform may need to extend/improve.
44+
45+
### NotificationStrategy
46+
47+
<!-- TODO(ecosystem): Add guidance here -->
48+
49+
### NotificationProviderRenderer
50+
51+
<!-- TODO(ecosystem): Add guidance here -->
52+
53+
### NotificationProvider
654

755
A notification provider is the system which Sentry will trigger in order to notify NotificationTargets, for example; Email or Slack.
856

@@ -14,3 +62,7 @@ To register a new provider:
1462
- Extend `NotificationProvider` from [`.provider`](./provider.py) and implement its methods/variables.
1563
- Import `provider_registry` from [`.registry`](./registry.py) and add it via decorator: `@provider_registry.register(<YOUR-KEY-HERE>)`
1664
4. In [../apps](../apps.py), explicitly import the module to register the provider on initialization.
65+
66+
### NotificationTemplate
67+
68+
<!-- TODO(ecosystem): Add guidance here -->

src/sentry/notifications/platform/discord/provider.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
from sentry.notifications.platform.types import (
88
NotificationData,
99
NotificationProviderKey,
10+
NotificationRenderedTemplate,
1011
NotificationTarget,
1112
NotificationTargetResourceType,
12-
NotificationTemplate,
1313
)
1414
from sentry.organizations.services.organization.model import RpcOrganizationSummary
1515

@@ -23,7 +23,7 @@ class DiscordRenderer(NotificationRenderer[DiscordRenderable]):
2323
@classmethod
2424
def render[
2525
DataT: NotificationData
26-
](cls, *, data: DataT, template: NotificationTemplate[DataT]) -> DiscordRenderable:
26+
](cls, *, data: DataT, rendered_template: NotificationRenderedTemplate) -> DiscordRenderable:
2727
return {}
2828

2929

src/sentry/notifications/platform/email/provider.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
from sentry.notifications.platform.types import (
88
NotificationData,
99
NotificationProviderKey,
10+
NotificationRenderedTemplate,
1011
NotificationTarget,
1112
NotificationTargetResourceType,
12-
NotificationTemplate,
1313
)
1414
from sentry.organizations.services.organization.model import RpcOrganizationSummary
1515

@@ -23,7 +23,7 @@ class EmailRenderer(NotificationRenderer[EmailRenderable]):
2323
@classmethod
2424
def render[
2525
DataT: NotificationData
26-
](cls, *, data: DataT, template: NotificationTemplate[DataT]) -> EmailRenderable:
26+
](cls, *, data: DataT, rendered_template: NotificationRenderedTemplate) -> EmailRenderable:
2727
return {}
2828

2929

src/sentry/notifications/platform/msteams/provider.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
from sentry.notifications.platform.types import (
88
NotificationData,
99
NotificationProviderKey,
10+
NotificationRenderedTemplate,
1011
NotificationTarget,
1112
NotificationTargetResourceType,
12-
NotificationTemplate,
1313
)
1414
from sentry.organizations.services.organization.model import RpcOrganizationSummary
1515

@@ -23,7 +23,7 @@ class MSTeamsRenderer(NotificationRenderer[MSTeamsRenderable]):
2323
@classmethod
2424
def render[
2525
DataT: NotificationData
26-
](cls, *, data: DataT, template: NotificationTemplate[DataT]) -> MSTeamsRenderable:
26+
](cls, *, data: DataT, rendered_template: NotificationRenderedTemplate) -> MSTeamsRenderable:
2727
return {}
2828

2929

src/sentry/notifications/platform/provider.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from sentry.notifications.platform.renderer import NotificationRenderer
44
from sentry.notifications.platform.types import (
55
NotificationCategory,
6+
NotificationData,
67
NotificationProviderKey,
78
NotificationTarget,
89
NotificationTargetResourceType,
@@ -51,7 +52,7 @@ def validate_target(cls, *, target: NotificationTarget) -> None:
5152

5253
@classmethod
5354
def get_renderer(
54-
cls, *, category: NotificationCategory
55+
cls, *, data: NotificationData, category: NotificationCategory
5556
) -> type[NotificationRenderer[RenderableT]]:
5657
"""
5758
Returns an instance of a renderer for a given notification, falling back to the default renderer.

src/sentry/notifications/platform/registry.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from typing import Any
44

55
from sentry.notifications.platform.provider import NotificationProvider
6+
from sentry.notifications.platform.types import NotificationTemplate
67
from sentry.organizations.services.organization.model import RpcOrganizationSummary
78
from sentry.utils.registry import Registry
89

@@ -33,3 +34,4 @@ def get_available(
3334

3435

3536
provider_registry = NotificationProviderRegistry()
37+
template_registry = Registry[type[NotificationTemplate[Any]]]()

src/sentry/notifications/platform/renderer.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from sentry.notifications.platform.types import (
44
NotificationData,
55
NotificationProviderKey,
6-
NotificationTemplate,
6+
NotificationRenderedTemplate,
77
)
88

99

@@ -22,9 +22,13 @@ class NotificationRenderer[RenderableT](Protocol):
2222
@classmethod
2323
def render[
2424
DataT: NotificationData
25-
](cls, *, data: DataT, template: NotificationTemplate[DataT]) -> RenderableT:
25+
](cls, *, data: DataT, rendered_template: NotificationRenderedTemplate) -> RenderableT:
2626
"""
27-
Convert template, and data into a renderable object.
28-
The form of the renderable object is defined by the provider.
27+
Convert a rendered template into a renderable object specific to the provider.
28+
For example, Slack might output BlockKit JSON, email might output HTML/txt.
29+
30+
We pass in the data as well since custom renderers may use raw data to modify the output
31+
for the provider where the template cannot. For example, custom markdown formatting,
32+
provider-specific features like modals, etc.
2933
"""
3034
...

src/sentry/notifications/platform/service.py

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import logging
22
from typing import Final
33

4-
from sentry.notifications.platform.registry import provider_registry
4+
from sentry.notifications.platform.registry import provider_registry, template_registry
55
from sentry.notifications.platform.target import prepare_targets
66
from sentry.notifications.platform.types import (
77
NotificationData,
88
NotificationStrategy,
99
NotificationTarget,
10-
NotificationTemplate,
1110
)
1211

1312
logger = logging.getLogger(__name__)
@@ -21,12 +20,8 @@ class NotificationService[T: NotificationData]:
2120
def __init__(self, *, data: T):
2221
self.data: Final[T] = data
2322

24-
def notify_prepared_target(
25-
self,
26-
*,
27-
target: NotificationTarget,
28-
template: NotificationTemplate[T],
29-
) -> None:
23+
# TODO(ecosystem): Eventually this should be converted to spawn a task with the business logic below
24+
def notify_prepared_target(self, *, target: NotificationTarget) -> None:
3025
"""
3126
Send a notification directly to a prepared target.
3227
NOTE: This method ignores notification settings. When possible, consider using a strategy instead of
@@ -48,8 +43,11 @@ def notify_prepared_target(
4843
provider.validate_target(target=target)
4944

5045
# Step 4: Render the template
51-
renderer = provider.get_renderer(category=self.data.category)
52-
renderable = renderer.render(data=self.data, template=template)
46+
template_cls = template_registry.get(self.data.source)
47+
template = template_cls()
48+
rendered_template = template.render(data=self.data)
49+
renderer = provider.get_renderer(data=self.data, category=template.category)
50+
renderable = renderer.render(data=self.data, rendered_template=rendered_template)
5351

5452
# Step 5: Send the notification
5553
provider.send(target=target, renderable=renderable)
@@ -59,9 +57,7 @@ def notify(
5957
*,
6058
strategy: NotificationStrategy | None = None,
6159
targets: list[NotificationTarget] | None = None,
62-
template: NotificationTemplate[T],
6360
) -> None:
64-
6561
if not strategy and not targets:
6662
raise NotificationServiceError(
6763
"Must provide either a strategy or targets. Strategy is preferred."
@@ -80,4 +76,4 @@ def notify(
8076
prepare_targets(targets=targets)
8177

8278
for target in targets:
83-
self.notify_prepared_target(target=target, template=template)
79+
self.notify_prepared_target(target=target)

src/sentry/notifications/platform/slack/provider.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
from sentry.notifications.platform.types import (
88
NotificationData,
99
NotificationProviderKey,
10+
NotificationRenderedTemplate,
1011
NotificationTarget,
1112
NotificationTargetResourceType,
12-
NotificationTemplate,
1313
)
1414
from sentry.organizations.services.organization.model import RpcOrganizationSummary
1515

@@ -23,7 +23,7 @@ class SlackRenderer(NotificationRenderer[SlackRenderable]):
2323
@classmethod
2424
def render[
2525
DataT: NotificationData
26-
](cls, *, data: DataT, template: NotificationTemplate[DataT]) -> SlackRenderable:
26+
](cls, *, data: DataT, rendered_template: NotificationRenderedTemplate) -> SlackRenderable:
2727
return {}
2828

2929

0 commit comments

Comments
 (0)