Skip to content

Commit

Permalink
adds secondary analytics event optionally (getsentry#30645)
Browse files Browse the repository at this point in the history
This PR is part of a greater effort to increase standardization of notifications by adding support for two different analytics events for sending a notification. Now we will always record the integrations.{provider.name}.notification_sent event for every notification in addition to an optional one based on analytics_event. The purpose of this is to make it easier to do different types of analysis (e.g: looking at a specific notification funnel vs tracking how many times we send any type of Slack notification).
  • Loading branch information
Stephen Cefali authored and pull[bot] committed Feb 23, 2024
1 parent bd7464b commit b73eb02
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 34 deletions.
31 changes: 19 additions & 12 deletions src/sentry/notifications/notifications/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class BaseNotification(abc.ABC):
message_builder = "SlackNotificationsMessageBuilder"
fine_tuning_key: str | None = None
metrics_key: str = ""
analytics_event: str = ""

def __init__(self, organization: Organization):
self.organization = organization
Expand Down Expand Up @@ -88,9 +89,6 @@ def get_type(self) -> str:
def get_unsubscribe_key(self) -> tuple[str, int, str | None] | None:
return None

def record_notification_sent(self, recipient: Team | User, provider: ExternalProviders) -> None:
raise NotImplementedError

def get_log_params(self, recipient: Team | User) -> Mapping[str, Any]:
return {
"organization_id": self.organization.id,
Expand All @@ -103,6 +101,24 @@ def get_message_actions(self, recipient: Team | User) -> Sequence[MessageAction]
def get_callback_data(self) -> Mapping[str, Any] | None:
return None

def record_analytics(self, event_name: str, **kwargs: Any) -> None:
analytics.record(event_name, **kwargs)

def record_notification_sent(self, recipient: Team | User, provider: ExternalProviders) -> None:
# may want to explicitly pass in the parameters for this event
self.record_analytics(
f"integrations.{provider.name}.notification_sent",
category=self.get_category(),
**self.get_log_params(recipient),
)
# record an optional second event
if self.analytics_event:
self.record_analytics(
self.analytics_event,
providers=provider.name.lower(),
**self.get_log_params(recipient),
)


class ProjectNotification(BaseNotification, abc.ABC):
def __init__(self, project: Project) -> None:
Expand All @@ -114,14 +130,5 @@ def get_project_link(self) -> str:
project_link: str = absolute_uri(f"/{self.organization.slug}/{self.project.slug}/")
return project_link

def record_notification_sent(self, recipient: Team | User, provider: ExternalProviders) -> None:
analytics.record(
f"integrations.{provider.name.lower()}.notification_sent",
actor_id=recipient.id,
category=self.get_category(),
organization_id=self.organization.id,
project_id=self.project.id,
)

def get_log_params(self, recipient: Team | User) -> Mapping[str, Any]:
return {"project_id": self.project.id, **super().get_log_params(recipient)}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

from django.urls import reverse

from sentry import analytics
from sentry.models import OrganizationMember
from sentry.notifications.notifications.organization_request import OrganizationRequestNotification
from sentry.notifications.notifications.strategies.member_write_role_recipient_strategy import (
Expand Down Expand Up @@ -80,12 +79,10 @@ def get_message_actions(self, recipient: Team | User) -> Sequence[MessageAction]
def get_callback_data(self) -> Mapping[str, Any]:
return {"member_id": self.pending_member.id, "member_email": self.pending_member.email}

def record_notification_sent(self, recipient: Team | User, provider: ExternalProviders) -> None:
analytics.record(
self.analytics_event,
organization_id=self.organization.id,
user_id=self.pending_member.inviter.id if self.pending_member.inviter else None,
target_user_id=recipient.id,
invited_member_id=self.pending_member.id,
providers=provider.name.lower(),
)
def get_log_params(self, recipient: Team | User) -> MutableMapping[str, Any]:
# TODO: figure out who the user should be when pending_member.inviter is None
return {
**super().get_log_params(recipient),
"user_id": self.pending_member.inviter.id if self.pending_member.inviter else None,
"invited_member_id": self.pending_member.id,
}
22 changes: 10 additions & 12 deletions src/sentry/notifications/notifications/organization_request/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import logging
from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping, Sequence, Type

from sentry import analytics, roles
from sentry import roles
from sentry.models import NotificationSetting, OrganizationMember, Team
from sentry.notifications.notifications.base import BaseNotification
from sentry.notifications.notifications.strategies.role_based_recipient_strategy import (
Expand All @@ -22,7 +22,6 @@


class OrganizationRequestNotification(BaseNotification, abc.ABC):
analytics_event: str = ""
referrer_base: str = ""
member_by_user_id: MutableMapping[int, OrganizationMember] = {}
fine_tuning_key = "approval"
Expand Down Expand Up @@ -113,13 +112,12 @@ def build_notification_footer(self, recipient: Team | User) -> str:
def get_title_link(self) -> str | None:
return None

def record_notification_sent(self, recipient: Team | User, provider: ExternalProviders) -> None:
# this event is meant to work for multiple providers but architecture
# limitations mean we will fire individual for each provider
analytics.record(
self.analytics_event,
organization_id=self.organization.id,
user_id=self.requester.id,
target_user_id=recipient.id,
providers=provider.name.lower(),
)
def get_log_params(self, recipient: Team | User) -> MutableMapping[str, Any]:
if isinstance(recipient, Team):
raise NotImplementedError

return {
**super().get_log_params(recipient),
"user_id": self.requester.id,
"target_user_id": recipient.id,
}

0 comments on commit b73eb02

Please sign in to comment.