Add support for Sanic.TouchUp metaclass (#607)

* Add support for Sanic.TouchUp metaclass

Starting in version 21.9.0 sanic added a TouchUp metaclass that rewrites methods on the
Sanic class effectively undoing our instrumentation wrapping. This adds back our
instrumentation wrapping.

Co-authored-by: TimPansino <[email protected]>

* Change Sanic "test app" name to "test-app"

Sanic app names cannot have spaces.

* Fix sanic warning

* Fix sanic testing setup

* Fix sanic response streaming

* Fix sanic testing init logic

* Add missing py39 tests for sanic

Co-authored-by: TimPansino <[email protected]>
Co-authored-by: Tim Pansino <[email protected]>

Author: hmstepanek

newrelic/config.py

@@ -2544,6 +2544,9 @@ def _process_module_builtin_defaults():
 
     _process_module_definition("sanic.app", "newrelic.hooks.framework_sanic", "instrument_sanic_app")
     _process_module_definition("sanic.response", "newrelic.hooks.framework_sanic", "instrument_sanic_response")
+    _process_module_definition(
+        "sanic.touchup.service", "newrelic.hooks.framework_sanic", "instrument_sanic_touchup_service"
+    )
 
     _process_module_definition("aiohttp.wsgi", "newrelic.hooks.framework_aiohttp", "instrument_aiohttp_wsgi")
     _process_module_definition("aiohttp.web", "newrelic.hooks.framework_aiohttp", "instrument_aiohttp_web")

newrelic/hooks/framework_sanic.py

@@ -15,13 +15,12 @@
 import sys
 from inspect import isawaitable
 
-from newrelic.api.web_transaction import web_transaction
-from newrelic.api.transaction import current_transaction
-from newrelic.api.function_trace import function_trace, FunctionTrace
+from newrelic.api.function_trace import FunctionTrace, function_trace
 from newrelic.api.time_trace import notice_error
-from newrelic.common.object_wrapper import (wrap_function_wrapper,
-    function_wrapper)
+from newrelic.api.transaction import current_transaction
+from newrelic.api.web_transaction import web_transaction
 from newrelic.common.object_names import callable_name
+from newrelic.common.object_wrapper import function_wrapper, wrap_function_wrapper
 
 
 def _bind_add(uri, methods, handler, *args, **kwargs):
@@ -36,19 +35,20 @@ def _nr_wrapper_handler_(wrapped, instance, args, kwargs):
         return wrapped(*args, **kwargs)
 
     name = callable_name(wrapped)
-    view_class = getattr(wrapped, 'view_class', None)
+    view_class = getattr(wrapped, "view_class", None)
     view = view_class or wrapped
     if view_class:
         try:
             method = args[0].method.lower()
-            name = callable_name(view_class) + '.' + method
+            name = callable_name(view_class) + "." + method
             view = getattr(view_class, method)
         except:
             pass
-    
+
     transaction.set_transaction_name(name, priority=3)
     import sanic
-    transaction.add_framework_info(name='Sanic', version=sanic.__version__)
+
+    transaction.add_framework_info(name="Sanic", version=sanic.__version__)
 
     with FunctionTrace(name=name, source=view):
         return wrapped(*args, **kwargs)
@@ -60,7 +60,7 @@ def _nr_sanic_router_add(wrapped, instance, args, kwargs):
 
     # Cache the callable_name on the handler object
     callable_name(handler)
-    if hasattr(wrapped, 'view_class'):
+    if hasattr(wrapped, "view_class"):
         callable_name(wrapped.view_class)
     wrapped_handler = _nr_wrapper_handler_(handler)
 
@@ -131,7 +131,7 @@ def error_response(wrapped, instance, args, kwargs):
         raise
     else:
         # response can be a response object or a coroutine
-        if hasattr(response, 'status'):
+        if hasattr(response, "status"):
             notice_error(error=exc_info, status_code=response.status)
         else:
             notice_error(exc_info)
@@ -144,18 +144,16 @@ def error_response(wrapped, instance, args, kwargs):
 def _sanic_app_init(wrapped, instance, args, kwargs):
     result = wrapped(*args, **kwargs)
 
-    error_handler = getattr(instance, 'error_handler')
-    if hasattr(error_handler, 'response'):
-        instance.error_handler.response = error_response(
-                error_handler.response)
-    if hasattr(error_handler, 'add'):
-        error_handler.add = _nr_sanic_error_handlers(
-                error_handler.add)
+    error_handler = getattr(instance, "error_handler")
+    if hasattr(error_handler, "response"):
+        instance.error_handler.response = error_response(error_handler.response)
+    if hasattr(error_handler, "add"):
+        error_handler.add = _nr_sanic_error_handlers(error_handler.add)
 
-    router = getattr(instance, 'router')
-    if hasattr(router, 'add'):
+    router = getattr(instance, "router")
+    if hasattr(router, "add"):
         router.add = _nr_sanic_router_add(router.add)
-    if hasattr(router, 'get'):
+    if hasattr(router, "get"):
         # Cache the callable_name on the router.get
         callable_name(router.get)
         router.get = _nr_sanic_router_get(router.get)
@@ -172,8 +170,7 @@ def _nr_sanic_response_get_headers(wrapped, instance, args, kwargs):
         return result
 
     # instance is the response object
-    cat_headers = transaction.process_response(str(instance.status),
-            instance.headers.items())
+    cat_headers = transaction.process_response(str(instance.status), instance.headers.items())
 
     for header_name, header_value in cat_headers:
         if header_name not in instance.headers:
@@ -189,27 +186,26 @@ async def _nr_sanic_response_send(wrapped, instance, args, kwargs):
         await result
 
     if transaction is None:
-        return wrapped(*args, **kwargs)
+        return result
 
     # instance is the response object
-    cat_headers = transaction.process_response(str(instance.status),
-            instance.headers.items())
+    cat_headers = transaction.process_response(str(instance.status), instance.headers.items())
 
     for header_name, header_value in cat_headers:
         if header_name not in instance.headers:
             instance.headers[header_name] = header_value
 
     return result
 
+
 def _nr_sanic_response_parse_headers(wrapped, instance, args, kwargs):
     transaction = current_transaction()
 
     if transaction is None:
         return wrapped(*args, **kwargs)
 
     # instance is the response object
-    cat_headers = transaction.process_response(str(instance.status),
-            instance.headers.items())
+    cat_headers = transaction.process_response(str(instance.status), instance.headers.items())
 
     for header_name, header_value in cat_headers:
         if header_name not in instance.headers:
@@ -219,7 +215,7 @@ def _nr_sanic_response_parse_headers(wrapped, instance, args, kwargs):
 
 
 def _nr_wrapper_middleware_(attach_to):
-    is_request_middleware = attach_to == 'request'
+    is_request_middleware = attach_to == "request"
 
     @function_wrapper
     def _wrapper(wrapped, instance, args, kwargs):
@@ -238,7 +234,7 @@ def _wrapper(wrapped, instance, args, kwargs):
     return _wrapper
 
 
-def _bind_middleware(middleware, attach_to='request', *args, **kwargs):
+def _bind_middleware(middleware, attach_to="request", *args, **kwargs):
     return middleware, attach_to
 
 
@@ -259,36 +255,55 @@ def _bind_request(request, *args, **kwargs):
 def _nr_sanic_transaction_wrapper_(wrapped, instance, args, kwargs):
     request = _bind_request(*args, **kwargs)
     # If the request is a websocket request do not wrap it
-    if request.headers.get('upgrade', '').lower() == 'websocket':
+    if request.headers.get("upgrade", "").lower() == "websocket":
         return wrapped(*args, **kwargs)
 
     return web_transaction(
         request_method=request.method,
         request_path=request.path,
         query_string=request.query_string,
-        headers=request.headers)(wrapped)(*args, **kwargs)
+        headers=request.headers,
+    )(wrapped)(*args, **kwargs)
+
+
+def _nr_wrap_touchup_run(wrapped, instance, args, kwargs):
+    # TouchUp uses metaprogramming to rewrite methods of classes on startup.
+    # To properly wrap them we need to catch the call to TouchUp.run and
+    # reinstrument any methods that were replaced with uninstrumented versions.
+
+    result = wrapped(*args, **kwargs)
+
+    if "sanic.app" in sys.modules:
+        module = sys.modules["sanic.app"]
+        target = args[0]
+
+        if isinstance(target, module.Sanic):
+            # Reinstrument class after metaclass "TouchUp" has finished rewriting methods on the class.
+            target_cls = module.Sanic
+            if hasattr(target_cls, "handle_request") and not hasattr(target_cls.handle_request, "__wrapped__"):
+                wrap_function_wrapper(module, "Sanic.handle_request", _nr_sanic_transaction_wrapper_)
+
+    return result
 
 
 def instrument_sanic_app(module):
