Skip to content

Commit 43d7c41

Browse files
authored
ref(transactions): add functionality to sample transactions in save_event_transaction (#81077)
first PR of #81065 no behavior changes until option is turned on. need to change post_process as well before we can turn the option on. equivalent of https://github.com/getsentry/sentry/blob/b84d3d9238ab75822b076d16de1111c3035fe073/src/sentry/tasks/post_process.py#L614 in post_process
1 parent c1f0132 commit 43d7c41

File tree

7 files changed

+301
-0
lines changed

7 files changed

+301
-0
lines changed

src/sentry/event_manager.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
from sentry.eventtypes import EventType
4848
from sentry.eventtypes.transaction import TransactionEvent
4949
from sentry.exceptions import HashDiscarded
50+
from sentry.features.rollout import in_rollout_group
5051
from sentry.grouping.api import (
5152
NULL_GROUPHASH_INFO,
5253
GroupHashInfo,
@@ -70,6 +71,9 @@
7071
)
7172
from sentry.grouping.variants import BaseVariant
7273
from sentry.ingest.inbound_filters import FilterStatKeys
74+
from sentry.ingest.transaction_clusterer.datasource.redis import (
75+
record_transaction_name as record_transaction_name_for_clustering,
76+
)
7377
from sentry.integrations.tasks.kick_off_status_syncs import kick_off_status_syncs
7478
from sentry.issues.grouptype import ErrorGroupType
7579
from sentry.issues.issue_occurrence import IssueOccurrence
@@ -99,6 +103,8 @@
99103
from sentry.net.http import connection_from_url
100104
from sentry.plugins.base import plugins
101105
from sentry.quotas.base import index_data_category
106+
from sentry.receivers.features import record_event_processed
107+
from sentry.receivers.onboarding import record_release_received, record_user_context_received
102108
from sentry.reprocessing2 import is_reprocessed_event
103109
from sentry.seer.signed_seer_api import make_signed_seer_api_request
104110
from sentry.signals import (
@@ -2512,6 +2518,34 @@ def _detect_performance_problems(
25122518
)
25132519

25142520

2521+
@sentry_sdk.tracing.trace
2522+
def _record_transaction_info(jobs: Sequence[Job], projects: ProjectsMapping) -> None:
2523+
"""
2524+
this function does what we do in post_process for transactions. if this option is
2525+
turned on, we do the actions here instead of in post_process, with the goal
2526+
eventually being to not run transactions through post_process
2527+
"""
2528+
for job in jobs:
2529+
try:
2530+
event = job["event"]
2531+
if not in_rollout_group("transactions.do_post_process_in_save", event.event_id):
2532+
continue
2533+
2534+
project = event.project
2535+
with sentry_sdk.start_span(op="event_manager.record_transaction_name_for_clustering"):
2536+
record_transaction_name_for_clustering(project, event.data)
2537+
2538+
# these are what the "transaction_processed" signal hooked into
2539+
# we should not use signals here, so call the recievers directly
2540+
# instead of sending a signal. we should consider potentially
2541+
# deleting these
2542+
record_event_processed(project, event)
2543+
record_user_context_received(project, event)
2544+
record_release_received(project, event)
2545+
except Exception:
2546+
sentry_sdk.capture_exception()
2547+
2548+
25152549
class PerformanceJob(TypedDict, total=False):
25162550
performance_problems: Sequence[PerformanceProblem]
25172551
event: Event
@@ -2637,6 +2671,9 @@ def save_transaction_events(jobs: Sequence[Job], projects: ProjectsMapping) -> S
26372671
with metrics.timer("save_transaction_events.send_occurrence_to_platform"):
26382672
_send_occurrence_to_platform(jobs, projects)
26392673

2674+
with metrics.timer("save_transaction_events.record_transaction_info"):
2675+
_record_transaction_info(jobs, projects)
2676+
26402677
return jobs
26412678

26422679

src/sentry/options/defaults.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,3 +2920,9 @@
29202920
default=0.0,
29212921
flags=FLAG_AUTOMATOR_MODIFIABLE,
29222922
)
2923+
2924+
register(
2925+
"transactions.do_post_process_in_save",
2926+
default=0.0,
2927+
flags=FLAG_AUTOMATOR_MODIFIABLE | FLAG_RATE,
2928+
)

src/sentry/tasks/post_process.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from sentry import features, projectoptions
1717
from sentry.eventstream.types import EventStreamEventType
1818
from sentry.exceptions import PluginError
19+
from sentry.features.rollout import in_rollout_group
1920
from sentry.issues.grouptype import GroupCategory
2021
from sentry.issues.issue_occurrence import IssueOccurrence
2122
from sentry.killswitches import killswitch_matches_context
@@ -480,6 +481,17 @@ def should_update_escalating_metrics(event: Event, is_transaction_event: bool) -
480481
)
481482

