Skip to content

Commit

Permalink
fix(txnames): Prevent recursion error (#59421)
Browse files Browse the repository at this point in the history
Prevent recursion errors in transaction clustering. This only happens
for very deep trees, which do not make good transaction names or span
descriptions in the first place.

Fixes [SENTRY-170T](https://sentry.sentry.io/issues/4559413640/)
  • Loading branch information
jjbayer committed Nov 7, 2023
1 parent 5f93df7 commit c857d91
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/sentry/ingest/transaction_clusterer/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ class Merged:
#: Separator by which we build the tree
SEP = "/"

#: Maximum tree depth. URLs with more than 200 segments do not seem useful,
#: and we occasionally run into `RecursionError`s.
MAX_DEPTH = 200


logger = logging.getLogger(__name__)

Expand All @@ -80,7 +84,7 @@ def __init__(self, *, merge_threshold: int) -> None:

def add_input(self, strings: Iterable[str]) -> None:
for string in strings:
parts = string.split(SEP)
parts = string.split(SEP, maxsplit=MAX_DEPTH)
node = self._tree
for part in parts:
node = node.setdefault(part, Node())
Expand Down
11 changes: 11 additions & 0 deletions tests/sentry/ingest/test_transaction_clusterer.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ def test_single_leaf():
assert clusterer.get_rules() == ["/a/*/**"]


def test_deep_tree():
clusterer = TreeClusterer(merge_threshold=1)
transaction_names = [
1001 * "/.",
]
clusterer.add_input(transaction_names)

# Does not throw an exception:
clusterer.get_rules()


@mock.patch("sentry.ingest.transaction_clusterer.datasource.redis.MAX_SET_SIZE", 5)
def test_collection():
org = Organization(pk=666)
Expand Down

0 comments on commit c857d91

Please sign in to comment.