fix(notification platform): Send issue owner team Slack notifications

[ceorourke]

If a team has Slack notifications enabled and an alert rule has a rule to notify issue owners and the owner resolves to said team AND the notification gets digested AND the team's users' issue alert notification settings were turned off, the alert wouldn't fire. This PR passes the event to get_send_to because without it, it'd never return the team as the recipient.

See #28617 for initial implementation.

[wedamija]

I'm digging around in this area too, and I think you should actually have access to the event from inside the digest.

We fetch it out of the record here, when we have a single record and decide to not send as a digest:

sentry/src/sentry/notifications/utils/digest.py

Lines 38 to 53 in 31feae1

	def send_as_alert_notification(
	context: Mapping[str, Any],
	target_type: ActionTargetType,
	target_identifier: Optional[int] = None,
	) -> None:
	"""If there is more than one record for a group, just choose the most recent one."""
	from sentry.mail import mail_adapter
	record = max(
	itertools.chain.from_iterable(
	groups.get(context["group"], []) for groups in context["digest"].values()
	),
	key=get_timestamp,
	)
	notification = Notification(record.value.event, rules=record.value.rules)
	mail_adapter.notify(notification, target_type, target_identifier)

[wedamija]

I guess one question here is whether we should be retrieving the affected participants from all groups in the digest, since there could be multiple owners involved?

Tbh I'm not sure how this worked before the most recent changes...

[mgaeta]

If there are multiple groups in the digest then we drop the Slack notification altogether.
https://github.com/getsentry/sentry/blob/master/src/sentry/notifications/utils/digest.py#L23-L29

def should_send_as_alert_notification(context: Mapping[str, Any]) -> bool:
	...
    return len(context["counts"]) == 1

[ceorourke]

At some point I'd like to build out a digest notification for Slack similar to the emails we send out, it just hasn't been prioritized 🤔

[mgaeta]

Can you also update this

# Get every user ID for every provider as a set.
user_ids = {user.id for users in participants_by_provider.values() for user in users}

to filter to only users?

user_ids = {
    participant.id
    for participants in participants_by_provider.values()
    for participant in participants
    if instanceof(participant, User)
}

Teams can't receive digests.

[wedamija]

Can you also update this

# Get every user ID for every provider as a set.
user_ids = {user.id for users in participants_by_provider.values() for user in users}

to filter to only users?

user_ids = {
    participant.id
    for participants in participants_by_provider.values()
    for participant in participants
    if instanceof(participant, User)
}

Teams can't receive digests.

Why can't a team receive digests?

[ceorourke]

Actually wait yeah, shouldn't we send a Slack notification to the team's channel if there is one group so it passes should_send_as_alert_notification? I know we don't support a team email address but if they have a Slack channel that should still go through. @mgaeta I keep going around in circles in my head if this makes sense or not

[mgaeta]

Why can't a team receive digests?
@wedamija Digests are email-only because we don't have a template in Slack. Teams are Slack-only because we don't have team email addresses.

[mgaeta]

I think it will help with debugging later if we pick better names here like:

event = [
  records 
  for records_by_group in self.digest.values() 
  for records in records_by_group.values()
][0][0].value.event

[mgaeta]

In fact this might be even clearer:

event = [
  record
  for records_by_group in self.digest.values() 
  for records in records_by_group.values()
  for record in records
][0].value.event

[mgaeta]

You should probably delete should_notify() now to avoid confusion later.

[mgaeta]

shouldn't we send a Slack notification to the team's channel if there is one group so it passes should_send_as_alert_notification()?
@ceorourke If there is just one group, then should_send_as_alert_notification() will return True and we'll send it as a regular Alert Rule Notification in send_as_alert_notification().

[wedamija]

Why can't a team receive digests?
@wedamija Digests are email-only because we don't have a template in Slack. Teams are Slack-only because we don't have team email addresses.

I guess maybe I'm confused as to how a team would get here at all? If we have a digest, and we have a team, shouldn't the team be converted to members so that they can receive the digests instead?

[mgaeta]

@wedamija When an alert rule is triggered where the action is "Send a Notification to Issue Owners", the recipients can be a set of users and teams. Before we can send any Alert Rule notifications we have to buffer them into digests (to prevent fire-hosing the user,) where the digest key is just a concatenation of project_id and the "Issue Owners" target_type. Once dequeued, the first thing we do is refetch the recipients. Then if there was only one group we send the notification as a single AlertRuleNotification.

@ceorourke We might want to only fetch participants after the should_send_as_alert_notification() check. That way we know that we're definitely dealing with a multi-group digest and we don't fetch participants twice.

[ceorourke]

I'm sort of wondering how worthwhile this PR is at this point given we now have a ticket to investigate sending Slack digests, which would be a much better solution to this problem. This PR only solves a tiny problem and my guess is a lot of this will be rewritten or moot once we build out Slack digests. @mgaeta what do you think?

I just updated it based on the feedback in case we still want to merge - I suppose this at least unblocks Phillip's demos and we can still go ahead and redo stuff soon.