-    wrap_function_wrapper(module, 'Sanic.handle_request',
-        _nr_sanic_transaction_wrapper_)
-    wrap_function_wrapper(module, 'Sanic.__init__',
-        _sanic_app_init)
-    wrap_function_wrapper(module, 'Sanic.register_middleware',
-        _nr_sanic_register_middleware_)
-    if hasattr(module.Sanic, 'register_named_middleware'):
-        wrap_function_wrapper(module, 'Sanic.register_named_middleware',
-            _nr_sanic_register_middleware_)
+    wrap_function_wrapper(module, "Sanic.handle_request", _nr_sanic_transaction_wrapper_)
+    wrap_function_wrapper(module, "Sanic.__init__", _sanic_app_init)
+    wrap_function_wrapper(module, "Sanic.register_middleware", _nr_sanic_register_middleware_)
+    if hasattr(module.Sanic, "register_named_middleware"):
+        wrap_function_wrapper(module, "Sanic.register_named_middleware", _nr_sanic_register_middleware_)
 
 
 def instrument_sanic_response(module):
-    if hasattr(module.BaseHTTPResponse, 'send'):
-        wrap_function_wrapper(module, 'BaseHTTPResponse.send',
-            _nr_sanic_response_send)
+    if hasattr(module.BaseHTTPResponse, "send"):
+        wrap_function_wrapper(module, "BaseHTTPResponse.send", _nr_sanic_response_send)
     else:
