nit: include query_extra for metric alert notifications (#73325)

we're constructing `query_extra` in a few spots now.

This consolidates the query_extra string all to utilize a single method
to derive the query extra from the existing snuba_query and subscription


NOTE:

There are a handful of areas where we are referencing
`snuba_query.query` _without_ taking into account the `query_extra`

This might possibly lead to discrepancies if we are updating the
snuba_query `query`, comparing queries, or executing one off snuba
queries 🤔

eg. 
- snuba/tasks.py::update_subscription_in_snuba
- incidents/logic.py::update_alert_rule
- subscription_processor.py::get_comparison_aggregation_value
etc.etc.

TODO: we may want to audit all uses and determine whether we should be
constructing the query from the subscription rather than the
`snuba_query` 🤔

src/sentry/incidents/action_handlers.py

@@ -35,6 +35,7 @@ from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.services.hybrid_cloud.user_option import RpcUserOption, user_option_service
 from sentry.snuba.metrics import format_mri_field, is_mri_field
+from sentry.snuba.utils import build_query_strings
 from sentry.types.actor import Actor, ActorType
 from sentry.utils.email import MessageBuilder, get_email_addresses
 
@@ -403,11 +404,6 @@ def generate_incident_trigger_email_context(
     is_active = trigger_status == TriggerStatus.ACTIVE
     is_threshold_type_above = alert_rule.threshold_type == AlertRuleThresholdType.ABOVE.value
     subscription = incident.subscription
-    query_extra = ""
-    if subscription and subscription.query_extra:
-        if snuba_query.query:
-            query_extra = " and "
-        query_extra += subscription.query_extra
 
     # if alert threshold and threshold type is above then show '>'
     # if resolve threshold and threshold type is *BELOW* then show '>'
@@ -437,8 +433,7 @@ def generate_incident_trigger_email_context(
                 alert_rule=incident.alert_rule,
                 selected_incident=incident,
                 size=ChartSize({"width": 600, "height": 200}),
-                query_extra=query_extra,
-                project_id=subscription.project_id if subscription else None,
+                subscription=subscription,
             )
         except Exception:
             logging.exception("Error while attempting to build_metric_alert_chart")
@@ -473,6 +468,7 @@ def generate_incident_trigger_email_context(
     snooze_alert = True
     snooze_alert_url = alert_link + "&" + urlencode({"mute": "1"})
 
+    query_str = build_query_strings(subscription=subscription, snuba_query=snuba_query).query_string
     return {
         "link": alert_link,
         "project_slug": project.slug,
@@ -481,7 +477,7 @@ def generate_incident_trigger_email_context(
         "time_window": format_duration(snuba_query.time_window / 60),
         "triggered_at": incident.date_added,
         "aggregate": aggregate,
-        "query": f"{snuba_query.query}{query_extra}",
+        "query": query_str,
         "threshold": threshold,
         # if alert threshold and threshold type is above then show '>'
         # if resolve threshold and threshold type is *BELOW* then show '>'

src/sentry/incidents/charts.py

@@ -22,7 +22,8 @@ from sentry.models.organization import Organization
 from sentry.models.user import User
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.entity_subscription import apply_dataset_query_conditions
-from sentry.snuba.models import SnubaQuery
+from sentry.snuba.models import QuerySubscription, SnubaQuery
+from sentry.snuba.utils import build_query_strings
 
 CRASH_FREE_SESSIONS = "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate"
 CRASH_FREE_USERS = "percentage(users_crashed, users) AS _crash_rate_alert_aggregate"
@@ -162,8 +163,7 @@ def build_metric_alert_chart(
     end: str | None = None,
     user: Optional["User"] = None,
     size: ChartSize | None = None,
-    query_extra: str | None = None,
-    project_id: int | None = None,
+    subscription: QuerySubscription | None = None,
 ) -> str | None:
     """
     Builds the dataset required for metric alert chart the same way the frontend would
@@ -212,10 +212,10 @@ def build_metric_alert_chart(
     if first_subscription_or_none is None:
         return None
 
-    project_id = project_id or first_subscription_or_none.project_id
+    project_id = subscription.project_id if subscription else first_subscription_or_none.project_id
     time_window_minutes = snuba_query.time_window // 60
     env_params = {"environment": snuba_query.environment.name} if snuba_query.environment else {}
-    query_str = f"{snuba_query.query}{query_extra if query_extra else ''}"
+    query_str = build_query_strings(subscription=subscription, snuba_query=snuba_query).query_string
     query = (
         query_str
         if is_crash_free_alert

src/sentry/incidents/subscription_processor.py

@@ -219,6 +219,7 @@ class SubscriptionProcessor:
         )
         try:
             project_ids = [self.subscription.project_id]
+            # TODO: determine whether we need to include the subscription query_extra here
             query_builder = entity_subscription.build_query_builder(
                 query=snuba_query.query,
                 project_ids=project_ids,

src/sentry/integrations/discord/actions/metric_alert.py

@@ -27,6 +27,7 @@ def send_incident_alert_notification(
                 organization=incident.organization,
                 alert_rule=incident.alert_rule,
                 selected_incident=incident,
+                subscription=incident.subscription,
             )
         except Exception as e:
             sentry_sdk.capture_exception(e)

src/sentry/integrations/metric_alerts.py

@@ -176,9 +176,9 @@ def metric_alert_attachment_info(
     else:
         status = INCIDENT_STATUS[IncidentStatus.CLOSED]
 
-    query = None
+    url_query = None
     if selected_incident:
-        query = parse.urlencode({"alert": str(selected_incident.identifier)})
+        url_query = parse.urlencode({"alert": str(selected_incident.identifier)})
     title = f"{status}: {alert_rule.name}"
     title_link = alert_rule.organization.absolute_url(
         reverse(
@@ -188,7 +188,7 @@ def metric_alert_attachment_info(
                 "alert_rule_id": alert_rule.id,
             },
         ),
-        query=query,
+        query=url_query,
     )
 
     if metric_value is None:
@@ -210,13 +210,16 @@ def metric_alert_attachment_info(
         text = get_incident_status_text(alert_rule, metric_value)
 
     date_started = None
+    activation = None
     if selected_incident:
         date_started = selected_incident.date_started
+        activation = selected_incident.activation
 
     last_triggered_date = None
     if latest_incident:
         last_triggered_date = latest_incident.date_started
 
+    # TODO: determine whether activated alert data is useful for integration messages
     return {
         "title": title,
         "text": text,
@@ -225,4 +228,9 @@ def metric_alert_attachment_info(
         "date_started": date_started,
         "last_triggered_date": last_triggered_date,
         "title_link": title_link,
+        "monitor_type": alert_rule.monitor_type,  # 0 = continuous, 1 = activated
+        "activator": (activation.activator if activation else ""),
+        "condition_type": (
+            activation.condition_type if activation else None
+        ),  # 0 = release creation, 1 = deploy creation
     }

src/sentry/integrations/slack/unfurl/metric_alerts.py

@@ -111,6 +111,7 @@ def unfurl_metric_alerts(
                     start=link.args["start"],
                     end=link.args["end"],
                     user=user,
+                    subscription=selected_incident.subscription if selected_incident else None,
                 )
             except Exception as e:
                 sentry_sdk.capture_exception(e)

src/sentry/integrations/slack/utils/notifications.py

@@ -57,6 +57,7 @@ def send_incident_alert_notification(
                 organization=organization,
                 alert_rule=incident.alert_rule,
                 selected_incident=incident,
+                subscription=incident.subscription,
             )
         except Exception as e:
             sentry_sdk.capture_exception(e)

src/sentry/snuba/tasks.py

@@ -17,6 +17,7 @@ from sentry.snuba.entity_subscription import (
     get_entity_subscription_from_snuba_query,
 )
 from sentry.snuba.models import QuerySubscription, SnubaQuery
+from sentry.snuba.utils import build_query_strings
 from sentry.tasks.base import instrumented_task
 from sentry.utils import metrics
 from sentry.utils.snuba import SNUBA_INFO, SnubaError, _snuba_pool
@@ -201,13 +202,9 @@ def _create_in_snuba(subscription: QuerySubscription) -> str:
             snuba_query,
             subscription.project.organization_id,
         )
-        extra = ""
-        if subscription.query_extra:
-            if snuba_query.query:
-                extra = " and "
-            extra += subscription.query_extra
+        query_string = build_query_strings(subscription, snuba_query).query_string
         snql_query = entity_subscription.build_query_builder(
-            query=f"{snuba_query.query}{extra}",
+            query=query_string,
             project_ids=[subscription.project_id],
             environment=snuba_query.environment,
             params={

src/sentry/snuba/utils.py

@@ -1,3 +1,4 @@
+from dataclasses import dataclass
 from typing import Any
 
 from sentry.snuba import (
@@ -12,6 +13,7 @@ from sentry.snuba import (
     spans_metrics,
     transactions,
 )
+from sentry.snuba.models import QuerySubscription, SnubaQuery
 
 # Doesn't map 1:1 with real datasets, but rather what we present to users
 # ie. metricsEnhanced is not a real dataset
@@ -32,3 +34,32 @@ DATASET_LABELS = {value: key for key, value in DATASET_OPTIONS.items()}
 
 def get_dataset(dataset_label: str) -> Any | None:
     return DATASET_OPTIONS.get(dataset_label)
+
+
+@dataclass
+class QueryStrings:
+    query_string: str
+    query_extra: str
+    query: str
+
+
+def build_query_strings(
+    subscription: QuerySubscription | None, snuba_query: SnubaQuery
+) -> QueryStrings:
+    """
+    Constructs a QueryStrings dataclass given a QuerySubscription and SnubaQuery.
+    query_string value is derived from the snuba_query.query and the subscription.query_extra.
+
+    TODO: determine whether this is necessary in all places where `snuba_query.query` is used.
+    """
+    query_extra = ""
+    if subscription and subscription.query_extra:
+        if snuba_query.query:
+            query_extra = " and "
+        query_extra += subscription.query_extra
+
+    return QueryStrings(
+        query=snuba_query.query,
+        query_extra=query_extra,
+        query_string=f"{snuba_query.query}{query_extra}",
+    )

static/app/views/alerts/rules/metric/details/metricActivity.tsx

@@ -77,6 +77,7 @@ function MetricAlertActivity({organization, incident}: MetricAlertActivityProps)
               }/releases/${encodeURIComponent(activation.activator)}/`,
               query: {project: project},
             }}
+            style={{textOverflow: 'ellipsis', overflowX: 'inherit'}}
           >
             {activation.activator}
           </GlobalSelectionLink>
@@ -171,5 +172,4 @@ const Cell = styled('div')`
   font-size: ${p => p.theme.fontSizeMedium};
   padding: ${space(1)};
   overflow-x: hidden;
-  text-overflow: ellipsis;
 `;