ref(transactions): option for not doing transactions sampling in post_process #81079
Conversation
github-actions
bot
added
the
Scope: Backend
JoshFerge
force-pushed
the
jferg/tx-post-process-1
branch
from
b07a90f
to
a44c8d6
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-2
branch
from
c7f02a1
to
a9f2ad3
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-1
branch
from
a44c8d6
to
74e168f
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-2
branch
from
a9f2ad3
to
f89cfd2
Compare
src/sentry/options/defaults.py
Outdated
| @@ -2919,6 +2919,7 @@ | |||
| "performance.event-tracker.sample-rate.transaction", | |||
| default=0.0, | |||
| ) | |||
|
|
|||
src/sentry/tasks/post_process.py
Outdated
| @@ -611,7 +611,7 @@ def get_event_raise_exception() -> Event: | |||
| # Simplified post processing for transaction events. | |||
| # This should eventually be completely removed and transactions | |||
| # will not go through any post processing. | |||
| if is_transaction_event: | |||
| if is_transaction_event and not options.get("transactions.do_post_process_in_save"): | |||
There was a problem hiding this comment.
I believe there's a slight chance of a small window where the servers processing the two different stages of processing will not have synchronized options applied.
What would be the consequences of this code not applying at all for some transactions?
Or the consequences of calling twice?
I don't think it is important to handle it if it would not cause any major problems.
There was a problem hiding this comment.
TLDR: it shouldn't matter very much, it's used for transaction clustering, some blips in processing shouldn't affect this very much. (and even if it did, it is not a super critical feature).
called out in the ticket here:
I assume that we're okay with a minute of data or so being lost from the transaction clusterer. by doing a simple switch-over, we can save some complexity. (i believe this is just used for accurate dynamic sampling calculations)
Lyn also asked about this. responded here:
from reading about the transactions sampler, i believe it is a feature that helps with parameterizing high cardinality transaction names. i don't believe double sampling (wouldn't happen as i've implemented) or not having samples (would be the case for a few moments for this deploy) would have a large effect on the product.
https://develop.sentry.dev/api-server/application-domains/transaction-clustering/#accidental-erasure-of-non-identifiers
JoshFerge
force-pushed
the
jferg/tx-post-process-1
branch
2 times, most recently
from
48e4329
to
80c2656
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-2
branch
from
f89cfd2
to
1505605
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-2
branch
from
1505605
to
f85382e
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-1
branch
from
80c2656
to
6f9afa7
Compare
JoshFerge
force-pushed
the
jferg/tx-post-process-2
branch
from
f85382e
to
5ccfc69
Compare
|
@markstory i've gone ahead and merged this into #81077 |
in lockstep with #81077, stops doing work in post_process on transactions. part of #81065