-        if hasattr(module.BaseHTTPResponse, 'get_headers'):
-            wrap_function_wrapper(module, 'BaseHTTPResponse.get_headers',
-                _nr_sanic_response_get_headers)
-        if hasattr(module.BaseHTTPResponse, '_parse_headers'):
-            wrap_function_wrapper(module, 'BaseHTTPResponse._parse_headers',
-                _nr_sanic_response_parse_headers)
+        if hasattr(module.BaseHTTPResponse, "get_headers"):
+            wrap_function_wrapper(module, "BaseHTTPResponse.get_headers", _nr_sanic_response_get_headers)
+        if hasattr(module.BaseHTTPResponse, "_parse_headers"):
+            wrap_function_wrapper(module, "BaseHTTPResponse._parse_headers", _nr_sanic_response_parse_headers)
+
+
+def instrument_sanic_touchup_service(module):
+    if hasattr(module, "TouchUp") and hasattr(module.TouchUp, "run"):
+        wrap_function_wrapper(module.TouchUp, "run", _nr_wrap_touchup_run)

tests/framework_sanic/_target_application.py

@@ -15,11 +15,18 @@
 from sanic import Blueprint, Sanic
 from sanic.exceptions import NotFound, SanicException, ServerError
 from sanic.handlers import ErrorHandler
-from sanic.response import json, stream
+from sanic.response import json
 from sanic.router import Router
 from sanic.views import HTTPMethodView
 
 
+try:
+    # Old style response streaming
+    from sanic.response import stream
+except ImportError:
+    stream = None
+
+
 class MethodView(HTTPMethodView):
     async def get(self, request):
         return json({"hello": "world"})
@@ -93,7 +100,7 @@ def get(self, *args):
     error_handler = CustomErrorHandler()
 
 router = CustomRouter()
-app = Sanic(name="test app", error_handler=error_handler, router=router)
+app = Sanic(name="test-app", error_handler=error_handler, router=router)
 router.app = app
 blueprint = Blueprint("test_bp")
 
