process PR feedback

.github/workflows/ci.yml

@@ -15,14 +15,21 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python: ["3.7", "3.8", "3.9", "3.10"]
+        python: ["3.8", "3.9", "3.10"]
         django: ["3.2", "4.1"]
-        exclude:
-          - python: "3.7"
-            django: "4.1"
 
     name: Run the test suite (Python ${{ matrix.python }}, Django ${{ matrix.django }})
 
+    services:
+      postgres:
+        image: postgres:14
+        env:
+          POSTGRES_HOST_AUTH_METHOD: trust
+        ports:
+          - 5432:5432
+        # needed because the postgres container does not provide a healthcheck
+        options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
+
     steps:
       - uses: actions/checkout@v3
       - uses: actions/setup-python@v4

README.rst

@@ -97,8 +97,16 @@ To use this with your project you need to follow these steps:
         }
 
         LOG_OUTGOING_REQUESTS_DB_SAVE = True # save logs enabled/disabled based on the boolean value
-        LOG_OUTGOING_REQUESTS_SAVE_BODY = True # save request/response body
-        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True # log request/response body to STDOUT
+        LOG_OUTGOING_REQUESTS_DB_SAVE_BODY = True # save request/response body
+        LOG_OUTGOING_REQUESTS_EMIT_BODY = True # log request/response body
+        LOG_OUTGOING_REQUESTS_CONTENT_TYPES = [
+                "text/*",
+                "application/json",
+                "application/xml",
+        ] # save request/response bodies with matching content type
+        LOG_OUTGOING_REQUESTS_MAX_CONTENT_LENGTH = 524_288  # maximal size (in bytes) for the request/response body
+        LOG_OUTGOING_REQUESTS_REQUIRE_CONTENT_LENGTH = True # if False, the content size of request/response bodies is not checked
+        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True
 
 
 #.  Run the migrations
@@ -115,8 +123,9 @@ To use this with your project you need to follow these steps:
         res = requests.get("https://httpbin.org/json")
         print(res.json())
 
-#.  Check stdout for the printable output, and navigate to ``/admin/log_outgoing_requests/outgoingrequestslog/`` to see 
-    the saved log records. The settings for saving logs can by overridden under ``/admin/log_outgoing_requests/outgoingrequestslogconfig/``.
+#.  Check stdout for the printable output, and navigate to ``Admin > Miscellaneous > Outgoing Requests Logs``
+    to see the saved log records. In order to override the settings for saving logs, navigate to
+    ``Admin > Miscellaneous > Outgoing Requests Log Configuration``.
 
 
 Local development

docs/quickstart.rst

@@ -58,8 +58,16 @@ Installation
         }
 
         LOG_OUTGOING_REQUESTS_DB_SAVE = True # save logs enabled/disabled based on the boolean value
-        LOG_OUTGOING_REQUESTS_SAVE_BODY = True # save request/response body
-        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True # log request/response body to STDOUT
+        LOG_OUTGOING_REQUESTS_DB_SAVE_BODY = True # save request/response body
+        LOG_OUTGOING_REQUESTS_EMIT_BODY = True # log request/response body
+        LOG_OUTGOING_REQUESTS_CONTENT_TYPES = [
+                "text/*",
+                "application/json",
+                "application/xml",
+        ] # save request/response bodies with matching content type
+        LOG_OUTGOING_REQUESTS_MAX_CONTENT_LENGTH = 524_288  # maximal size (in bytes) for the request/response body
+        LOG_OUTGOING_REQUESTS_REQUIRE_CONTENT_LENGTH = True # if True, the content size of request/response bodies is not checked
+        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True
 
 
 #.  Run ``python manage.py migrate`` to create the necessary database tables.

log_outgoing_requests/admin.py

@@ -1,3 +1,5 @@
+from django import forms
+from django.conf import settings
 from django.contrib import admin
 from django.utils.translation import gettext as _
 
