Merge pull request #75 from honeycombio/tredman.post-context

[middleware] enable subclassing of other middleware implementations, remove POST in django

CHANGELOG.md

@@ -1,5 +1,16 @@
 # beeline-python changelog
 
+## 2.8.0 2019-08-06
+
+Features
+
+- Django, Flask, Bottle, and Werkzeug middleware can now be subclassed to provide alternative implementations of `get_context_from_request` (Django) `get_context_from_environ` (Flask, Bottle, Werkzeug) methods. This allows customization of the request fields that are automatically instrumented at the start of a trace. Thanks to sjoerdjob's initial contribution in [#73](https://github.com/honeycombio/beeline-python/pull/73).
+
+Fixes
+
+- Django's `HoneyMiddleware` no longer adds a `request.post` field by default. This was removed for two reasons. First, calling `request.POST.dict()` could break other middleware by exhausting the request stream prematurely. See issue [#74](https://github.com/honeycombio/beeline-python/issues/74). Second, POST bodies can contain arbitrary values and potentially sensitive data, and the decision to instrument these values should be a deliberate choice by the user. If you currently rely on this behavior currently, you can swap out `HoneyMiddleware` with `HoneyMiddlewareWithPOST` to maintain the same functionality.
+- The `awslambda` middleware no longer crashes if the `context` object is missing certain attributes. See [#76](https://github.com/honeycombio/beeline-python/pull/76).
+
 ## 2.7.0 2019-07-26
 
 Features

beeline/middleware/bottle/__init__.py

@@ -6,12 +6,24 @@ def __init__(self, app):
         self.app = app
 
     def __call__(self, environ, start_response):
+        trace = beeline.start_trace(context=self.get_context_from_environ(environ))
+
+        def _start_response(status, headers, *args):
+            beeline.add_context_field("response.status_code", status)
+            beeline.finish_trace(trace)
+
+            return start_response(status, headers, *args)
+
+        return self.app(environ, _start_response)
+
+    def get_context_from_environ(self, environ):
         request_method = environ.get('REQUEST_METHOD')
         if request_method:
             trace_name = "bottle_http_%s" % request_method.lower()
         else:
             trace_name = "bottle_http"
-        trace = beeline.start_trace(context={
+
+        return {
             "name": trace_name,
             "type": "http_server",
             "request.host": environ.get('HTTP_HOST'),
@@ -22,12 +34,4 @@ def __call__(self, environ, start_response):
             "request.user_agent": environ.get('HTTP_USER_AGENT'),
             "request.scheme": environ.get('wsgi.url_scheme'),
             "request.query": environ.get('QUERY_STRING')
-        })
-
-        def _start_response(status, headers, *args):
-            beeline.add_context_field("response.status_code", status)
-            beeline.finish_trace(trace)
-
-            return start_response(status, headers, *args)
-
-        return self.app(environ, _start_response)
+        }

beeline/middleware/bottle/test_bottle.py

@@ -0,0 +1,32 @@
+import unittest
+from mock import Mock, patch, ANY
+
+from beeline.middleware.bottle import HoneyWSGIMiddleware
+
+class SimpleWSGITest(unittest.TestCase):
+    def setUp(self):
+        self.addCleanup(patch.stopall)
+        self.m_gbl = patch('beeline.middleware.bottle.beeline').start()
+
+    def test_call_middleware(self):
+        ''' Just call the middleware and ensure that the code runs '''
+        mock_app = Mock()
+        mock_resp = Mock()
+        mock_trace = Mock()
+        mock_environ = {}
+        self.m_gbl.start_trace.return_value = mock_trace
+
+        mw = HoneyWSGIMiddleware(mock_app)
+        mw({}, mock_resp)
+        self.m_gbl.start_trace.assert_called_once()
+
+        mock_app.assert_called_once_with(mock_environ, ANY)
+
+        # get the response function passed to the app
+        resp_func = mock_app.mock_calls[0][1][1]
+        # call it to make sure it does what we want
+        # the values here don't really matter
+        resp_func(1, 2)
+
+        mock_resp.assert_called_once_with(1, 2)
+        self.m_gbl.finish_trace.assert_called_once_with(mock_trace)

beeline/middleware/django/__init__.py

@@ -70,7 +70,6 @@ def get_context_from_request(self, request):
             "request.secure": request.is_secure(),
             "request.query": request.GET.dict(),
             "request.xhr": request.is_ajax(),
-            "request.post": request.POST.dict(),
         }
 
     def get_context_from_response(self, request, response):
@@ -126,3 +125,25 @@ def __call__(self, request):
             response = self.create_http_event(request)
 
         return response
