Skip to content

fix: Disable transaction events in store #14088

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.

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

Merged
merged 1 commit into from
Jul 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions requirements-test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 25 additions & 4 deletions src/sentry/web/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
64 changes: 64 additions & 0 deletions tests/integration/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": (
Expand Down Expand Up @@ -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):
Expand Down