SS-1250 Remove ip addresses from logs #364
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes IP addresses from structured logs by clearing the ip field in request metadata.
- Adds a signal handler to unset the
ipcontext variable. - Registers the new signal handler in the app’s
ready()method.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| portal/signals.py | Introduce remove_ip_address handler to clear ip. |
| portal/apps.py | Import portal.signals in AppsConfig.ready(). |
Comments suppressed due to low confidence (1)
portal/signals.py:7
- [nitpick] Consider adding a unit test to verify that the
ipfield is indeed cleared from structured logs when this handler runs.
@receiver(bind_extra_request_metadata)
portal/signals.py
Outdated
|
|
||
|
|
||
| @receiver(bind_extra_request_metadata) | ||
| def remove_ip_address(request, logger, **kwargs): |
There was a problem hiding this comment.
The signal handler signature is missing the sender parameter. It should be defined as def remove_ip_address(sender, request, logger, **kwargs): so arguments are aligned correctly.
| def remove_ip_address(request, logger, **kwargs): | |
| def remove_ip_address(sender, request, logger, **kwargs): |
Copilot uses AI. Check for mistakes.
|
|
||
| @receiver(bind_extra_request_metadata) | ||
| def remove_ip_address(request, logger, **kwargs): | ||
| structlog.contextvars.bind_contextvars(ip=None) |
There was a problem hiding this comment.
Signal handlers for bind_extra_request_metadata should return a dict of metadata (e.g., return {"ip": None}) instead of only binding contextvars, otherwise the ip field may not be removed as intended.
| structlog.contextvars.bind_contextvars(ip=None) | |
| return {"ip": None} |
Copilot uses AI. Check for mistakes.
|
Hi @churnikov, I just wondered if this means that no IP addresses will show up in the Loki logs after this change? |
@j-awada |
That makes sense, thanks! |
Add explicit flag for development logs Signed-off-by: Nikita Churikov <[email protected]>
|
Please also review this related PR |
Description
Remove ip addresses from logs.
This should work according to https://betterstack.com/community/guides/logging/structlog#installing-and-configuring-django-structlog
Checklist
If you're unsure about any of the items below, don't hesitate to ask. We're here to help!
This is simply a reminder of what we are going to look for before merging your code.
Further comments
Anything else you think we should know before merging your code!