Add querystring to MozLog "request.summary" (#97)

* Add querystring to MozLog request.summary

* Put querystring logging behing a flag

Author: leplatrem

docs/django.rst

@@ -287,7 +287,7 @@ spec:
 .. http:get:: /__heartbeat__
 
    The heartbeat view will go through the list of configured Dockerflow
-   checks in the :ref:`DOCKERFLOW_CHECKS` setting, run each check, and, if 
+   checks in the :ref:`DOCKERFLOW_CHECKS` setting, run each check, and, if
    `settings.DEBUG` is `True`, add their results to a JSON response.
 
    The view will return HTTP responses with either a status code of 200 if
@@ -398,6 +398,12 @@ configure **at least** the ``request.summary`` logger that way::
         }
     }
 
+In order to include querystrings in the request summary log, set this flag in settings:
+
+.. code-block:: python
+
+    DOCKERFLOW_SUMMARY_LOG_QUERYSTRING = True
+
 
 .. _django-static:
 

docs/fastapi.rst

@@ -307,6 +307,15 @@ for at least the ``request.summary`` logger:
         }
     })
 
+
+In order to include querystrings in the request summary log, set this flag in the application state:
+
+.. code-block:: python
+
+    app.state.DOCKERFLOW_SUMMARY_LOG_QUERYSTRING = True
+
+
+
 .. _fastapi-static:
 
 Static content

docs/flask.rst

@@ -453,6 +453,10 @@ for at least the ``request.summary`` logger::
         }
     })
 
+In order to include querystrings in the request summary log, set this flag in :ref:`configuration <flask-config>`::
+
+    DOCKERFLOW_SUMMARY_LOG_QUERYSTRING = True
+
 .. _flask-static:
 
 Static content

docs/sanic.rst

@@ -452,6 +452,11 @@ Alternatively you can also pass the same logging config dictionary to the
 
     sanic = Sanic(__name__)
 
+In order to include querystrings in the request summary log, set this flag in :ref:`configuration <sanic-config>`::
+
+    DOCKERFLOW_SUMMARY_LOG_QUERYSTRING = True
+
+
 .. _sanic-static:
 
 Static content

src/dockerflow/django/middleware.py

@@ -1,8 +1,10 @@
 import logging
 import re
 import time
+import urllib
 import uuid
 
+from django.conf import settings
 from django.utils.deprecation import MiddlewareMixin
 
 from . import views
@@ -44,6 +46,11 @@ def _build_extra_meta(self, request):
             "path": request.path,
         }
 
+        if getattr(settings, "DOCKERFLOW_SUMMARY_LOG_QUERYSTRING", False):
+            out["querystring"] = urllib.parse.unquote(
+                request.META.get("QUERY_STRING", "")
+            )
+
         # HACK: It's possible some other middleware has replaced the request we
         # modified earlier, so be sure to check for existence of these
         # attributes before trying to use them.

src/dockerflow/fastapi/middleware.py

@@ -3,6 +3,7 @@
 import logging
 import sys
 import time
+import urllib
 from typing import Any, Dict
 
 from asgiref.typing import (
@@ -66,11 +67,15 @@ def _format(self, scope: HTTPScope, info) -> Dict[str, Any]:
             info["request_headers"][header_key] = header_val
 
         request_duration_ms = (info["end_time"] - info["start_time"]) * 1000.0
-        return {
+        fields = {
             "agent": info["request_headers"].get("user-agent", ""),
             "path": scope["path"],
             "method": scope["method"],
             "code": info["response"]["status"],
             "lang": info["request_headers"].get("accept-language"),
             "t": int(request_duration_ms),
         }
+
+        if getattr(scope["app"].state, "DOCKERFLOW_SUMMARY_LOG_QUERYSTRING", False):
+            fields["querystring"] = urllib.parse.unquote(scope["query_string"].decode())
+        return fields

src/dockerflow/flask/app.py

@@ -259,6 +259,9 @@ def summary_extra(self):
             "path": flask.request.path,
         }
 
+        if flask.current_app.config.get("DOCKERFLOW_SUMMARY_LOG_QUERYSTRING", False):
+            out["querystring"] = flask.request.query_string.decode()
+
         # set the uid value to the current user ID
         user_id = self.user_id()
         if user_id is None:

src/dockerflow/sanic/app.py

@@ -4,6 +4,7 @@
 
 import logging
 import time
+import urllib
 import uuid
 import warnings
 from inspect import isawaitable
@@ -162,6 +163,9 @@ def summary_extra(self, request):
             "uid": "",
         }
 
+        if request.app.config.get("DOCKERFLOW_SUMMARY_LOG_QUERYSTRING", False):
+            out["querystring"] = urllib.parse.unquote(request.query_string)
+
         # the rid value to the current request ID
         try:
             out["rid"] = request.ctx.id

tests/django/test_django.py

@@ -169,6 +169,21 @@ def test_request_summary(admin_user, caplog, dockerflow_middleware, dockerflow_r
     assert getattr(dockerflow_request, "uid", None) is None
 
 
+def test_request_summary_querystring(
+    settings, admin_user, caplog, dockerflow_middleware, rf
+):
+    settings.DOCKERFLOW_SUMMARY_LOG_QUERYSTRING = True
+
+    request = rf.get("/?x=%D8%B4%D9%83%D8%B1")
+    response = dockerflow_middleware.process_request(request)
+    response = dockerflow_middleware.process_response(request, response)
+
+    assert len(caplog.records) == 1
+    record = caplog.records[0]
+    assert record.querystring == "x=شكر"
+    assert isinstance(record.t, int)
+
+
 def test_request_summary_admin_user(
     admin_user, caplog, dockerflow_middleware, dockerflow_request
 ):

tests/fastapi/test_fastapi.py

@@ -42,9 +42,11 @@ def test_lbheartbeat_head(client):
     assert response.content == b""
 
 
-def test_mozlog(client, caplog):
+def test_mozlog(app, client, caplog):
+    app.state.DOCKERFLOW_SUMMARY_LOG_QUERYSTRING = True
+
     client.get(
-        "/__lbheartbeat__",
+        "/__lbheartbeat__?x=شكر",
         headers={
             "User-Agent": "dockerflow/tests",
             "Accept-Language": "en-US",
@@ -58,6 +60,7 @@ def test_mozlog(client, caplog):
     assert record.method == "GET"
     assert record.code == 200
     assert record.path == "/__lbheartbeat__"
+    assert record.querystring == "x=شكر"
     assert isinstance(record.t, int)