ref(mep): Move transaction thresholds to new tagging system

In getsentry/relay#1225 we introduced a
general-purpose tagging system for transaction metrics. This may later
be extended:

* we will add outlier tagging for histograms soon
* session metrics could be tagged the same way (unlikely to happen for now, needs changes in Relay)

Here we move away from the purpose-built user satisfaction impl to the
more generic one

mypy.ini

@@ -59,6 +59,7 @@ files = src/sentry/analytics/,
         src/sentry/processing/realtime_metrics/,
         src/sentry/profiles/,
         src/sentry/ratelimits/,
+        src/sentry/relay/config/metric_extraction.py,
         src/sentry/release_health/,
         src/sentry/roles/manager.py,
         src/sentry/rules/,

src/sentry/relay/config.py → src/sentry/relay/config/__init__.py

@@ -1,13 +1,12 @@
 import logging
 import uuid
 from datetime import datetime
-from typing import Any, List, Mapping, Optional, TypedDict
+from typing import Any, Mapping, Optional, Sequence
 
 from pytz import utc
 from sentry_sdk import Hub, capture_exception
 
 from sentry import features, quotas, utils
-from sentry.api.endpoints.project_transaction_threshold import DEFAULT_THRESHOLD
 from sentry.constants import ObjectStatus
 from sentry.datascrubbing import get_datascrubbing_settings, get_pii_config
 from sentry.grouping.api import get_grouping_config_dict_for_project
@@ -19,7 +18,7 @@
 )
 from sentry.interfaces.security import DEFAULT_DISALLOWED_SOURCES
 from sentry.models import Project
-from sentry.models.transaction_threshold import TRANSACTION_METRICS as TRANSACTION_THRESHOLD_KEYS
+from sentry.relay.config.metric_extraction import get_metric_conditional_tagging_rules
 from sentry.relay.utils import to_camel_case_name
 from sentry.utils import metrics
 from sentry.utils.http import get_origins
@@ -34,7 +33,7 @@
 logger = logging.getLogger(__name__)
 
 
-def get_exposed_features(project: Project) -> List[str]:
+def get_exposed_features(project: Project) -> Sequence[str]:
 
     active_features = []
     for feature in EXPOSABLE_FEATURES:
@@ -190,6 +189,18 @@ def get_project_config(project, full_config=True, project_keys=None):
         cfg["config"]["transactionMetrics"] = get_transaction_metrics_settings(
             project, cfg["config"].get("breakdownsV2")
         )
+
+        # This config key is technically not specific to _transaction_ metrics,
+        # is however currently both only applied to transaction metrics in
+        # Relay, and only used to tag transaction metrics in Sentry.
+        try:
+            cfg["config"]["metricConditionalTagging"] = get_metric_conditional_tagging_rules(
+                project
+            )
+        except Exception:
+            capture_exception()
+            raise
+
     if features.has("projects:performance-suspect-spans-ingestion", project=project):
         cfg["config"]["spanAttributes"] = project.get_option("sentry:span_attributes")
     with Hub.current.start_span(op="get_filter_settings"):
@@ -402,50 +413,6 @@ def _filter_option_to_config_setting(flt, setting):
 )
 
 
-class _TransactionThreshold(TypedDict):
-    metric: Optional[str]  # Either 'duration' or 'lcp'
-    threshold: float
-
-
-class _TransactionThresholdConfig(TypedDict):
-    #: The project-wide threshold to apply (see `ProjectTransactionThreshold`)
-    projectThreshold: _TransactionThreshold
-    #: Transaction-specific overrides of the project-wide threshold
-    #: (see `ProjectTransactionThresholdOverride`)
-    transactionThresholds: Mapping[str, _TransactionThreshold]
-
-
-def _get_satisfaction_thresholds(project: Project) -> _TransactionThresholdConfig:
-    # Always start with the default threshold, so we do not have to maintain
-    # A separate default in Relay
-    project_threshold: _TransactionThreshold = {
-        "metric": DEFAULT_THRESHOLD["metric"],
-        "threshold": float(DEFAULT_THRESHOLD["threshold"]),
-    }
-
-    # Apply custom project threshold
-    for i, threshold in enumerate(project.projecttransactionthreshold_set.all()):
-        if i > 0:
-            logger.error("More than one transaction threshold exists for project")
-            break
-        project_threshold = {
-            "metric": TRANSACTION_THRESHOLD_KEYS[threshold.metric],
-            "threshold": threshold.threshold,
-        }
-
-    # Apply transaction-specific override
-    return {
-        "projectThreshold": project_threshold,
-        "transactionThresholds": {
-            threshold.transaction: {
-                "metric": TRANSACTION_THRESHOLD_KEYS[threshold.metric],
-                "threshold": threshold.threshold,
-            }
-            for threshold in project.projecttransactionthresholdoverride_set.all()
-        },
-    }
-
-
 def get_transaction_metrics_settings(
     project: Project, breakdowns_config: Optional[Mapping[str, Any]]
 ):
