Skip to content

Commit 05221d8

Browse files
untitakermitsuhiko
authored andcommitted
fix: Disable transaction events in store (#14088)
1 parent cbc7b96 commit 05221d8

File tree

3 files changed

+90
-4
lines changed

3 files changed

+90
-4
lines changed

requirements-test.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ pytest-timeout==1.2.1
66
pytest-xdist>=1.18.0,<1.19.0
77
responses>=0.8.1,<0.9.0
88
sqlparse==0.1.19
9+
werkzeug==0.15.5

src/sentry/web/api.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
from sentry.utils.outcomes import Outcome, track_outcome
6565
from sentry.utils.pubsub import QueuedPublisherService, KafkaPublisher
6666
from sentry.utils.safe import safe_execute
67+
from sentry.utils.sdk import configure_scope
6768
from sentry.web.helpers import render_to_response
6869
from sentry.web.relay_config import get_full_relay_config
6970
from sentry.web.client_config import get_client_config
@@ -130,6 +131,17 @@ def allow_cors_options_wrapper(self, request, *args, **kwargs):
130131
return allow_cors_options_wrapper
131132

132133

134+
def disable_transaction_events():
135+
"""
136+
Do not send a transaction event for the current transaction.
137+
138+
This is used in StoreView to prevent infinite recursion.
139+
"""
140+
with configure_scope() as scope:
141+
if scope.span:
142+
scope.span.sampled = False
143+
144+
133145
def api(func):
134146
@wraps(func)
135147
def wrapped(request, *args, **kwargs):
@@ -327,7 +339,12 @@ def _parse_header(self, request, relay_config):
327339
)
328340

329341
if not auth.client:
330-
track_outcome(relay_config.organization_id, relay_config.project_id, None, Outcome.INVALID, "auth_client")
342+
track_outcome(
343+
relay_config.organization_id,
344+
relay_config.project_id,
345+
None,
346+
Outcome.INVALID,
347+
"auth_client")
331348
raise APIError("Client did not send 'client' identifier")
332349

333350
return auth
@@ -382,7 +399,8 @@ def dispatch(self, request, project_id=None, *args, **kwargs):
382399
)
383400

384401
# if the project id is not directly specified get it from the authentication information
385-
project_id = _get_project_id_from_request(project_id, request, self.auth_helper_cls, helper)
402+
project_id = _get_project_id_from_request(
403+
project_id, request, self.auth_helper_cls, helper)
386404

387405
relay_config = get_full_relay_config(project_id)
388406

@@ -561,7 +579,9 @@ def pre_normalize(self, data, helper):
561579
"""Mutate the given EventManager. Hook for subtypes of StoreView (CSP)"""
562580
pass
563581

564-
def process(self, request, project, key, auth, helper, data, relay_config, attachments=None, **kwargs):
582+
def process(self, request, project, key, auth, helper,
583+
data, relay_config, attachments=None, **kwargs):
584+
disable_transaction_events()
565585
metrics.incr('events.total', skip_internal=False)
566586

567587
project_id = relay_config.project_id
@@ -829,7 +849,8 @@ def post(self, request, project, relay_config, **kwargs):
829849
))
830850

831851
# Append all other files as generic attachments.
832-
# RaduW 4 Jun 2019 always sent attachments for minidump (does not use event-attachments feature)
852+
# RaduW 4 Jun 2019 always sent attachments for minidump (does not use
853+
# event-attachments feature)
833854
for name, file in six.iteritems(request_files):
834855
if name == 'upload_file_minidump':
835856
continue

tests/integration/tests.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,19 @@
1818
from exam import fixture
1919
from gzip import GzipFile
2020
from sentry_sdk import Hub, Client
21+
from sentry_sdk.integrations.celery import CeleryIntegration
22+
from sentry_sdk.integrations.django import DjangoIntegration
2123
from six import StringIO
24+
from werkzeug.test import Client as WerkzeugClient
2225

2326
from sentry.models import (Group, Event)
2427
from sentry.testutils import TestCase, TransactionTestCase
2528
from sentry.testutils.helpers import get_auth_header
2629
from sentry.utils.settings import (validate_settings, ConfigurationError, import_string)
30+
from sentry.utils.sdk import configure_scope
31+
from sentry.web.api import disable_transaction_events
32+
from sentry.wsgi import application
33+
2734

2835
DEPENDENCY_TEST_DATA = {
2936
"postgresql": (
@@ -464,6 +471,63 @@ def test_protocol_v6(self):
464471
assert instance.message == 'hello'
465472

466473

474+
class SentryWsgiRemoteTest(TransactionTestCase):
475+
476+
def test_traceparent_header_wsgi(self):
477+
# Assert that posting something to store will not create another
478+
# (transaction) event under any circumstances.
479+
#
480+
# We use Werkzeug's test client because Django's test client bypasses a
481+
# lot of request handling code that we want to test implicitly (such as
482+
# all our WSGI middlewares and the entire Django instrumentation by
483+
# sentry-sdk).
484+
#
485+
# XXX(markus): Ideally methods such as `_postWithHeader` would always
486+
# call the WSGI application => swap out Django's test client with e.g.
487+
# Werkzeug's.
488+
client = WerkzeugClient(application)
489+
490+
calls = []
491+
492+
def new_disable_transaction_events():
493+
with configure_scope() as scope:
494+
assert scope.span.sampled
495+
assert scope.span.transaction
496+
disable_transaction_events()
497+
assert not scope.span.sampled
498+
499+
calls.append(1)
500+
501+
events = []
502+
503+
auth = get_auth_header(
504+
'_postWithWerkzeug/0.0.0',
505+
self.projectkey.public_key,
506+
self.projectkey.secret_key,
507+
'7'
508+
)
509+
510+
with mock.patch('sentry.web.api.disable_transaction_events', new_disable_transaction_events):
511+
with self.tasks():
512+
with Hub(Client(transport=events.append, integrations=[CeleryIntegration(), DjangoIntegration()])):
513+
app_iter, status, headers = client.post(
514+
reverse('sentry-api-store'),
515+
data=b'{"message": "hello"}',
516+
headers={
517+
'x-sentry-auth': auth,
518+
'sentry-trace': '1',
519+
'content-type': 'application/octet-stream',
520+
},
521+
environ_base={'REMOTE_ADDR': '127.0.0.1'}
522+
)
523+
524+
body = ''.join(app_iter)
525+
526+
assert status == '200 OK', body
527+
assert not events
528+
assert calls == [1]
529+
530+
467531
class DependencyTest(TestCase):
468532
def raise_import_error(self, package):
469533
def callable(package_name):

0 commit comments

Comments
 (0)