@@ -6,11 +8,6 @@
 from .models import OutgoingRequestsLog, OutgoingRequestsLogConfig
 
 
-@admin.display(description="Response body")
-def response_body(obj):
-    return f"{obj}".upper()
-
-
 @admin.register(OutgoingRequestsLog)
 class OutgoingRequestsLogAdmin(admin.ModelAdmin):
     fields = (
@@ -62,13 +59,33 @@ class Media:
         }
 
 
+class ConfigAdminForm(forms.ModelForm):
+    class Meta:
+        model = OutgoingRequestsLogConfig
+        fields = "__all__"
+        widgets = {"allowed_content_types": forms.CheckboxSelectMultiple}
+        help_texts = {
+            "save_to_db": _(
+                "Whether request logs should be saved to the database (default: {default})."
+            ).format(default=settings.LOG_OUTGOING_REQUESTS_DB_SAVE),
+            "save_body": _(
+                "Wheter the body of the request and response should be logged (default: "
+                "{default}). This option is ignored if 'Save Logs to database' is set to "
+                "False."
+            ).format(default=settings.LOG_OUTGOING_REQUESTS_DB_SAVE_BODY),
+            "require_content_length": _(
+                "Whether request & response headers must specify the content length or not. "
+                "If checked, the body of requests/responses whose headers do not specify "
+                "a content length will not be saved. Default: {default}."
+            ).format(default=settings.LOG_OUTGOING_REQUESTS_REQUIRE_CONTENT_LENGTH),
+        }
+
+
 @admin.register(OutgoingRequestsLogConfig)
 class OutgoingRequestsLogConfigAdmin(SingletonModelAdmin):
-    fields = (
-        "save_to_db",
-        "save_body",
-    )
-    list_display = (
-        "save_to_db",
-        "save_body",
-    )
+    form = ConfigAdminForm
+
+    class Media:
+        css = {
+            "all": ("log_outgoing_requests/css/admin.css",),
+        }

log_outgoing_requests/constants.py

@@ -0,0 +1,17 @@
+from django.db import models
+from django.utils.translation import gettext_lazy as _
+
+
+class SaveLogsChoice(models.TextChoices):
+    use_default = "use_default", _("Use default")
+    yes = "yes", _("Yes")
+    no = "no", _("No")
+
+
+class ContentType(models.TextChoices):
+    audio = "audio/*", _("Audio")
+    form_data = "multipart/form-data", _("Form data")
+    json = "application/json", _("JSON")
+    text = "text/*", ("Plain text & HTML")
+    video = "video/*", _("Video")
+    xml = "application/xml", _("XML")

log_outgoing_requests/formatters.py

@@ -8,32 +8,37 @@ class HttpFormatter(logging.Formatter):
     def _formatHeaders(self, d):
         return "\n".join(f"{k}: {v}" for k, v in d.items())
 
-    def _formatBody(self, content: dict, request_or_response: str) -> str:
+    def _formatBody(self, content: str, request_or_response: str) -> str:
         if settings.LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT:
             return f"\n{request_or_response} body:\n{content}"
         return ""
 
     def formatMessage(self, record):
         result = super().formatMessage(record)
