From 06081a99a077c5893f97ccd56f2100e540e3408d Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 11 Dec 2018 15:46:36 -0800 Subject: [PATCH 1/5] feat: Tornado integration --- sentry_sdk/integrations/_wsgi_common.py | 19 ++- sentry_sdk/integrations/tornado.py | 148 +++++++++++++++++++++ tests/integrations/tornado/test_tornado.py | 108 +++++++++++++++ 3 files changed, 269 insertions(+), 6 deletions(-) create mode 100644 sentry_sdk/integrations/tornado.py create mode 100644 tests/integrations/tornado/test_tornado.py diff --git a/sentry_sdk/integrations/_wsgi_common.py b/sentry_sdk/integrations/_wsgi_common.py index 6313853170..8b0c6b9e51 100644 --- a/sentry_sdk/integrations/_wsgi_common.py +++ b/sentry_sdk/integrations/_wsgi_common.py @@ -75,12 +75,7 @@ def parsed_body(self): return self.json() def is_json(self): - mt = (self.env().get("CONTENT_TYPE") or "").split(";", 1)[0] - return ( - mt == "application/json" - or (mt.startswith("application/")) - and mt.endswith("+json") - ) + return _is_json_content_type(self.env().get("CONTENT_TYPE")) def json(self): try: @@ -98,6 +93,18 @@ def files(self): def size_of_file(self, file): raise NotImplementedError() + def env(self): + raise NotImplementedError() + + +def _is_json_content_type(ct): + mt = (ct or "").split(";", 1)[0] + return ( + mt == "application/json" + or (mt.startswith("application/")) + and mt.endswith("+json") + ) + def _filter_headers(headers): if _should_send_default_pii(): diff --git a/sentry_sdk/integrations/tornado.py b/sentry_sdk/integrations/tornado.py new file mode 100644 index 0000000000..e446f73a9e --- /dev/null +++ b/sentry_sdk/integrations/tornado.py @@ -0,0 +1,148 @@ +import sys +import weakref + +from sentry_sdk.hub import Hub, _should_send_default_pii +from sentry_sdk.utils import event_from_exception, capture_internal_exceptions +from sentry_sdk.integrations import Integration +from sentry_sdk.integrations._wsgi_common import ( + RequestExtractor, + _filter_headers, + _is_json_content_type, +) +from sentry_sdk.integrations.logging import ignore_logger + +from tornado.web import RequestHandler, HTTPError +from tornado.gen import coroutine + + +class TornadoIntegration(Integration): + identifier = "tornado" + + @staticmethod + def setup_once(): + import tornado + + tornado_version = getattr(tornado, "version_info", None) + if tornado_version is None or tornado_version < (5, 0): + raise RuntimeError("Tornado 5+ required") + + if sys.version_info < (3, 7): + # Tornado is async. We better have contextvars or we're going to leak + # state between requests. + raise RuntimeError( + "The tornado integration for Sentry requires Python 3.7+" + ) + + ignore_logger("tornado.application") + ignore_logger("tornado.access") + + old_execute = RequestHandler._execute + + @coroutine + def sentry_execute_request_handler(self, *args, **kwargs): + hub = Hub.current + integration = hub.get_integration(TornadoIntegration) + if integration is None: + return old_execute(self, *args, **kwargs) + + weak_handler = weakref.ref(self) + + with Hub(hub) as hub: + with hub.configure_scope() as scope: + scope.add_event_processor(_make_event_processor(weak_handler)) + result = yield from old_execute(self, *args, **kwargs) + return result + + RequestHandler._execute = sentry_execute_request_handler + + old_log_exception = RequestHandler.log_exception + + def sentry_log_exception(self, ty, value, tb, *args, **kwargs): + _capture_exception(ty, value, tb) + return old_log_exception(self, ty, value, tb, *args, **kwargs) + + RequestHandler.log_exception = sentry_log_exception + + +def _capture_exception(ty, value, tb): + hub = Hub.current + if hub.get_integration(TornadoIntegration) is None: + return + if isinstance(value, HTTPError): + return + + event, hint = event_from_exception( + (ty, value, tb), + client_options=hub.client.options, + mechanism={"type": "tornado", "handled": False}, + ) + + hub.capture_event(event, hint=hint) + + +def _make_event_processor(weak_handler): + def tornado_processor(event, hint): + handler = weak_handler() + if handler is None: + return event + + request = handler.request + + with capture_internal_exceptions(): + extractor = TornadoRequestExtractor(request) + extractor.extract_into_event(event) + + request_info = event["request"] + + if "url" not in request_info: + request_info["url"] = "%s://%s%s" % ( + request.protocol, + request.host, + request.path, + ) + + if "query_string" not in request_info: + request_info["query_string"] = request.query + + if "method" not in request_info: + request_info["method"] = request.method + + if "env" not in request_info: + request_info["env"] = {"REMOTE_ADDR": request.remote_ip} + + if "headers" not in request_info: + request_info["headers"] = _filter_headers(dict(request.headers)) + + with capture_internal_exceptions(): + if handler.current_user and _should_send_default_pii(): + event.setdefault("user", {})["is_authenticated"] = True + + return event + + return tornado_processor + + +class TornadoRequestExtractor(RequestExtractor): + def content_length(self): + if self.request.body is None: + return 0 + return len(self.request.body) + + def cookies(self): + return dict(self.request.cookies) + + def raw_data(self): + return self.request.body + + def form(self): + # TODO: Where to get formdata and nothing else? + return None + + def is_json(self): + return _is_json_content_type(self.request.headers.get("content-type")) + + def files(self): + return {k: v[0] for k, v in self.request.files.items() if v} + + def size_of_file(self, file): + return len(file.body or ()) diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py new file mode 100644 index 0000000000..c6a8a7a302 --- /dev/null +++ b/tests/integrations/tornado/test_tornado.py @@ -0,0 +1,108 @@ +import pytest + +from sentry_sdk import configure_scope + +from sentry_sdk.integrations.tornado import TornadoIntegration + +from tornado.web import RequestHandler, Application, HTTPError +from tornado.testing import AsyncHTTPTestCase + + +class TestBasic(AsyncHTTPTestCase): + @pytest.fixture(autouse=True) + def initialize(self, sentry_init, capture_events): + sentry_init(integrations=[TornadoIntegration()]) + self.events = capture_events() + + def get_app(self): + class CrashingHandler(RequestHandler): + def get(self): + with configure_scope() as scope: + scope.set_tag("foo", 42) + 1 / 0 + + return Application([(r"/hi", CrashingHandler)]) + + def test_basic(self): + response = self.fetch("/hi?foo=bar") + assert response.code == 500 + + event, = self.events + exception, = event["exception"]["values"] + assert exception["type"] == "ZeroDivisionError" + + request = event["request"] + host = request["headers"]["Host"] + assert event["request"] == { + "env": {"REMOTE_ADDR": "127.0.0.1"}, + "headers": {"Accept-Encoding": "gzip", "Connection": "close", "Host": host}, + "method": "GET", + "query_string": "foo=bar", + "url": "http://{host}/hi".format(host=host), + } + + assert event["tags"] == {"foo": 42} + + with configure_scope() as scope: + assert not scope._tags + + +class Test400NotLogged(AsyncHTTPTestCase): + @pytest.fixture(autouse=True) + def initialize(self, sentry_init, capture_events): + sentry_init(integrations=[TornadoIntegration()]) + self.events = capture_events() + + def get_app(self): + class CrashingHandler(RequestHandler): + def get(self): + raise HTTPError(400, "Oops") + + return Application([(r"/", CrashingHandler)]) + + def test_400_not_logged(self): + response = self.fetch("/") + assert response.code == 400 + + assert not self.events + + +class TestUserAuth(AsyncHTTPTestCase): + @pytest.fixture(autouse=True) + def initialize(self, sentry_init, capture_events): + sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) + self.events = capture_events() + + def get_app(self): + class UserHandler(RequestHandler): + def get(self): + 1 / 0 + + def get_current_user(self): + return 42 + + class NoUserHandler(RequestHandler): + def get(self): + 1 / 0 + + return Application([(r"/auth", UserHandler), (r"/noauth", NoUserHandler)]) + + def test_has_user(self): + response = self.fetch("/auth") + assert response.code == 500 + + event, = self.events + exception, = event["exception"]["values"] + assert exception["type"] == "ZeroDivisionError" + + assert event["user"] == {"is_authenticated": True} + + def test_has_no_user(self): + response = self.fetch("/noauth") + assert response.code == 500 + + event, = self.events + exception, = event["exception"]["values"] + assert exception["type"] == "ZeroDivisionError" + + assert "user" not in event From fabef46e51ea715469bd03a5e53aa525ffd0b10d Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 11 Dec 2018 15:50:41 -0800 Subject: [PATCH 2/5] build: Add tornado to tox.ini --- tox.ini | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tox.ini b/tox.ini index 869e2a88c8..bec86ca522 100644 --- a/tox.ini +++ b/tox.ini @@ -34,6 +34,7 @@ envlist = {pypy,py2.7,py3.5,py3.6,py3.7,py3.8}-rq-0.12 {py3.7,py3.8}-aiohttp + {py3.7,py3.8}-tornado [testenv] deps = @@ -89,6 +90,8 @@ deps = aiohttp: aiohttp>=3.4.0,<3.5.0 aiohttp: pytest-aiohttp + tornado: tornado>=5,<6 + linters: black linters: flake8 setenv = @@ -103,6 +106,7 @@ setenv = pyramid: TESTPATH=tests/integrations/pyramid rq: TESTPATH=tests/integrations/rq aiohttp: TESTPATH=tests/integrations/aiohttp + tornado: TESTPATH=tests/integrations/tornado passenv = AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY From add294300309a8752a8edba4670f49e7229f8909 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 11 Dec 2018 16:00:18 -0800 Subject: [PATCH 3/5] fix: Skip tests for tornado when tornado is not installed --- tests/integrations/tornado/__init__.py | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 tests/integrations/tornado/__init__.py diff --git a/tests/integrations/tornado/__init__.py b/tests/integrations/tornado/__init__.py new file mode 100644 index 0000000000..a6ccd8a4ec --- /dev/null +++ b/tests/integrations/tornado/__init__.py @@ -0,0 +1,3 @@ +import pytest + +tornado = pytest.importorskip("tornado") From 5035f64ef13675c6bf3179bb5f39f90dc771f24e Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 11 Dec 2018 16:50:16 -0800 Subject: [PATCH 4/5] fix: Set transaction --- sentry_sdk/integrations/django/__init__.py | 6 ++-- .../integrations/django/transactions.py | 31 ------------------- sentry_sdk/integrations/tornado.py | 11 ++++++- sentry_sdk/utils.py | 31 +++++++++++++++++++ tests/integrations/tornado/test_tornado.py | 16 +++++++--- 5 files changed, 54 insertions(+), 41 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index bdd3d0e858..848d69460d 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -34,15 +34,13 @@ def sql_to_string(sql): event_from_exception, safe_repr, format_and_strip, + transaction_from_function, ) from sentry_sdk.integrations import Integration from sentry_sdk.integrations.logging import ignore_logger from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware from sentry_sdk.integrations._wsgi_common import RequestExtractor -from sentry_sdk.integrations.django.transactions import ( - LEGACY_RESOLVER, - transaction_from_function, -) +from sentry_sdk.integrations.django.transactions import LEGACY_RESOLVER if DJANGO_VERSION < (1, 10): diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 692eac0bef..3664cda02d 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -110,34 +110,3 @@ def resolve(self, path, urlconf=None): LEGACY_RESOLVER = RavenResolver() - - -def transaction_from_function(func): - # Methods in Python 2 - try: - return "%s.%s.%s" % ( - func.im_class.__module__, - func.im_class.__name__, - func.__name__, - ) - except Exception: - pass - - func_qualname = ( - getattr(func, "__qualname__", None) or getattr(func, "__name__", None) or None - ) - - if not func_qualname: - # No idea what it is - return None - - # Methods in Python 3 - # Functions - # Classes - try: - return "%s.%s" % (func.__module__, func_qualname) - except Exception: - pass - - # Possibly a lambda - return func_qualname diff --git a/sentry_sdk/integrations/tornado.py b/sentry_sdk/integrations/tornado.py index e446f73a9e..421b5c3fbb 100644 --- a/sentry_sdk/integrations/tornado.py +++ b/sentry_sdk/integrations/tornado.py @@ -2,7 +2,11 @@ import weakref from sentry_sdk.hub import Hub, _should_send_default_pii -from sentry_sdk.utils import event_from_exception, capture_internal_exceptions +from sentry_sdk.utils import ( + event_from_exception, + capture_internal_exceptions, + transaction_from_function, +) from sentry_sdk.integrations import Integration from sentry_sdk.integrations._wsgi_common import ( RequestExtractor, @@ -88,6 +92,11 @@ def tornado_processor(event, hint): request = handler.request + if "transaction" not in event: + with capture_internal_exceptions(): + method = getattr(handler, handler.request.method.lower()) + event["transaction"] = transaction_from_function(method) + with capture_internal_exceptions(): extractor = TornadoRequestExtractor(request) extractor.extract_into_event(event) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index a0a0ba3bd7..6388f96ea0 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -712,3 +712,34 @@ def get(self, default): def set(self, value): setattr(self._local, "value", value) + + +def transaction_from_function(func): + # Methods in Python 2 + try: + return "%s.%s.%s" % ( + func.im_class.__module__, + func.im_class.__name__, + func.__name__, + ) + except Exception: + pass + + func_qualname = ( + getattr(func, "__qualname__", None) or getattr(func, "__name__", None) or None + ) + + if not func_qualname: + # No idea what it is + return None + + # Methods in Python 3 + # Functions + # Classes + try: + return "%s.%s" % (func.__module__, func_qualname) + except Exception: + pass + + # Possibly a lambda + return func_qualname diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index c6a8a7a302..d346d86acf 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -8,6 +8,13 @@ from tornado.testing import AsyncHTTPTestCase +class CrashingHandler(RequestHandler): + def get(self): + with configure_scope() as scope: + scope.set_tag("foo", 42) + 1 / 0 + + class TestBasic(AsyncHTTPTestCase): @pytest.fixture(autouse=True) def initialize(self, sentry_init, capture_events): @@ -15,11 +22,6 @@ def initialize(self, sentry_init, capture_events): self.events = capture_events() def get_app(self): - class CrashingHandler(RequestHandler): - def get(self): - with configure_scope() as scope: - scope.set_tag("foo", 42) - 1 / 0 return Application([(r"/hi", CrashingHandler)]) @@ -42,6 +44,10 @@ def test_basic(self): } assert event["tags"] == {"foo": 42} + assert ( + event["transaction"] + == "tests.integrations.tornado.test_tornado.CrashingHandler.get" + ) with configure_scope() as scope: assert not scope._tags From 99348f1f1c326e1610d05a54ee55889e302e37f7 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 11 Dec 2018 21:12:46 -0800 Subject: [PATCH 5/5] fix: Move test --- .../integrations/django/test_transactions.py | 27 +------------------ tests/utils/test_transaction.py | 20 ++++++++++++++ 2 files changed, 21 insertions(+), 26 deletions(-) create mode 100644 tests/utils/test_transaction.py diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 29871b134f..5cf3f17c32 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -9,10 +9,7 @@ # for Django version less than 1.4 from django.conf.urls.defaults import url, include # NOQA -from sentry_sdk.integrations.django.transactions import ( - RavenResolver, - transaction_from_function, -) +from sentry_sdk.integrations.django.transactions import RavenResolver if django.VERSION < (1, 9): @@ -53,25 +50,3 @@ def test_legacy_resolver_newstyle_django20_urlconf(): resolver = RavenResolver() result = resolver.resolve("/api/v2/1234/store/", url_conf) assert result == "/api/v2/{project_id}/store/" - - -class MyClass: - def myfunc(): - pass - - -def myfunc(): - pass - - -def test_transaction_from_function(): - x = transaction_from_function - assert x(MyClass) == "tests.integrations.django.test_transactions.MyClass" - assert ( - x(MyClass.myfunc) - == "tests.integrations.django.test_transactions.MyClass.myfunc" - ) - assert x(myfunc) == "tests.integrations.django.test_transactions.myfunc" - assert x(None) is None - assert x(42) is None - assert x(lambda: None).endswith("") diff --git a/tests/utils/test_transaction.py b/tests/utils/test_transaction.py new file mode 100644 index 0000000000..6548d806f5 --- /dev/null +++ b/tests/utils/test_transaction.py @@ -0,0 +1,20 @@ +from sentry_sdk.utils import transaction_from_function + + +class MyClass: + def myfunc(): + pass + + +def myfunc(): + pass + + +def test_transaction_from_function(): + x = transaction_from_function + assert x(MyClass) == "tests.utils.test_transaction.MyClass" + assert x(MyClass.myfunc) == "tests.utils.test_transaction.MyClass.myfunc" + assert x(myfunc) == "tests.utils.test_transaction.myfunc" + assert x(None) is None + assert x(42) is None + assert x(lambda: None).endswith("")