Skip to content

Commit

Permalink
ref(transactions): remove option for skipping transactions post proce…
Browse files Browse the repository at this point in the history
…ss in ingest (#83105)

This option has been turned on in all regions for over a month now, we
can delete it and the associated checks in the code.
first step of cleanups for
#81065
  • Loading branch information
JoshFerge authored Jan 8, 2025
1 parent cf172ef commit c09e99d
Show file tree
Hide file tree
Showing 7 changed files with 1 addition and 126 deletions.
3 changes: 0 additions & 3 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
from sentry.eventtypes import EventType
from sentry.eventtypes.transaction import TransactionEvent
from sentry.exceptions import HashDiscarded
from sentry.features.rollout import in_rollout_group
from sentry.grouping.api import (
NULL_GROUPHASH_INFO,
GroupHashInfo,
Expand Down Expand Up @@ -2529,8 +2528,6 @@ def _record_transaction_info(jobs: Sequence[Job], projects: ProjectsMapping) ->
for job in jobs:
try:
event = job["event"]
if not in_rollout_group("transactions.do_post_process_in_save", event.event_id):
continue

project = event.project
with sentry_sdk.start_span(op="event_manager.record_transaction_name_for_clustering"):
Expand Down
5 changes: 0 additions & 5 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -2901,11 +2901,6 @@
default=0.0,
flags=FLAG_AUTOMATOR_MODIFIABLE,
)
register(
"transactions.do_post_process_in_save",
default=0.0,
flags=FLAG_AUTOMATOR_MODIFIABLE | FLAG_RATE,
)

# allows us to disable indexing during maintenance events
register(
Expand Down
23 changes: 0 additions & 23 deletions src/sentry/tasks/post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from sentry import features, options, projectoptions
from sentry.eventstream.types import EventStreamEventType
from sentry.exceptions import PluginError
from sentry.features.rollout import in_rollout_group
from sentry.issues.grouptype import GroupCategory
from sentry.issues.issue_occurrence import IssueOccurrence
from sentry.killswitches import killswitch_matches_context
Expand Down Expand Up @@ -481,17 +480,6 @@ def should_update_escalating_metrics(event: Event, is_transaction_event: bool) -
)


def _get_event_id_from_cache_key(cache_key: str) -> str | None:
"""
format is "e:{}:{}",event_id,project_id
"""

try:
return cache_key.split(":")[1]
except IndexError:
return None


@instrumented_task(
name="sentry.tasks.post_process.post_process_group",
time_limit=120,
Expand Down Expand Up @@ -540,17 +528,6 @@ def post_process_group(
# need to rewind history.
data = processing_store.get(cache_key)
if not data:
event_id = _get_event_id_from_cache_key(cache_key)
if event_id:
if in_rollout_group(
"transactions.do_post_process_in_save",
event_id,
):
# if we're doing the work for transactions in save_event_transaction
# instead of here, this is expected, so simply increment a metric
# instead of logging
metrics.incr("post_process.skipped_do_post_process_in_save")
return

logger.info(
"post_process.skipped",
Expand Down
7 changes: 1 addition & 6 deletions src/sentry/tasks/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from sentry.constants import DEFAULT_STORE_NORMALIZER_ARGS
from sentry.datascrubbing import scrub_data
from sentry.eventstore import processing
from sentry.features.rollout import in_rollout_group
from sentry.feedback.usecases.create_feedback import FeedbackCreationSource, create_feedback_issue
from sentry.ingest.types import ConsumerType
from sentry.killswitches import killswitch_matches_context
Expand Down Expand Up @@ -584,11 +583,7 @@ def _do_save_event(
raise

finally:
if (
consumer_type == ConsumerType.Transactions
and event_id
and in_rollout_group("transactions.do_post_process_in_save", event_id)
):
if consumer_type == ConsumerType.Transactions and event_id:
# we won't use the transaction data in post_process
# so we can delete it from the cache now.
if cache_key:
Expand Down
2 changes: 0 additions & 2 deletions tests/sentry/event_manager/test_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,6 @@ def test_transaction_event_span_grouping(self) -> None:
# the basic strategy is to simply use the description
assert spans == [{"hash": hash_values([span["description"]])} for span in data["spans"]]

@override_options({"transactions.do_post_process_in_save": 1.0})
def test_transaction_sampler_and_receive(self) -> None:
# make sure with the option on we don't get any errors
manager = EventManager(
Expand Down Expand Up @@ -1578,7 +1577,6 @@ def test_transaction_sampler_and_receive(self) -> None:
manager.normalize()
manager.save(self.project.id)

@override_options({"transactions.do_post_process_in_save": 1.0})
@patch("sentry.event_manager.record_event_processed")
@patch("sentry.event_manager.record_user_context_received")
@patch("sentry.event_manager.record_release_received")
Expand Down
55 changes: 0 additions & 55 deletions tests/sentry/tasks/test_post_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
from sentry.tasks.post_process import (
HIGHER_ISSUE_OWNERS_PER_PROJECT_PER_MIN_RATELIMIT,
ISSUE_OWNERS_PER_PROJECT_PER_MIN_RATELIMIT,
_get_event_id_from_cache_key,
feedback_filter_decorator,
locks,
post_process_group,
Expand Down Expand Up @@ -2750,60 +2749,6 @@ def test_process_transaction_event_clusterer(
]


class ProcessingStoreTransactionEmptyTestcase(TestCase):
@patch("sentry.tasks.post_process.logger")
def test_logger_called_when_empty(self, mock_logger):
post_process_group(
is_new=False,
is_regression=False,
is_new_group_environment=False,
cache_key="e:1:2",
group_id=None,
project_id=self.project.id,
eventstream_type=EventStreamEventType.Transaction,
)
assert mock_logger.info.called
mock_logger.info.assert_called_with(
"post_process.skipped", extra={"cache_key": "e:1:2", "reason": "missing_cache"}
)

@patch("sentry.tasks.post_process.logger")
@patch("sentry.utils.metrics.incr")
@override_options({"transactions.do_post_process_in_save": 1.0})
def test_logger_called_when_empty_option_on(self, mock_metric_incr, mock_logger):
post_process_group(
is_new=False,
is_regression=False,
is_new_group_environment=False,
cache_key="e:1:2",
group_id=None,
project_id=self.project.id,
eventstream_type=EventStreamEventType.Transaction,
)
assert not mock_logger.info.called
mock_metric_incr.assert_called_with("post_process.skipped_do_post_process_in_save")

@patch("sentry.tasks.post_process.logger")
@override_options({"transactions.do_post_process_in_save": 1.0})
def test_logger_called_when_empty_option_on_invalid_cache_key(self, mock_logger):
post_process_group(
is_new=False,
is_regression=False,
is_new_group_environment=False,
cache_key="invalidhehe",
group_id=None,
project_id=self.project.id,
eventstream_type=EventStreamEventType.Transaction,
)
mock_logger.info.assert_called_with(
"post_process.skipped", extra={"cache_key": "invalidhehe", "reason": "missing_cache"}
)

def test_get_event_id_from_cache_key(self):
assert _get_event_id_from_cache_key("e:1:2") == "1"
assert _get_event_id_from_cache_key("invalid") is None


class PostProcessGroupGenericTest(
TestCase,
SnubaTestCase,
Expand Down
32 changes: 0 additions & 32 deletions tests/sentry/tasks/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
save_event,
save_event_transaction,
)
from sentry.testutils.helpers.options import override_options
from sentry.testutils.pytest.fixtures import django_db_all

EVENT_ID = "cc3e6c2bb6b6498097f336d1e6979f4b"
Expand Down Expand Up @@ -382,37 +381,6 @@ def test_store_consumer_type(
mock_transaction_processing_store.get.return_value = transaction_data
mock_transaction_processing_store.store.return_value = "tx:3"

with mock.patch("sentry.event_manager.EventManager.save", return_value=None):
save_event_transaction(
cache_key="tx:3",
data=None,
start_time=1,
event_id=EVENT_ID,
project_id=default_project.id,
)

mock_transaction_processing_store.get.assert_called_once_with("tx:3")
mock_transaction_processing_store.delete_by_key.assert_not_called()


@django_db_all
@override_options({"transactions.do_post_process_in_save": 1.0})
def test_transactions_do_post_process_in_save_deletes_from_processing_store(
default_project,
mock_transaction_processing_store,
):
transaction_data = {
"project": default_project.id,
"platform": "transaction",
"event_id": EVENT_ID,
"extra": {"foo": "bar"},
"timestamp": time(),
"start_timestamp": time() - 1,
}

mock_transaction_processing_store.get.return_value = transaction_data
mock_transaction_processing_store.store.return_value = "tx:3"

with mock.patch("sentry.event_manager.EventManager.save", return_value=None):
save_event_transaction(
cache_key="tx:3",
Expand Down

0 comments on commit c09e99d

Please sign in to comment.