Harden logger production compliance#118
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bea170a45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception: | ||
| if content_type.startswith('text/') or content_type in ( | ||
| 'application/xml', | ||
| 'application/x-www-form-urlencoded', |
There was a problem hiding this comment.
Mask form-encoded request bodies before logging
When a request uses Content-Type: application/x-www-form-urlencoded, this branch now returns the raw decoded body (for example password=secret&token=abc), but the later call is mask_sensitive_data(request_data) without mask_api_parameters=True, so string bodies are stored/emitted unchanged. This contradicts the new compliance/security docs that sensitive body keys are masked before database or signal delivery and leaks common form secrets whenever the response content type is logged.
Useful? React with 👍 / 👎.
| @@ -110,8 +262,13 @@ def __call__(self, request): | |||
| # Run only if logger is enabled. | |||
| if self.DRF_API_LOGGER_DATABASE or self.DRF_API_LOGGER_SIGNAL: | |||
There was a problem hiding this comment.
Include metrics and OTel in the middleware gate
With only DRF_API_LOGGER_ENABLE_METRICS=True or only DRF_API_LOGGER_ENABLE_OTEL=True (the README documents both as standalone settings), this guard is false unless database logging or signals are also enabled, so the middleware jumps straight to get_response() and never records metrics or starts/finishes spans. Users following the new metrics/OTel setup without also enabling DRF_API_LOGGER_SIGNAL/DRF_API_LOGGER_DATABASE get empty metrics and no traces.
Useful? React with 👍 / 👎.
| except Exception as e: | ||
| caught_exception = e | ||
| raise |
There was a problem hiding this comment.
Finish owned OTel spans when views raise
If OTel is enabled and get_response() raises an unhandled exception after start_span() created an owned span, this except immediately re-raises and the later finish_span(..., otel_span_owned, ...) call is skipped. In that scenario the span is never ended/exported and the error is not reflected in tracing; wrap the view call/logging path so owned spans are ended in a finally even on exceptions.
Useful? React with 👍 / 👎.
1bea170 to
1aa1308
Compare
Summary
This PR has been rebuilt from
mainand contains the production hardening/compliance changes, the package version bump to1.2.1, and a GitHub Actions workflow for PyPI publishing.DRF_API_LOGGER_CUSTOM_HANDLERandDRF_API_LOGGER_PROFILING_SAMPLE_RATE..github/workflows/publish.ymlto test/build packages and publish to PyPI frommainusing Trusted Publishing.Validation
J:\projects\drf-demo\venv\Scripts\python.exe -m django test tests.test_utils tests.test_middleware tests.test_signals tests.test_profiling tests.test_backward_compat --settings=tests.test_settings --verbosity=1- 111 tests OKJ:\projects\drf-demo\venv\Scripts\python.exe -m django test tests --settings=tests.test_settings --verbosity=1- 142 tests OKgit diff --check- OKgit diff --cached --checkbefore workflow commit - OKpython setup.py --namereturneddrf-api-loggerpython setup.py --versionreturned1.2.1Publish Behavior
The workflow runs tests and builds package artifacts for PRs and pushes. It publishes only when running on
refs/heads/main, uses thepypiGitHub environment, requestsid-token: write, checks that the package version is not already on PyPI, then usespypa/gh-action-pypi-publish@release/v1.