From 54257ba11c78fa8644a11c80dcfc95772eef496e Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 19 Jul 2019 15:44:49 +0200 Subject: [PATCH] fix: Disable transaction events in store --- requirements-test.txt | 1 + src/sentry/web/api.py | 29 ++++++++++++++--- tests/integration/tests.py | 64 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/requirements-test.txt b/requirements-test.txt index 267b282dbff1cf..71c613904ff327 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -6,3 +6,4 @@ pytest-timeout==1.2.1 pytest-xdist>=1.18.0,<1.19.0 responses>=0.8.1,<0.9.0 sqlparse==0.1.19 +werkzeug==0.15.5 diff --git a/src/sentry/web/api.py b/src/sentry/web/api.py index e41de83fddbad3..0aba578a9f2121 100644 --- a/src/sentry/web/api.py +++ b/src/sentry/web/api.py @@ -64,6 +64,7 @@ from sentry.utils.outcomes import Outcome, track_outcome from sentry.utils.pubsub import QueuedPublisherService, KafkaPublisher from sentry.utils.safe import safe_execute +from sentry.utils.sdk import configure_scope from sentry.web.helpers import render_to_response from sentry.web.relay_config import get_full_relay_config from sentry.web.client_config import get_client_config @@ -130,6 +131,17 @@ def allow_cors_options_wrapper(self, request, *args, **kwargs): return allow_cors_options_wrapper +def disable_transaction_events(): + """ + Do not send a transaction event for the current transaction. + + This is used in StoreView to prevent infinite recursion. + """ + with configure_scope() as scope: + if scope.span: + scope.span.sampled = False + + def api(func): @wraps(func) def wrapped(request, *args, **kwargs): @@ -327,7 +339,12 @@ def _parse_header(self, request, relay_config): ) if not auth.client: - track_outcome(relay_config.organization_id, relay_config.project_id, None, Outcome.INVALID, "auth_client") + track_outcome( + relay_config.organization_id, + relay_config.project_id, + None, + Outcome.INVALID, + "auth_client") raise APIError("Client did not send 'client' identifier") return auth @@ -382,7 +399,8 @@ def dispatch(self, request, project_id=None, *args, **kwargs): ) # if the project id is not directly specified get it from the authentication information - project_id = _get_project_id_from_request(project_id, request, self.auth_helper_cls, helper) + project_id = _get_project_id_from_request( + project_id, request, self.auth_helper_cls, helper) relay_config = get_full_relay_config(project_id) @@ -561,7 +579,9 @@ def pre_normalize(self, data, helper): """Mutate the given EventManager. Hook for subtypes of StoreView (CSP)""" pass - def process(self, request, project, key, auth, helper, data, relay_config, attachments=None, **kwargs): + def process(self, request, project, key, auth, helper, + data, relay_config, attachments=None, **kwargs): + disable_transaction_events() metrics.incr('events.total', skip_internal=False) project_id = relay_config.project_id @@ -829,7 +849,8 @@ def post(self, request, project, relay_config, **kwargs): )) # Append all other files as generic attachments. - # RaduW 4 Jun 2019 always sent attachments for minidump (does not use event-attachments feature) + # RaduW 4 Jun 2019 always sent attachments for minidump (does not use + # event-attachments feature) for name, file in six.iteritems(request_files): if name == 'upload_file_minidump': continue diff --git a/tests/integration/tests.py b/tests/integration/tests.py index de4965d3fd3081..a7874f72d764df 100644 --- a/tests/integration/tests.py +++ b/tests/integration/tests.py @@ -18,12 +18,19 @@ from exam import fixture from gzip import GzipFile from sentry_sdk import Hub, Client +from sentry_sdk.integrations.celery import CeleryIntegration +from sentry_sdk.integrations.django import DjangoIntegration from six import StringIO +from werkzeug.test import Client as WerkzeugClient from sentry.models import (Group, Event) from sentry.testutils import TestCase, TransactionTestCase from sentry.testutils.helpers import get_auth_header from sentry.utils.settings import (validate_settings, ConfigurationError, import_string) +from sentry.utils.sdk import configure_scope +from sentry.web.api import disable_transaction_events +from sentry.wsgi import application + DEPENDENCY_TEST_DATA = { "postgresql": ( @@ -464,6 +471,63 @@ def test_protocol_v6(self): assert instance.message == 'hello' +class SentryWsgiRemoteTest(TransactionTestCase): + + def test_traceparent_header_wsgi(self): + # Assert that posting something to store will not create another + # (transaction) event under any circumstances. + # + # We use Werkzeug's test client because Django's test client bypasses a + # lot of request handling code that we want to test implicitly (such as + # all our WSGI middlewares and the entire Django instrumentation by + # sentry-sdk). + # + # XXX(markus): Ideally methods such as `_postWithHeader` would always + # call the WSGI application => swap out Django's test client with e.g. + # Werkzeug's. + client = WerkzeugClient(application) + + calls = [] + + def new_disable_transaction_events(): + with configure_scope() as scope: + assert scope.span.sampled + assert scope.span.transaction + disable_transaction_events() + assert not scope.span.sampled + + calls.append(1) + + events = [] + + auth = get_auth_header( + '_postWithWerkzeug/0.0.0', + self.projectkey.public_key, + self.projectkey.secret_key, + '7' + ) + + with mock.patch('sentry.web.api.disable_transaction_events', new_disable_transaction_events): + with self.tasks(): + with Hub(Client(transport=events.append, integrations=[CeleryIntegration(), DjangoIntegration()])): + app_iter, status, headers = client.post( + reverse('sentry-api-store'), + data=b'{"message": "hello"}', + headers={ + 'x-sentry-auth': auth, + 'sentry-trace': '1', + 'content-type': 'application/octet-stream', + }, + environ_base={'REMOTE_ADDR': '127.0.0.1'} + ) + + body = ''.join(app_iter) + + assert status == '200 OK', body + assert not events + assert calls == [1] + + class DependencyTest(TestCase): def raise_import_error(self, package): def callable(package_name):