diff --git a/src/sentry/ingest/transaction_clusterer/datasource/redis.py b/src/sentry/ingest/transaction_clusterer/datasource/redis.py index 3f146f9a09001d..542a86edca13cc 100644 --- a/src/sentry/ingest/transaction_clusterer/datasource/redis.py +++ b/src/sentry/ingest/transaction_clusterer/datasource/redis.py @@ -1,9 +1,10 @@ """Write transactions into redis sets""" import logging -from collections.abc import Callable, Iterator, Mapping +from collections.abc import Callable, Iterator, Mapping, Sequence from typing import Any +import orjson import sentry_sdk from django.conf import settings from rediscluster import RedisCluster @@ -133,6 +134,12 @@ def record_segment_name(project: Project, segment_span: CompatibleSpan) -> None: project, segment_name, ) + if in_random_rollout("txnames.bump-lifetime-sample-rate"): + safe_execute( + _bump_rule_lifetime_for_segment, + project, + segment_span, + ) def _should_store_transaction_name(event_data: Mapping[str, Any]) -> str | None: @@ -181,9 +188,22 @@ def _should_store_segment_name_inner( def _bump_rule_lifetime(project: Project, event_data: Mapping[str, Any]) -> None: + applied_rules = event_data.get("_meta", {}).get("transaction", {}).get("", {}).get("rem", {}) + _bump_rule_lifetime_inner(project, applied_rules) + + +def _bump_rule_lifetime_for_segment(project: Project, segment_span: CompatibleSpan) -> None: + meta_str: str = attribute_value( + segment_span, f"sentry._meta.fields.attributes.{ATTRIBUTE_NAMES.SENTRY_SEGMENT_NAME}" + ) + meta = orjson.loads(meta_str) + applied_rules = meta.get("meta", {}).get("", {}).get("rem", []) + _bump_rule_lifetime_inner(project, applied_rules) + + +def _bump_rule_lifetime_inner(project: Project, applied_rules: Sequence): from sentry.ingest.transaction_clusterer import rules as clusterer_rules - applied_rules = event_data.get("_meta", {}).get("transaction", {}).get("", {}).get("rem", {}) if not applied_rules: return diff --git a/tests/sentry/ingest/test_transaction_clusterer.py b/tests/sentry/ingest/test_transaction_clusterer.py index df26b28472ebb1..bc341ec1bf6831 100644 --- a/tests/sentry/ingest/test_transaction_clusterer.py +++ b/tests/sentry/ingest/test_transaction_clusterer.py @@ -1,5 +1,6 @@ from unittest import mock +import orjson import pytest from sentry_conventions.attributes import ATTRIBUTE_NAMES @@ -554,6 +555,66 @@ def test_transaction_clusterer_bumps_rules(_, default_organization) -> None: assert get_rules(ClustererNamespace.TRANSACTIONS, project1) == {"/user/*/**": 2} +@mock.patch("sentry.ingest.transaction_clusterer.datasource.redis.MAX_SET_SIZE", 10) +@mock.patch("sentry.ingest.transaction_clusterer.tasks.MERGE_THRESHOLD", 5) +@mock.patch( + "sentry.ingest.transaction_clusterer.tasks.cluster_projects.delay", + wraps=cluster_projects, # call immediately +) +@django_db_all +def test_segment_clusterer_bumps_rules(_, default_organization) -> None: + project1 = Project(id=123, name="project1", organization_id=default_organization.id) + project1.save() + + with override_options({"txnames.bump-lifetime-sample-rate": 1.0}): + for i in range(10): + _record_sample( + ClustererNamespace.TRANSACTIONS, project1, f"/user/tx-{project1.name}-{i}/settings" + ) + + with mock.patch("sentry.ingest.transaction_clusterer.rules._now", lambda: 1): + spawn_clusterers() + + assert get_rules(ClustererNamespace.TRANSACTIONS, project1) == {"/user/*/**": 1} + + with mock.patch("sentry.ingest.transaction_clusterer.rules._now", lambda: 2): + record_segment_name( + project1, + { + "name": "/user/*/settings", + "attributes": { + ATTRIBUTE_NAMES.SENTRY_SPAN_SOURCE: { + "type": "string", + "value": "sanitized", + }, + f"sentry._meta.fields.attributes.{ATTRIBUTE_NAMES.SENTRY_SEGMENT_NAME}": { + "type": "string", + "value": orjson.dumps( + { + "meta": { + "": { + "rem": [["int", "s", 0, 0], ["/user/*/**", "s"]], + "val": "/user/tx-project1-pi/settings", + } + } + } + ).decode(), + }, + }, + }, # type: ignore[typeddict-item] + ) + + # _get_rules fetches from project options, which arent updated yet. + assert get_redis_rules(ClustererNamespace.TRANSACTIONS, project1) == {"/user/*/**": 2} + assert get_rules(ClustererNamespace.TRANSACTIONS, project1) == {"/user/*/**": 1} + # Update rules to update the project option storage. + with mock.patch("sentry.ingest.transaction_clusterer.rules._now", lambda: 3): + assert 0 == update_rules(ClustererNamespace.TRANSACTIONS, project1, []) + # After project options are updated, the last_seen should also be updated. + assert get_redis_rules(ClustererNamespace.TRANSACTIONS, project1) == {"/user/*/**": 2} + assert get_rules(ClustererNamespace.TRANSACTIONS, project1) == {"/user/*/**": 2} + + @mock.patch("sentry.ingest.transaction_clusterer.datasource.redis.MAX_SET_SIZE", 3) @mock.patch("sentry.ingest.transaction_clusterer.tasks.MERGE_THRESHOLD", 2) @mock.patch(