+
+class HoneyMiddlewareWithPOST(HoneyMiddleware):
+    ''' HoneyMiddlewareWithPOST is a subclass of HoneyMiddleware. The only difference is that
+    the `request.post` field is instrumented. This was removed from the base implementation in 2.8.0
+    due to conflicts with other middleware. See https://github.com/honeycombio/beeline-python/issues/74.'''
+    def get_context_from_request(self, request):
+        trace_name = "django_http_%s" % request.method.lower()
+        return {
+            "name": trace_name,
+            "type": "http_server",
+            "request.host": request.get_host(),
+            "request.method": request.method,
+            "request.path": request.path,
+            "request.remote_addr": request.META.get('REMOTE_ADDR'),
+            "request.content_length": request.META.get('CONTENT_LENGTH', 0),
+            "request.user_agent": request.META.get('HTTP_USER_AGENT'),
+            "request.scheme": request.scheme,
+            "request.secure": request.is_secure(),
+            "request.query": request.GET.dict(),
+            "request.xhr": request.is_ajax(),
+            "request.post": request.POST.dict(),
+        }

beeline/middleware/django/test_django.py

@@ -0,0 +1,25 @@
+import unittest
+from mock import Mock, patch
+
+from beeline.middleware.django import HoneyMiddlewareBase
+
+class SimpleWSGITest(unittest.TestCase):
+    def setUp(self):
+        self.addCleanup(patch.stopall)
+        self.m_gbl = patch('beeline.middleware.django.beeline').start()
+
+    def test_call_middleware(self):
+        ''' Just call the middleware and ensure that the code runs '''
+        mock_req = Mock()
+        mock_resp = Mock()
+        mock_trace = Mock()
+        self.m_gbl.start_trace.return_value = mock_trace
+
+        mw = HoneyMiddlewareBase(mock_resp)
+        resp = mw(mock_req)
+        self.m_gbl.start_trace.assert_called_once()
+
+        mock_resp.assert_called_once_with(mock_req)
+
+        self.m_gbl.finish_trace.assert_called_once_with(mock_trace)
+        self.assertEqual(resp, mock_resp.return_value)

beeline/middleware/flask/__init__.py

@@ -12,8 +12,6 @@ def _get_trace_context(environ):
     # http://werkzeug.pocoo.org/docs/0.14/wrappers/#base-wrappers
     req = Request(environ, shallow=True)
 
-
-
     trace_context = req.headers.get('x-honeycomb-trace')
     beeline.internal.log("got trace context: %s", trace_context)
     if trace_context:
@@ -24,6 +22,7 @@ def _get_trace_context(environ):
 
     return None, None, None
 
+
 class HoneyMiddleware(object):
 
     def __init__(self, app, db_events=True):
@@ -47,26 +46,11 @@ def __init__(self, app):
         self.app = app
 
     def __call__(self, environ, start_response):
-        request_method = environ.get('REQUEST_METHOD')
-        if request_method:
-            trace_name = "flask_http_%s" % request_method.lower()
-        else:
-            trace_name = "flask_http"
-
         trace_id, parent_id, context = _get_trace_context(environ)
 
-        root_span = beeline.start_trace(context={
-            "type": "http_server",
-            "name": trace_name,
-            "request.host": environ.get('HTTP_HOST'),
-            "request.method": request_method,
-            "request.path": environ.get('PATH_INFO'),
-            "request.remote_addr": environ.get('REMOTE_ADDR'),
-            "request.content_length": environ.get('CONTENT_LENGTH', 0),
-            "request.user_agent": environ.get('HTTP_USER_AGENT'),
-            "request.scheme": environ.get('wsgi.url_scheme'),
-            "request.query": environ.get('QUERY_STRING')
-        }, trace_id=trace_id, parent_span_id=parent_id)
+        root_span = beeline.start_trace(
+            context=self.get_context_from_environ(environ), 
+            trace_id=trace_id, parent_span_id=parent_id)
 
         # populate any propagated custom context
         if isinstance(context, dict):
@@ -85,6 +69,26 @@ def _start_response(status, headers, *args):
 
         return self.app(environ, _start_response)
 
+    def get_context_from_environ(self, environ):
+        request_method = environ.get('REQUEST_METHOD')
+        if request_method:
+            trace_name = "flask_http_%s" % request_method.lower()
+        else:
+            trace_name = "flask_http"
+
+        return {
+            "type": "http_server",
+            "name": trace_name,
+            "request.host": environ.get('HTTP_HOST'),
+            "request.method": request_method,
+            "request.path": environ.get('PATH_INFO'),
+            "request.remote_addr": environ.get('REMOTE_ADDR'),
+            "request.content_length": environ.get('CONTENT_LENGTH', 0),
+            "request.user_agent": environ.get('HTTP_USER_AGENT'),
+            "request.scheme": environ.get('wsgi.url_scheme'),
+            "request.query": environ.get('QUERY_STRING')
+        }
+
 
 class HoneyDBMiddleware(object):
 

