feat(txnames): Include transactions without source in clustering (#51437)

When an SDK does not provide a [transaction
source](https://develop.sentry.dev/sdk/event-payloads/transaction/#transaction-annotations)
annotation, Relay does [one of two
things](https://github.com/getsentry/relay/blob/ab54df17d285a2210203e3a41dcf0373d7c6a3d2/relay-general/src/store/transactions/processor.rs#L369-L374):

1. When the transaction name is expected to contain identifiers
(high-cardinality), leave the source empty.
2. When the transaction name is expected to be low-cardinality, set it
to `unknown`.

In the first case, we want to run the
[clusterer](https://develop.sentry.dev/transaction-clustering/) to
detect high-cardinality patterns. We have to make the simplifying
assumption that a transaction name containing `/` is a URL.

Note: This PR includes `source:null` transactions in the clustering
input. To also apply the discovered rules in Relay, we need another
change in https://github.com/getsentry/relay/.

Author: jjbayer

src/sentry/ingest/transaction_clusterer/datasource/redis.py

@@ -101,9 +101,7 @@ def clear_samples(namespace: ClustererNamespace, project: Project) -> None:
 
 
 def record_transaction_name(project: Project, event_data: Mapping[str, Any], **kwargs: Any) -> None:
-    transaction_name = event_data.get("transaction")
-
-    if transaction_name and _should_store_transaction_name(event_data):
+    if transaction_name := _should_store_transaction_name(event_data):
         safe_execute(
             _record_sample,
             ClustererNamespace.TRANSACTIONS,
@@ -116,28 +114,41 @@ def record_transaction_name(project: Project, event_data: Mapping[str, Any], **k
             safe_execute(_bump_rule_lifetime, project, event_data, _with_transaction=False)
 
 
-def _should_store_transaction_name(event_data: Mapping[str, Any]) -> bool:
+def _should_store_transaction_name(event_data: Mapping[str, Any]) -> Optional[str]:
     """Returns whether the given event must be stored as input for the
     transaction clusterer."""
-    tags = event_data.get("tags")
+    transaction_name = event_data.get("transaction")
+    if not transaction_name:
+        return None
+
+    tags = event_data.get("tags") or {}
     transaction_info = event_data.get("transaction_info") or {}
     source = transaction_info.get("source")
 
-    # For now, we also feed back transactions into the clustering algorithm
+    # We also feed back transactions into the clustering algorithm
     # that have already been sanitized, so we have a chance to discover
     # more high cardinality segments after partial sanitation.
     # For example, we may have sanitized `/orgs/*/projects/foo`,
     # But the clusterer has yet to discover `/orgs/*/projects/*`.
     #
     # Disadvantage: the load on redis does not decrease over time.
     #
-    if source not in (TRANSACTION_SOURCE_URL, TRANSACTION_SOURCE_SANITIZED):
-        return False
+    source_matches = source in (TRANSACTION_SOURCE_URL, TRANSACTION_SOURCE_SANITIZED) or (
+        # Relay leaves source None if it expects it to be high cardinality, (otherwise it sets it to "unknown")
+        # (see https://github.com/getsentry/relay/blob/2d07bef86415cc0ae8af01d16baecde10cdb23a6/relay-general/src/store/transactions/processor.rs#L369-L373).
+        #
+        # Our data shows that a majority of these `None` source transactions contain slashes, so treat them as URL transactions:
+        source is None
+        and "/" in transaction_name
+    )
+
+    if not source_matches:
+        return None
 
     if tags and HTTP_404_TAG in tags:
-        return False
+        return None
 
-    return True
+    return transaction_name
 
 
 def _bump_rule_lifetime(project: Project, event_data: Mapping[str, Any]) -> None:

tests/sentry/ingest/test_transaction_clusterer.py

@@ -155,6 +155,8 @@ def test_distribution():
         ("route", "/", [["transaction", "/"]], 0),
         ("url", None, [], 0),
         ("url", "/a/b/c", [["http.status_code", "404"]], 0),
+        (None, "/a/b/c", [], 1),
+        (None, "foo", [], 0),
     ],
 )
 def test_record_transactions(mocked_record, default_organization, source, txname, tags, expected):