From e5a9610d9bcee0b3cb1a2a85d566359265090b77 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 19 Mar 2021 10:47:12 +0100 Subject: [PATCH 1/4] feat: Support tracing on Tornado --- sentry_sdk/integrations/aiohttp.py | 2 +- sentry_sdk/integrations/tornado.py | 64 +++++++++++++--------- tests/integrations/tornado/test_tornado.py | 33 ++++++++++- 3 files changed, 72 insertions(+), 27 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 2d8eaedfab..f74e6f4bf2 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -92,7 +92,7 @@ async def sentry_app_handle(self, request, *args, **kwargs): weak_request = weakref.ref(request) - with Hub(Hub.current) as hub: + with Hub(hub) as hub: # Scope data will not leak between requests because aiohttp # create a task to wrap each request. with hub.configure_scope() as scope: diff --git a/sentry_sdk/integrations/tornado.py b/sentry_sdk/integrations/tornado.py index 27f254844d..e13549d4f7 100644 --- a/sentry_sdk/integrations/tornado.py +++ b/sentry_sdk/integrations/tornado.py @@ -1,7 +1,9 @@ import weakref +import contextlib from inspect import iscoroutinefunction from sentry_sdk.hub import Hub, _should_send_default_pii +from sentry_sdk.tracing import Transaction from sentry_sdk.utils import ( HAS_REAL_CONTEXTVARS, CONTEXTVARS_ERROR_MESSAGE, @@ -32,6 +34,7 @@ from typing import Optional from typing import Dict from typing import Callable + from typing import Generator from sentry_sdk._types import EventProcessor @@ -63,19 +66,8 @@ def setup_once(): # Starting Tornado 6 RequestHandler._execute method is a standard Python coroutine (async/await) # In that case our method should be a coroutine function too async def sentry_execute_request_handler(self, *args, **kwargs): - # type: (Any, *Any, **Any) -> Any - hub = Hub.current - integration = hub.get_integration(TornadoIntegration) - if integration is None: - return await old_execute(self, *args, **kwargs) - - weak_handler = weakref.ref(self) - - with Hub(hub) as hub: - with hub.configure_scope() as scope: - scope.clear_breadcrumbs() - processor = _make_event_processor(weak_handler) # type: ignore - scope.add_event_processor(processor) + # type: (RequestHandler, *Any, **Any) -> Any + with _handle_request_impl(self): return await old_execute(self, *args, **kwargs) else: @@ -83,18 +75,7 @@ async def sentry_execute_request_handler(self, *args, **kwargs): @coroutine # type: ignore def sentry_execute_request_handler(self, *args, **kwargs): # type: (RequestHandler, *Any, **Any) -> Any - 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.clear_breadcrumbs() - processor = _make_event_processor(weak_handler) # type: ignore - scope.add_event_processor(processor) + with _handle_request_impl(self): result = yield from old_execute(self, *args, **kwargs) return result @@ -110,6 +91,39 @@ def sentry_log_exception(self, ty, value, tb, *args, **kwargs): RequestHandler.log_exception = sentry_log_exception # type: ignore +@contextlib.contextmanager +def _handle_request_impl(self): + # type: (RequestHandler) -> Generator[None, None, None] + hub = Hub.current + integration = hub.get_integration(TornadoIntegration) + + if integration is None: + yield + + weak_handler = weakref.ref(self) + + with Hub(hub) as hub: + with hub.configure_scope() as scope: + scope.clear_breadcrumbs() + processor = _make_event_processor(weak_handler) # type: ignore + scope.add_event_processor(processor) + + transaction = Transaction.continue_from_headers( + self.request.headers, + op="http.server", + # Like with all other integrations, this is our + # fallback transaction in case there is no route. + # sentry_urldispatcher_resolve is responsible for + # setting a transaction name later. + name="generic Tornado request", + ) + + with hub.start_transaction( + transaction, custom_sampling_context={"tornado_request": self.request} + ): + yield + + def _capture_exception(ty, value, tb): # type: (type, BaseException, Any) -> None hub = Hub.current diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index 0cec16c4b7..f9fba01a50 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -2,7 +2,7 @@ import pytest -from sentry_sdk import configure_scope +from sentry_sdk import configure_scope, start_transaction from sentry_sdk.integrations.tornado import TornadoIntegration from tornado.web import RequestHandler, Application, HTTPError @@ -82,6 +82,37 @@ def test_basic(tornado_testcase, sentry_init, capture_events): assert not scope._tags +def test_transactions(tornado_testcase, sentry_init, capture_events): + sentry_init(integrations=[TornadoIntegration()], traces_sample_rate=1.0, debug=True) + events = capture_events() + client = tornado_testcase(Application([(r"/hi", CrashingHandler)])) + + with start_transaction(name="client") as span: + pass + + response = client.fetch("/hi", headers=dict(span.iter_headers())) + assert response.code == 500 + + client_tx, server_error, server_tx = events + + assert client_tx["type"] == "transaction" + assert client_tx["transaction"] == "client" + + assert server_error["exception"]["values"][0]["type"] == "ZeroDivisionError" + assert server_tx["type"] == "transaction" + assert ( + server_tx["transaction"] + == server_error["transaction"] + == "tests.integrations.tornado.test_tornado.CrashingHandler.get" + ) + + assert ( + client_tx["contexts"]["trace"]["trace_id"] + == server_error["contexts"]["trace"]["trace_id"] + == server_tx["contexts"]["trace"]["trace_id"] + ) + + def test_400_not_logged(tornado_testcase, sentry_init, capture_events): sentry_init(integrations=[TornadoIntegration()]) events = capture_events() From 8bae62e2b2374649eb720e810a411e11405515a9 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 19 Mar 2021 11:54:58 +0100 Subject: [PATCH 2/4] add extra assertion about request body --- tests/integrations/tornado/test_tornado.py | 25 ++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index f9fba01a50..a198ae8fa6 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -40,6 +40,11 @@ def get(self): scope.set_tag("foo", "42") 1 / 0 + def post(self): + with configure_scope() as scope: + scope.set_tag("foo", "43") + 1 / 0 + def test_basic(tornado_testcase, sentry_init, capture_events): sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) @@ -90,7 +95,7 @@ def test_transactions(tornado_testcase, sentry_init, capture_events): with start_transaction(name="client") as span: pass - response = client.fetch("/hi", headers=dict(span.iter_headers())) + response = client.fetch("/hi", method="POST", body=b"heyoo", headers=dict(span.iter_headers())) assert response.code == 500 client_tx, server_error, server_tx = events @@ -103,9 +108,25 @@ def test_transactions(tornado_testcase, sentry_init, capture_events): assert ( server_tx["transaction"] == server_error["transaction"] - == "tests.integrations.tornado.test_tornado.CrashingHandler.get" + == "tests.integrations.tornado.test_tornado.CrashingHandler.post" ) + + request = server_tx["request"] + host = request["headers"]["Host"] + assert server_tx["request"] == { + "env": {"REMOTE_ADDR": "127.0.0.1"}, + "headers": { + "Accept-Encoding": "gzip", + "Connection": "close", + **request["headers"], + }, + "method": "POST", + "query_string": "", + "data": {"heyoo": ['']}, + "url": "http://{host}/hi".format(host=host), + } + assert ( client_tx["contexts"]["trace"]["trace_id"] == server_error["contexts"]["trace"]["trace_id"] From 7f65e718d62300afcc880a01e98fa3806722ceb1 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 19 Mar 2021 12:20:58 +0100 Subject: [PATCH 3/4] parametrize transaction test --- tests/integrations/tornado/test_tornado.py | 50 +++++++++++++++++----- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index a198ae8fa6..52e4a15d56 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -46,6 +46,20 @@ def post(self): 1 / 0 +class HelloHandler(RequestHandler): + async def get(self): + with configure_scope() as scope: + scope.set_tag("foo", "42") + + return b"hello" + + async def post(self): + with configure_scope() as scope: + scope.set_tag("foo", "43") + + return b"hello" + + def test_basic(tornado_testcase, sentry_init, capture_events): sentry_init(integrations=[TornadoIntegration()], send_default_pii=True) events = capture_events() @@ -87,29 +101,40 @@ def test_basic(tornado_testcase, sentry_init, capture_events): assert not scope._tags -def test_transactions(tornado_testcase, sentry_init, capture_events): +@pytest.mark.parametrize("handler,code", [ + (CrashingHandler, 500), + (HelloHandler, 200), +]) +def test_transactions(tornado_testcase, sentry_init, capture_events, handler, code): sentry_init(integrations=[TornadoIntegration()], traces_sample_rate=1.0, debug=True) events = capture_events() - client = tornado_testcase(Application([(r"/hi", CrashingHandler)])) + client = tornado_testcase(Application([(r"/hi", handler)])) with start_transaction(name="client") as span: pass response = client.fetch("/hi", method="POST", body=b"heyoo", headers=dict(span.iter_headers())) - assert response.code == 500 + assert response.code == code - client_tx, server_error, server_tx = events + if code == 200: + client_tx, server_tx = events + server_error = None + else: + client_tx, server_error, server_tx = events assert client_tx["type"] == "transaction" assert client_tx["transaction"] == "client" - assert server_error["exception"]["values"][0]["type"] == "ZeroDivisionError" + if server_error is not None: + assert server_error["exception"]["values"][0]["type"] == "ZeroDivisionError" + assert server_error["transaction"] == "tests.integrations.tornado.test_tornado.CrashingHandler.post" + + if code == 200: + assert server_tx["transaction"] == "tests.integrations.tornado.test_tornado.HelloHandler.post" + else: + assert server_tx["transaction"] == "tests.integrations.tornado.test_tornado.CrashingHandler.post" + assert server_tx["type"] == "transaction" - assert ( - server_tx["transaction"] - == server_error["transaction"] - == "tests.integrations.tornado.test_tornado.CrashingHandler.post" - ) request = server_tx["request"] @@ -129,11 +154,14 @@ def test_transactions(tornado_testcase, sentry_init, capture_events): assert ( client_tx["contexts"]["trace"]["trace_id"] - == server_error["contexts"]["trace"]["trace_id"] == server_tx["contexts"]["trace"]["trace_id"] ) + if server_error is not None: + assert server_error["contexts"]["trace"]["trace_id"] == server_tx["contexts"]["trace"]["trace_id"] + + def test_400_not_logged(tornado_testcase, sentry_init, capture_events): sentry_init(integrations=[TornadoIntegration()]) events = capture_events() From 65bda5af9b13a9c60dbf8542643b664bbb45d187 Mon Sep 17 00:00:00 2001 From: sentry-bot Date: Fri, 19 Mar 2021 11:23:58 +0000 Subject: [PATCH 4/4] fix: Formatting --- tests/integrations/tornado/test_tornado.py | 39 +++++++++++++++------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/tests/integrations/tornado/test_tornado.py b/tests/integrations/tornado/test_tornado.py index 52e4a15d56..1c5137f2b2 100644 --- a/tests/integrations/tornado/test_tornado.py +++ b/tests/integrations/tornado/test_tornado.py @@ -101,10 +101,13 @@ def test_basic(tornado_testcase, sentry_init, capture_events): assert not scope._tags -@pytest.mark.parametrize("handler,code", [ - (CrashingHandler, 500), - (HelloHandler, 200), -]) +@pytest.mark.parametrize( + "handler,code", + [ + (CrashingHandler, 500), + (HelloHandler, 200), + ], +) def test_transactions(tornado_testcase, sentry_init, capture_events, handler, code): sentry_init(integrations=[TornadoIntegration()], traces_sample_rate=1.0, debug=True) events = capture_events() @@ -113,7 +116,9 @@ def test_transactions(tornado_testcase, sentry_init, capture_events, handler, co with start_transaction(name="client") as span: pass - response = client.fetch("/hi", method="POST", body=b"heyoo", headers=dict(span.iter_headers())) + response = client.fetch( + "/hi", method="POST", body=b"heyoo", headers=dict(span.iter_headers()) + ) assert response.code == code if code == 200: @@ -127,16 +132,24 @@ def test_transactions(tornado_testcase, sentry_init, capture_events, handler, co if server_error is not None: assert server_error["exception"]["values"][0]["type"] == "ZeroDivisionError" - assert server_error["transaction"] == "tests.integrations.tornado.test_tornado.CrashingHandler.post" + assert ( + server_error["transaction"] + == "tests.integrations.tornado.test_tornado.CrashingHandler.post" + ) if code == 200: - assert server_tx["transaction"] == "tests.integrations.tornado.test_tornado.HelloHandler.post" + assert ( + server_tx["transaction"] + == "tests.integrations.tornado.test_tornado.HelloHandler.post" + ) else: - assert server_tx["transaction"] == "tests.integrations.tornado.test_tornado.CrashingHandler.post" + assert ( + server_tx["transaction"] + == "tests.integrations.tornado.test_tornado.CrashingHandler.post" + ) assert server_tx["type"] == "transaction" - request = server_tx["request"] host = request["headers"]["Host"] assert server_tx["request"] == { @@ -148,7 +161,7 @@ def test_transactions(tornado_testcase, sentry_init, capture_events, handler, co }, "method": "POST", "query_string": "", - "data": {"heyoo": ['']}, + "data": {"heyoo": [""]}, "url": "http://{host}/hi".format(host=host), } @@ -157,9 +170,11 @@ def test_transactions(tornado_testcase, sentry_init, capture_events, handler, co == server_tx["contexts"]["trace"]["trace_id"] ) - if server_error is not None: - assert server_error["contexts"]["trace"]["trace_id"] == server_tx["contexts"]["trace"]["trace_id"] + assert ( + server_error["contexts"]["trace"]["trace_id"] + == server_tx["contexts"]["trace"]["trace_id"] + ) def test_400_not_logged(tornado_testcase, sentry_init, capture_events):