@@ -139,13 +146,25 @@ async def blueprint_middleware(request):
 app.register_middleware(request_middleware)
 
 
+async def do_streaming(request):
+    if stream is not None:
+        # Old style response streaming
+        async def streaming_fn(response):
+            response.write("foo")
+            response.write("bar")
+
+        return stream(streaming_fn)
+    else:
+        # New style response streaming
+        response = await request.respond(content_type="text/plain")
+        await response.send("foo")
+        await response.send("bar")
+        await response.eof()
+
+
 @app.route("/streaming")
 async def streaming(request):
-    async def streaming_fn(response):
-        response.write("foo")
-        response.write("bar")
-
-    return stream(streaming_fn)
+    return await do_streaming(request)
 
 
 # Fake websocket endpoint to enable websockets on the server
@@ -200,17 +219,11 @@ async def async_error(request):
 
 @blueprint.route("/blueprint")
 async def blueprint_route(request):
-    async def streaming_fn(response):
-        response.write("foo")
-
-    return stream(streaming_fn)
-
+    return await do_streaming(request)
 
 app.blueprint(blueprint)
 app.add_route(MethodView.as_view(), "/method_view")
 
-if not getattr(router, "finalized", True):
-    router.finalize()
 
 if __name__ == "__main__":
     app.run(host="127.0.0.1", port=8000)

tests/framework_sanic/conftest.py

@@ -80,10 +80,13 @@ async def send(*args, **kwargs):
         proto = MockProtocol(loop=loop, app=app)
         proto.recv_buffer = bytearray()
         http = Http(proto)
+        
+        if hasattr(http, "init_for_request"):
+            http.init_for_request()
+
         http.stage = Stage.HANDLER
         http.response_func = http.http1_response_header
         _request.stream = http
-        pass
     except ImportError:
         pass
 
@@ -123,6 +126,15 @@ def request(app, method, url, headers=None):
     if loop is None:
         loop = asyncio.new_event_loop()
 
+    if not getattr(app.router, "finalized", True):
+        # Handle startup if the router hasn't been finalized.
+        # Older versions don't have this requirement or variable so
+        # the default should be True.
+        if hasattr(app, "_startup"):
+            loop.run_until_complete(app._startup())
+        else:
+            app.router.finalize()
+
     coro = create_request_coroutine(app, method, url, headers, loop)
     loop.run_until_complete(coro)
     return RESPONSES.pop()

tox.ini

@@ -134,8 +134,8 @@ envlist =
     python-framework_pyramid-{pypy,py27,py38}-Pyramid0104,
     python-framework_pyramid-{pypy,py27,pypy37,py37,py38,py39,py310}-Pyramid0110-cornice,
     python-framework_pyramid-{py37,py38,py39,py310,pypy37}-Pyramidmaster,
-    python-framework_sanic-{py38,pypy37}-sanic{190301,1906,1812,1912,200904,210300},
-    python-framework_sanic-{py37,py38,py310,pypy37}-saniclatest,
+    python-framework_sanic-{py38,pypy37}-sanic{190301,1906,1812,1912,200904,210300,2109,2112,2203},
+    python-framework_sanic-{py37,py38,py39,py310,pypy37}-saniclatest,
     python-framework_starlette-{py310,pypy37}-starlette{0014,0015,0019},
     python-framework_starlette-{py37,py38,py39,py310,pypy37}-starlettelatest,
     python-framework_strawberry-{py37,py38,py39,py310}-strawberrylatest,
@@ -319,8 +319,10 @@ deps =
     framework_sanic-sanic1912: sanic<19.13
     framework_sanic-sanic200904: sanic<20.9.5
     framework_sanic-sanic210300: sanic<21.3.1
-    ; Temporarily test older sanic version until issues are resolved
-    framework_sanic-saniclatest: sanic<21.9.0
+    framework_sanic-sanic2109: sanic<21.10
+    framework_sanic-sanic2112: sanic<21.13
+    framework_sanic-sanic2203: sanic<22.4
+    framework_sanic-saniclatest: sanic
     framework_sanic-sanic{1812,190301,1906}: aiohttp
     framework_starlette: graphene<3
     framework_starlette-starlette0014: starlette<0.15