@@ -485,5 +452,4 @@ def get_transaction_metrics_settings(
     return {
         "extractMetrics": metrics,
         "extractCustomTags": custom_tags,
-        "satisfactionThresholds": _get_satisfaction_thresholds(project),
     }

src/sentry/relay/config/metric_extraction.py

@@ -0,0 +1,130 @@
+from dataclasses import dataclass
+from typing import Any, List, Literal, Sequence, TypedDict, Union
+
+from sentry.api.endpoints.project_transaction_threshold import DEFAULT_THRESHOLD
+from sentry.models import (
+    Project,
+    ProjectTransactionThreshold,
+    ProjectTransactionThresholdOverride,
+    TransactionMetric,
+)
+
+
+class RuleConditionInner(TypedDict):
+    op: Literal["eq", "gt"]
+    name: str
+    value: Any
+
+
+# mypy does not support recursive types. type definition is a very small subset
+# of the values relay actually accepts
+class RuleCondition(TypedDict):
+    op: Literal["and"]
+    inner: Sequence[RuleConditionInner]
+
+
+class MetricConditionalTaggingRule(TypedDict):
+    condition: RuleCondition
+    targetMetrics: Sequence[str]
+    targetTag: str
+    tagValue: str
+
+
+_TRANSACTION_METRICS_TO_RULE_FIELD = {
+    TransactionMetric.LCP.value: "transaction.measurements.lcp",
+    TransactionMetric.DURATION.value: "transaction.duration",
+}
+
+_SATISFACTION_TARGET_METRICS = frozenset(
+    [
+        "s:transactions/user@none",
+        "d:transactions/duration@millisecond",
+    ]
+)
+
+_SATISFACTION_TARGET_TAG = "satisfaction"
+
+
+@dataclass
+class _DefaultThreshold:
+    metric: TransactionMetric
+    threshold: int
+
+
+_DEFAULT_THRESHOLD = _DefaultThreshold(
+    metric=TransactionMetric[DEFAULT_THRESHOLD["metric"].upper()].value,
+    threshold=int(DEFAULT_THRESHOLD["threshold"]),
+)
+
+
+def get_metric_conditional_tagging_rules(
+    project: Project,
+) -> Sequence[MetricConditionalTaggingRule]:
+    rules: List[MetricConditionalTaggingRule] = []
+
+    for threshold in project.projecttransactionthresholdoverride_set.all():
+        rules.extend(
+            _threshold_to_rules(
+                threshold,
+                [{"op": "eq", "name": "event.transaction", "value": threshold.transaction}],
+            )
+        )
+
+    try:
+        threshold = ProjectTransactionThreshold.objects.get(project=project)
+        rules.extend(_threshold_to_rules(threshold, []))
+    except ProjectTransactionThreshold.DoesNotExist:
+        rules.extend(_threshold_to_rules(_DEFAULT_THRESHOLD, []))
+        pass
+
+    return rules
+
+
+def _threshold_to_rules(
+    threshold: Union[
+        ProjectTransactionThreshold, ProjectTransactionThresholdOverride, _DefaultThreshold
+    ],
+    extra_conditions: Sequence[RuleConditionInner],
+) -> Sequence[MetricConditionalTaggingRule]:
+    frustrated: MetricConditionalTaggingRule = {
+        "condition": {
+            "op": "and",
+            "inner": [
+                {
+                    "op": "gt",
+                    "name": _TRANSACTION_METRICS_TO_RULE_FIELD[threshold.metric],
+                    # The frustration threshold is always four times the threshold
+                    # (see https://docs.sentry.io/product/performance/metrics/#apdex)
+                    "value": threshold.threshold * 4,
+                },
+                *extra_conditions,
+            ],
+        },
+        "targetMetrics": list(_SATISFACTION_TARGET_METRICS),
+        "targetTag": _SATISFACTION_TARGET_TAG,
+        "tagValue": "frustrated",
+    }
+    tolerated: MetricConditionalTaggingRule = {
+        "condition": {
+            "op": "and",
+            "inner": [
+                {
+                    "op": "gt",
+                    "name": _TRANSACTION_METRICS_TO_RULE_FIELD[threshold.metric],
+                    "value": threshold.threshold,
+                },
+                *extra_conditions,
+            ],
+        },
+        "targetMetrics": list(_SATISFACTION_TARGET_METRICS),
+        "targetTag": _SATISFACTION_TARGET_TAG,
+        "tagValue": "tolerated",
+    }
+    satisfied: MetricConditionalTaggingRule = {
+        "condition": {"op": "and", "inner": list(extra_conditions)},
+        "targetMetrics": list(_SATISFACTION_TARGET_METRICS),
+        "targetTag": _SATISFACTION_TARGET_TAG,
+        "tagValue": "satisfied",
+    }
+
+    return [frustrated, tolerated, satisfied]

tests/sentry/relay/snapshots/test_config/test_project_config_satisfaction_thresholds/False/False.pysnap

@@ -1,9 +1,35 @@
 ---
-created: '2022-02-28T13:20:49.317250Z'
+created: '2022-04-19T12:36:49.105349Z'
 creator: sentry
 source: tests/sentry/relay/test_config.py
 ---
-projectThreshold:
-  metric: duration
-  threshold: 300.0
-transactionThresholds: {}
+- condition:
+    inner:
+    - name: transaction.duration
+      op: gt
+      value: 1200
+    op: and
+  tagValue: frustrated
+  targetMetrics:
+  - s:transactions/user@none
+  - d:transactions/duration@millisecond
+  targetTag: satisfaction
+- condition:
+    inner:
+    - name: transaction.duration
+      op: gt
+      value: 300
+    op: and
+  tagValue: tolerated
+  targetMetrics:
+  - s:transactions/user@none
+  - d:transactions/duration@millisecond
+  targetTag: satisfaction
+- condition:
+    inner: []
+    op: and
+  tagValue: satisfied
+  targetMetrics:
+  - s:transactions/user@none
+  - d:transactions/duration@millisecond
+  targetTag: satisfaction

tests/sentry/relay/snapshots/test_config/test_project_config_satisfaction_thresholds/False/True.pysnap

@@ -1,9 +1,35 @@
 ---
-created: '2022-02-28T13:20:49.410307Z'
+created: '2022-04-19T12:33:09.203815Z'
 creator: sentry
 source: tests/sentry/relay/test_config.py
 ---
-projectThreshold:
-  metric: lcp
-  threshold: 500
-transactionThresholds: {}
+- condition:
+    inner:
+    - name: transaction.measurements.lcp
+      op: gt
+      value: 2000
+    op: and
+  tagValue: frustrated
+  targetMetrics:
+  - s:transactions/user@none
+  - d:transactions/duration@millisecond
+  targetTag: satisfaction
+- condition:
+    inner:
+    - name: transaction.measurements.lcp
+      op: gt
+      value: 500
+    op: and
+  tagValue: tolerated
+  targetMetrics:
+  - s:transactions/user@none
+  - d:transactions/duration@millisecond
+  targetTag: satisfaction
+- condition:
+    inner: []
+    op: and
+  tagValue: satisfied
+  targetMetrics:
+  - s:transactions/user@none
+  - d:transactions/duration@millisecond
+  targetTag: satisfaction