-        if record.name == "requests":
-            result += textwrap.dedent(
-                """
-                ---------------- request ----------------
-                {req.method} {req.url}
-                {reqhdrs} {request_body}
 
-                ---------------- response ----------------
-                {res.status_code} {res.reason} {res.url}
-                {reshdrs} {response_body}
+        if record.name != "requests":
+            return result
 
+        result += textwrap.dedent(
             """
-            ).format(
-                req=record.req,
-                res=record.res,
-                reqhdrs=self._formatHeaders(record.req.headers),
-                reshdrs=self._formatHeaders(record.res.headers),
-                request_body=self._formatBody(record.req.body, "Request"),
-                response_body=self._formatBody(record.res.json(), "Response"),
-            )
+            ---------------- request ----------------
+            {req.method} {req.url}
+            {reqhdrs} {request_body}
+
+            ---------------- response ----------------
+            {res.status_code} {res.reason} {res.url}
+            {reshdrs} {response_body}
+
+        """
+        ).format(
+            req=record.req,
+            res=record.res,
+            reqhdrs=self._formatHeaders(record.req.headers),
+            reshdrs=self._formatHeaders(record.res.headers),
+            request_body=self._formatBody(record.req.body, "Request"),
+            response_body=self._formatBody(
+                record.res.content.decode("utf-8"), "Response"
+            ),
+        )
 
         return result

log_outgoing_requests/handlers.py

@@ -2,47 +2,26 @@
 import traceback
 from urllib.parse import urlparse
 
-from django.conf import settings
-
-ALLOWED_CONTENT_TYPES = [
-    "application/json",
-    "multipart/form-data",
-    "text/html",
-    "text/plain",
-    "",
-    None,
-]
+from .utils import is_content_admissible_for_logging
 
 
 class DatabaseOutgoingRequestsHandler(logging.Handler):
     def emit(self, record):
-        from .models import OutgoingRequestsLogConfig
+        from .models import OutgoingRequestsLog, OutgoingRequestsLogConfig
 
         config = OutgoingRequestsLogConfig.get_solo()
 
-        if config.save_to_db or settings.LOG_OUTGOING_REQUESTS_DB_SAVE:
-            from .models import OutgoingRequestsLog
-
-            trace = None
+        if config.save_logs_enabled:
+            trace = ""
 
             # skip requests not coming from the library requests
             if not record or not record.getMessage() == "Outgoing request":
                 return
 
-            # skip requests with non-allowed content
-            request_content_type = record.req.headers.get("Content-Type", "")
-            response_content_type = record.res.headers.get("Content-Type", "")
-
-            if not (
-                request_content_type in ALLOWED_CONTENT_TYPES
-                and response_content_type in ALLOWED_CONTENT_TYPES
-            ):
-                return
-
-            safe_req_headers = record.req.headers.copy()
+            scrubbed_req_headers = record.req.headers.copy()
 
-            if "Authorization" in safe_req_headers:
-                safe_req_headers["Authorization"] = "***hidden***"
+            if "Authorization" in scrubbed_req_headers:
+                scrubbed_req_headers["Authorization"] = "***hidden***"
 
             if record.exc_info:
                 trace = traceback.format_exc()
@@ -54,18 +33,31 @@ def emit(self, record):
                 "params": parsed_url.params,
                 "status_code": record.res.status_code,
                 "method": record.req.method,
-                "req_content_type": record.req.headers.get("Content-Type", ""),
-                "res_content_type": record.res.headers.get("Content-Type", ""),
+                "req_content_type": (
+                    request_content_type := record.req.headers.get("Content-Type", "")
+                ),
+                "res_content_type": (
+                    response_content_type := record.res.headers.get("Content-Type", "")
+                ),
                 "timestamp": record.requested_at,
                 "response_ms": int(record.res.elapsed.total_seconds() * 1000),
-                "req_headers": self.format_headers(safe_req_headers),
+                "req_headers": self.format_headers(scrubbed_req_headers),
                 "res_headers": self.format_headers(record.res.headers),
                 "trace": trace,
             }
 
-            if config.save_body or settings.LOG_OUTGOING_REQUESTS_SAVE_BODY:
-                kwargs["req_body"] = (record.req.body,)
-                kwargs["res_body"] = (record.res.json(),)
+            if config.save_body_enabled:
+                request_content_length = record.req.headers.get("Content-Length")
+                if is_content_admissible_for_logging(
+                    request_content_length, request_content_type, config
+                ):
+                    kwargs["req_body"] = record.req.text or ""
+
+                response_content_length = record.res.headers.get("Content-Length")
+                if is_content_admissible_for_logging(
+                    response_content_length, response_content_type, config
+                ):
+                    kwargs["res_body"] = record.res.text
 
             OutgoingRequestsLog.objects.create(**kwargs)