beeline/middleware/flask/test_flask.py

@@ -0,0 +1,32 @@
+import unittest
+from mock import Mock, patch, ANY
+
+from beeline.middleware.flask import HoneyWSGIMiddleware
+
+class SimpleWSGITest(unittest.TestCase):
+    def setUp(self):
+        self.addCleanup(patch.stopall)
+        self.m_gbl = patch('beeline.middleware.flask.beeline').start()
+
+    def test_call_middleware(self):
+        ''' Just call the middleware and ensure that the code runs '''
+        mock_app = Mock()
+        mock_resp = Mock()
+        mock_trace = Mock()
+        mock_environ = {}
+        self.m_gbl.start_trace.return_value = mock_trace
+
+        mw = HoneyWSGIMiddleware(mock_app)
+        mw({}, mock_resp)
+        self.m_gbl.start_trace.assert_called_once()
+
+        mock_app.assert_called_once_with(mock_environ, ANY)
+
+        # get the response function passed to the app
+        resp_func = mock_app.mock_calls[0][1][1]
+        # call it to make sure it does what we want
+        # the values here don't really matter
+        resp_func("200", 2)
+
+        mock_resp.assert_called_once_with("200", 2)
+        self.m_gbl.finish_trace.assert_called_once_with(mock_trace)

beeline/middleware/werkzeug/__init__.py

@@ -6,12 +6,25 @@ def __init__(self, app):
         self.app = app
 
     def __call__(self, environ, start_response):
+
+        trace = beeline.start_trace(context=self.get_context_from_environ(environ))
+
+        def _start_response(status, headers, *args):
+            beeline.add_context_field("response.status_code", status)
+            beeline.finish_trace(trace)
+
+            return start_response(status, headers, *args)
+
+        return self.app(environ, _start_response)
+
+    def get_context_from_environ(self, environ):
         request_method = environ.get('REQUEST_METHOD')
         if request_method:
             trace_name = "werkzeug_http_%s" % request_method.lower()
         else:
             trace_name = "werkzeug_http"
-        trace = beeline.start_trace(context={
+
+        return {
             "name": trace_name,
             "type": "http_server",
             "request.host": environ.get('HTTP_HOST'),
@@ -22,12 +35,4 @@ def __call__(self, environ, start_response):
             "request.user_agent": environ.get('HTTP_USER_AGENT'),
             "request.scheme": environ.get('wsgi.url_scheme'),
             "request.query": environ.get('QUERY_STRING')
-        })
-
-        def _start_response(status, headers, *args):
-            beeline.add_context_field("response.status_code", status)
-            beeline.finish_trace(trace)
-
-            return start_response(status, headers, *args)
-
-        return self.app(environ, _start_response)
+        }

beeline/middleware/werkzeug/test_werkzeug.py

@@ -0,0 +1,32 @@
+import unittest
+from mock import Mock, patch, ANY
+
+from beeline.middleware.werkzeug import HoneyWSGIMiddleware
+
+class SimpleWSGITest(unittest.TestCase):
+    def setUp(self):
+        self.addCleanup(patch.stopall)
+        self.m_gbl = patch('beeline.middleware.werkzeug.beeline').start()
+
+    def test_call_middleware(self):
+        ''' Just call the middleware and ensure that the code runs '''
+        mock_app = Mock()
+        mock_resp = Mock()
+        mock_trace = Mock()
+        mock_environ = {}
+        self.m_gbl.start_trace.return_value = mock_trace
+
+        mw = HoneyWSGIMiddleware(mock_app)
+        mw({}, mock_resp)
+        self.m_gbl.start_trace.assert_called_once()
+
+        mock_app.assert_called_once_with(mock_environ, ANY)
+
+        # get the response function passed to the app
+        resp_func = mock_app.mock_calls[0][1][1]
+        # call it to make sure it does what we want
+        # the values here don't really matter
+        resp_func(1, 2)
+
+        mock_resp.assert_called_once_with(1, 2)
+        self.m_gbl.finish_trace.assert_called_once_with(mock_trace)

beeline/version.py

@@ -1 +1 @@
-VERSION = '2.7.0'
+VERSION = '2.8.0'

setup.py

@@ -4,7 +4,7 @@
 setup(
     python_requires='>=2.7',
     name='honeycomb-beeline',
-    version='2.7.0',
+    version='2.8.0',
     description='Honeycomb library for easy instrumentation',
     url='https://github.com/honeycombio/beeline-python',
     author='Honeycomb.io',