Update log format to include X-Forwarded-For #605
Conversation
Updating deployment configurations
jonespm
force-pushed
the
issue_359-2
branch
3 times, most recently
from
f3f5bb1 to
506cc97
Compare
|
I think this is ready for review. Running this locally should give you logs for both access and server that are in a similar format. If you run this |
|
Deploying to prod will need to get the new secret FORWARDED_ALLOW_IPS as only some IP's are allowed to be forwarded. |
|
Is there any changes needed in the .env file? I got " Error: Got unexpected extra arguments (# Trust all IPs in development for testing)" and the app is not up running |
|
No, it doesn't need any locally as * is in the docker-compose, it just looks like it's pulling the comments into the array, I'll move that 😁 |
| record.request = f"{method} {path} HTTP/{http_version}" | ||
| record.status = status | ||
| else: | ||
| # Regular Python log |
There was a problem hiding this comment.
I wonder whether the regular Python log can be of any usage. Currently I only see patterns of:
web-1 | 2025-01-23 14:03:29,760 [DEBUG] [-] "% received keepalive pong" 760ms
web-1 | 2025-01-23 14:03:49,764 [DEBUG] [-] "% sending keepalive ping" 764ms
web-1 | 2025-01-23 14:03:49,764 [DEBUG] [-] "> %s" 764ms
web-1 | 2025-01-23 14:03:49,766 [DEBUG] [-] "< %s" 766ms
web-1 | 2025-01-23 14:02:29,752 [INFO] [-] "connection open" 752ms
web-1 | 2025-01-23 14:02:29,752 [DEBUG] [-] "= connection is OPEN" 752ms
There was a problem hiding this comment.
Without this patch current format looks like
[2025-01-23 19:37:09 -0500] [12] [INFO] ('1.2.3.4', 53268) - "WebSocket /ws/users/59784/" [accepted]
[2025-01-23 19:37:09 -0500] [12] [INFO] connection open
[2025-01-23 19:37:18 -0500] [12] [INFO] connection closed
These noisy DEBUG messages here shouldn't be displayed if "DEBUG=False". You could test this.
So new consistent format in this patch for Python and access logs is
2025-01-23 14:02:29,752 [INFO] [-] "connection open" 752ms
The [-] would have an IP address if it was from the web with the number being like 200 or 404.
Which is similar but not identicalto the old format
[2025-01-23 19:37:09 -0500] [12] [INFO] connection open
Happy to make additional adjustments if we want.
Some changes -
- The time looks like it was changed to remove the TZ information
- The time it took to execute the request is attached now
- The message is quoted
- There is no process ID
jonespm
added
the
config change
There was a problem hiding this comment.
I like these changes. I'd like to suggest that before merging, make sure you're happy with the log format, too. If possible, make the information in the line as compact as possible if you want to leave more line space available for the actual log message. In the timestamps, removing TZ info helps. Maybe removing the milliseconds would help, too, if the server doesn't seem busy enough to warrant it.
Fixes #359