482483

484+
def _get_event_id_from_cache_key(cache_key: str) -> str | None:
485+
"""
486+
format is "e:{}:{}",event_id,project_id
487+
"""
488+
489+
try:
490+
return cache_key.split(":")[1]
491+
except IndexError:
492+
return None
493+
494+
483495
@instrumented_task(
484496
name="sentry.tasks.post_process.post_process_group",
485497
time_limit=120,
@@ -527,6 +539,18 @@ def post_process_group(
527539
# need to rewind history.
528540
data = processing_store.get(cache_key)
529541
if not data:
542+
event_id = _get_event_id_from_cache_key(cache_key)
543+
if event_id:
544+
if in_rollout_group(
545+
"transactions.do_post_process_in_save",
546+
event_id,
547+
):
548+
# if we're doing the work for transactions in save_event_transaction
549+
# instead of here, this is expected, so simply increment a metric
550+
# instead of logging
551+
metrics.incr("post_process.skipped_do_post_process_in_save")
552+
return
553+
530554
logger.info(
531555
"post_process.skipped",
532556
extra={"cache_key": cache_key, "reason": "missing_cache"},

src/sentry/tasks/store.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from sentry.constants import DEFAULT_STORE_NORMALIZER_ARGS
1717
from sentry.datascrubbing import scrub_data
1818
from sentry.eventstore import processing
19+
from sentry.features.rollout import in_rollout_group
1920
from sentry.feedback.usecases.create_feedback import FeedbackCreationSource, create_feedback_issue
2021
from sentry.ingest.types import ConsumerType
2122
from sentry.killswitches import killswitch_matches_context
@@ -582,6 +583,16 @@ def _do_save_event(
582583
raise
583584

584585
finally:
586+
if (
587+
consumer_type == ConsumerType.Transactions
588+
and event_id
589+
and in_rollout_group("transactions.do_post_process_in_save", event_id)
590+
):
591+
# we won't use the transaction data in post_process
592+
# so we can delete it from the cache now.
593+
if cache_key:
594+
processing_store.delete_by_key(cache_key)
595+
585596
reprocessing2.mark_event_reprocessed(data)
586597
if cache_key and has_attachments:
587598
attachment_cache.delete(cache_key)

tests/sentry/event_manager/test_event_manager.py

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
from sentry.grouping.api import load_grouping_config
5050
from sentry.grouping.utils import hash_from_values
5151
from sentry.ingest.inbound_filters import FilterStatKeys
52+
from sentry.ingest.transaction_clusterer import ClustererNamespace
5253
from sentry.integrations.models.external_issue import ExternalIssue
5354
from sentry.integrations.models.integration import Integration
5455
from sentry.issues.grouptype import (
@@ -1540,6 +1541,140 @@ def test_transaction_event_span_grouping(self) -> None:
15401541
# the basic strategy is to simply use the description
15411542
assert spans == [{"hash": hash_values([span["description"]])} for span in data["spans"]]
15421543

1544+
@override_options({"transactions.do_post_process_in_save": 1.0})
1545+
def test_transaction_sampler_and_receive(self) -> None:
1546+
# make sure with the option on we don't get any errors
1547+
manager = EventManager(
1548+
make_event(
1549+
**{
1550+
"transaction": "wait",
1551+
"contexts": {
1552+
"trace": {
1553+
"parent_span_id": "bce14471e0e9654d",
1554+
"op": "foobar",
1555+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1556+
"span_id": "bf5be759039ede9a",
1557+
}
1558+
},
1559+
"spans": [
1560+
{
1561+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1562+
"parent_span_id": "bf5be759039ede9a",
1563+
"span_id": "a" * 16,
1564+
"start_timestamp": 0,
1565+
"timestamp": 1,
1566+
"same_process_as_parent": True,
1567+
"op": "default",
1568+
"description": "span a",
1569+
},
1570+
{
1571+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1572+
"parent_span_id": "bf5be759039ede9a",
1573+
"span_id": "b" * 16,
1574+
"start_timestamp": 0,
1575+
"timestamp": 1,
1576+
"same_process_as_parent": True,
1577+
"op": "default",
1578+
"description": "span a",
1579+
},
1580+
{
1581+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1582+
"parent_span_id": "bf5be759039ede9a",
1583+
"span_id": "c" * 16,
1584+
"start_timestamp": 0,
1585+
"timestamp": 1,
1586+
"same_process_as_parent": True,
1587+
"op": "default",
1588+
"description": "span b",
1589+
},
1590+
],
1591+
"timestamp": "2019-06-14T14:01:40Z",
1592+
"start_timestamp": "2019-06-14T14:01:40Z",
1593+
"type": "transaction",
1594+
"transaction_info": {
1595+
"source": "url",
1596+
},
1597+
}
1598+
)
1599+
)
1600+
manager.normalize()
1601+
manager.save(self.project.id)
1602+
1603+
@override_options({"transactions.do_post_process_in_save": 1.0})
1604+
@patch("sentry.event_manager.record_event_processed")
1605+
@patch("sentry.event_manager.record_user_context_received")
1606+
@patch("sentry.event_manager.record_release_received")
1607+
@patch("sentry.ingest.transaction_clusterer.datasource.redis._record_sample")
1608+
def test_transaction_sampler_and_receive_mock_called(
1609+
self,
1610+
mock_record_sample: mock.MagicMock,
1611+
mock_record_release: mock.MagicMock,
1612+
mock_record_user: mock.MagicMock,
1613+
mock_record_event: mock.MagicMock,
1614+
) -> None:
1615+
manager = EventManager(
1616+
make_event(
1617+
**{
1618+
"transaction": "wait",
1619+
"contexts": {
1620+
"trace": {
1621+
"parent_span_id": "bce14471e0e9654d",
1622+
"op": "foobar",
1623+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1624+
"span_id": "bf5be759039ede9a",
1625+
}
1626+
},
1627+
"spans": [
1628+
{
1629+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1630+
"parent_span_id": "bf5be759039ede9a",
1631+
"span_id": "a" * 16,
1632+
"start_timestamp": 0,
1633+
"timestamp": 1,
1634+
"same_process_as_parent": True,
1635+
"op": "default",
1636+
"description": "span a",
1637+
},
1638+
{
1639+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1640+
"parent_span_id": "bf5be759039ede9a",
1641+
"span_id": "b" * 16,
1642+
"start_timestamp": 0,
1643+
"timestamp": 1,
1644+
"same_process_as_parent": True,
1645+
"op": "default",
1646+
"description": "span a",
1647+
},
1648+
{
1649+
"trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
1650+
"parent_span_id": "bf5be759039ede9a",
1651+
"span_id": "c" * 16,
1652+
"start_timestamp": 0,
1653+
"timestamp": 1,
1654+
"same_process_as_parent": True,
1655+
"op": "default",
1656+
"description": "span b",
1657+
},
1658+
],
1659+
"timestamp": "2019-06-14T14:01:40Z",
1660+
"start_timestamp": "2019-06-14T14:01:40Z",
1661+
"type": "transaction",
1662+
"transaction_info": {
1663+
"source": "url",
1664+
},
1665+
}
1666+
)
1667+
)
1668+
manager.normalize()
1669+
event = manager.save(self.project.id)
1670+
1671+
mock_record_event.assert_called_once_with(self.project, event)
1672+
mock_record_user.assert_called_once_with(self.project, event)
1673+
mock_record_release.assert_called_once_with(self.project, event)
1674+
assert mock_record_sample.mock_calls == [
1675+
mock.call(ClustererNamespace.TRANSACTIONS, self.project, "wait")
1676+
]
1677+
15431678
def test_sdk(self) -> None:
15441679
manager = EventManager(make_event(**{"sdk": {"name": "sentry-unity", "version": "1.0"}}))
15451680
manager.normalize()

tests/sentry/tasks/test_post_process.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
from sentry.tasks.post_process import (
6161
HIGHER_ISSUE_OWNERS_PER_PROJECT_PER_MIN_RATELIMIT,
6262
ISSUE_OWNERS_PER_PROJECT_PER_MIN_RATELIMIT,
63+
_get_event_id_from_cache_key,
6364
feedback_filter_decorator,
6465
locks,
6566
post_process_group,
@@ -2735,6 +2736,60 @@ def test_process_transaction_event_clusterer(
27352736
]
27362737

27372738

2739+
class ProcessingStoreTransactionEmptyTestcase(TestCase):
2740+
@patch("sentry.tasks.post_process.logger")
2741+
def test_logger_called_when_empty(self, mock_logger):
2742+
post_process_group(
2743+
is_new=False,
2744+
is_regression=False,
2745+
is_new_group_environment=False,
2746+
cache_key="e:1:2",
2747+
group_id=None,
2748+
project_id=self.project.id,
2749+
eventstream_type=EventStreamEventType.Transaction,
2750+
)
2751+
assert mock_logger.info.called
2752+
mock_logger.info.assert_called_with(
2753+
"post_process.skipped", extra={"cache_key": "e:1:2", "reason": "missing_cache"}
2754+
)
2755+
2756+
@patch("sentry.tasks.post_process.logger")
2757+
@patch("sentry.utils.metrics.incr")
2758+
@override_options({"transactions.do_post_process_in_save": 1.0})
2759+
def test_logger_called_when_empty_option_on(self, mock_metric_incr, mock_logger):
2760+
post_process_group(
2761+
is_new=False,
2762+
is_regression=False,
2763+
is_new_group_environment=False,
2764+
cache_key="e:1:2",
2765+
group_id=None,
2766+
project_id=self.project.id,
2767+
eventstream_type=EventStreamEventType.Transaction,
2768+
)
2769+
assert not mock_logger.info.called
2770+
mock_metric_incr.assert_called_with("post_process.skipped_do_post_process_in_save")
2771+
2772+
@patch("sentry.tasks.post_process.logger")
2773+
@override_options({"transactions.do_post_process_in_save": 1.0})
2774+
def test_logger_called_when_empty_option_on_invalid_cache_key(self, mock_logger):
2775+
post_process_group(
2776+
is_new=False,
2777+
is_regression=False,
2778+
is_new_group_environment=False,
2779+
cache_key="invalidhehe",
2780+
group_id=None,
2781+
project_id=self.project.id,
2782+
eventstream_type=EventStreamEventType.Transaction,
2783+
)
2784+
mock_logger.info.assert_called_with(
2785+
"post_process.skipped", extra={"cache_key": "invalidhehe", "reason": "missing_cache"}
2786+
)
2787+
2788+
def test_get_event_id_from_cache_key(self):
2789+
assert _get_event_id_from_cache_key("e:1:2") == "1"
2790+
assert _get_event_id_from_cache_key("invalid") is None
2791+
2792+
27382793
class PostProcessGroupGenericTest(
27392794
TestCase,
27402795
SnubaTestCase,

0 commit comments

Comments
 (0)