Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

create rc-processing-transaction #79200

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
+91 −18
Draft

create rc-processing-transaction #79200

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bd10897
create rc-processing-transaction
kneeyo1 Oct 16, 2024
8ab9f15
add to all
kneeyo1 Oct 16, 2024
1059d02
Merge branch 'master' into rc-processing-split-pt1
kneeyo1 Oct 16, 2024
987cf21
add to everywhere transactions uses eventprocessing
kneeyo1 Oct 16, 2024
96b5134
Update src/sentry/conf/server.py
kneeyo1 Oct 16, 2024
021461b
update to just read/write to transactions
kneeyo1 Oct 16, 2024
6302093
cleanup
kneeyo1 Oct 16, 2024
c15dc45
transaction
kneeyo1 Oct 16, 2024
31c3521
Update src/sentry/conf/server.py
kneeyo1 Oct 16, 2024
9d7fa8b
cleanup
kneeyo1 Oct 17, 2024
2a1aece
added some better ways to split up errors and transactions
kneeyo1 Oct 17, 2024
db433f0
Merge branch 'master' into rc-processing-split-pt1
kneeyo1 Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/sentry/conf/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1666,6 +1666,10 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]:
)
SENTRY_EVENT_PROCESSING_STORE_OPTIONS: dict[str, str] = {}

# Transactions processing backend
SENTRY_TRANSACTIONS_PROCESSING_STORE: str | None = None
SENTRY_TRANSACTION_PROCESSING_STORE_OPTIONS: dict[str, str] | None = None
kneeyo1 marked this conversation as resolved.
Show resolved Hide resolved

# The internal Django cache is still used in many places
# TODO(dcramer): convert uses over to Sentry's backend
CACHES = {"default": {"BACKEND": "django.core.cache.backends.dummy.DummyCache"}}
Expand Down
15 changes: 14 additions & 1 deletion src/sentry/eventstore/processing/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,18 @@
settings.SENTRY_EVENT_PROCESSING_STORE_OPTIONS,
)

transaction_processing_store = LazyServiceWrapper(
EventProcessingStore,
(
settings.SENTRY_EVENT_PROCESSING_STORE
if settings.SENTRY_TRANSACTIONS_PROCESSING_STORE is None
else settings.SENTRY_TRANSACTIONS_PROCESSING_STORE
),
(
settings.SENTRY_EVENT_PROCESSING_STORE_OPTIONS
if settings.SENTRY_TRANSACTIONS_PROCESSING_STORE_OPTIONS is None
else settings.SENTRY_TRANSACTIONS_PROCESSING_STORE_OPTIONS
kneeyo1 marked this conversation as resolved.
Show resolved Hide resolved
),
)

__all__ = ["event_processing_store"]
__all__ = ["event_processing_store", "transaction_processing_store"]
13 changes: 11 additions & 2 deletions src/sentry/ingest/consumer/processors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from sentry import eventstore, features
from sentry.attachments import CachedAttachment, attachment_cache
from sentry.event_manager import EventManager, save_attachment
from sentry.eventstore.processing import event_processing_store
from sentry.eventstore.processing import event_processing_store, transaction_processing_store
from sentry.feedback.usecases.create_feedback import FeedbackCreationSource, is_in_feedback_denylist
from sentry.ingest.userreport import Conflict, save_userreport
from sentry.killswitches import killswitch_matches_context
Expand Down Expand Up @@ -73,6 +73,7 @@ def process_transaction_no_celery(
# Delete the event payload from cache since it won't show up in post-processing.
if cache_key:
event_processing_store.delete_by_key(cache_key)
transaction_processing_store.delete_by_key(cache_key)
kneeyo1 marked this conversation as resolved.
Show resolved Hide resolved
return

manager = EventManager(data)
Expand All @@ -89,6 +90,7 @@ def process_transaction_no_celery(
if not isinstance(data, dict):
data = dict(data.items())
event_processing_store.store(data)
transaction_processing_store.store(data)


@trace_func(name="ingest_consumer.process_event")
Expand Down Expand Up @@ -199,11 +201,18 @@ def process_event(
return

with metrics.timer("ingest_consumer._store_event"):
cache_key = event_processing_store.store(data)
event_type = data.get("type")

if event_type == "transaction":
cache_key = transaction_processing_store.store(data)
else:
cache_key = event_processing_store.store(data)

try:
# Records rc-processing usage broken down by
# event type.

# todo: rewrite this after rc-processing split
event_type = data.get("type")
kneeyo1 marked this conversation as resolved.
Show resolved Hide resolved
if event_type == "error":
app_feature = "errors"
Expand Down
7 changes: 6 additions & 1 deletion src/sentry/tasks/post_process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from google.api_core.exceptions import ServiceUnavailable

from sentry import features, projectoptions
from sentry.eventstore.processing import transaction_processing_store
from sentry.exceptions import PluginError
from sentry.issues.grouptype import GroupCategory
from sentry.issues.issue_occurrence import IssueOccurrence
Expand Down Expand Up @@ -524,7 +525,11 @@ def post_process_group(
)
return
with metrics.timer("tasks.post_process.delete_event_cache"):
event_processing_store.delete_by_key(cache_key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you also need to update line 520 to point at the transacations store?

event_type = data.get("type", None)
if event_type == "transaction":
transaction_processing_store.delete_by_key(cache_key)
else:
event_processing_store.delete_by_key(cache_key)
kneeyo1 marked this conversation as resolved.
Show resolved Hide resolved

occurrence = None
event = process_event(data, group_id)
Expand Down
11 changes: 9 additions & 2 deletions src/sentry/tasks/store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,11 +562,18 @@ def _do_save_event(
data = manager.get_data()
if not isinstance(data, dict):
data = dict(data.items())
processing.event_processing_store.store(data)

if event_type == "transaction":
processing.transaction_processing_store.store(data)
else:
processing.event_processing_store.store(data)
kneeyo1 marked this conversation as resolved.
Show resolved Hide resolved
except HashDiscarded:
# Delete the event payload from cache since it won't show up in post-processing.
if cache_key:
processing.event_processing_store.delete_by_key(cache_key)
if event_type == "transaction":
processing.transaction_processing_store.delete_by_key(cache_key)
else:
processing.event_processing_store.delete_by_key(cache_key)
Copy link
Member

@lynnagara lynnagara Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have this if/else block twice in this file..

suggest to refactor to something like:

processing_store = transaction_processing_store if event_type == "transaction" else event_processing_store
processing_store.get(cache_key)
processing_store.store(data)
processing_store.delete_by_key(cache_key)

also, there needs to be a 3rd time, as line 509 also needs to go to either the event or transaction processing store

except Exception:
metrics.incr("events.save_event.exception", tags={"event_type": event_type})
raise
Expand Down
Loading