Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging the request object itself produces WSGIRequest is not JSON serializable #72

Closed
humitos opened this issue Nov 18, 2021 · 6 comments · Fixed by #73
Closed

Logging the request object itself produces WSGIRequest is not JSON serializable #72

humitos opened this issue Nov 18, 2021 · 6 comments · Fixed by #73

Comments

@humitos
Copy link

humitos commented Nov 18, 2021

Hi, thanks for this amazing django application!

While configuring it I saw that you are logging the request object as it in this line https://github.com/jrobichaud/django-structlog/blob/master/django_structlog/middlewares/request.py#L58 and in https://github.com/jrobichaud/django-structlog/blob/master/django_structlog/middlewares/request.py#L72 as well. This produces an error when trying to serialize it as JSON to be sent to an API from a custom processor for example:

(Pdb++) json.dumps({'request': event_dict['request']})
*** TypeError: Object of type WSGIRequest is not JSON serializable

I was wondering what was the reason to log the WSGIRequest as an object instead of the request.path or request.build_absolute_url(). Also, I didn't find a way to extend the middleware to be able to change how it logs these by default, so I just copy&paste it and modify it by myself 😄 --is there any other way to do it that I'm not seeing?

Does it make sense to log any of these attributes instead for you? Thanks!

@jrobichaud
Copy link
Owner

jrobichaud commented Nov 18, 2021

I wanted to have the method and path. (Ex: GET /some/path?foo=bar) and the whole object was giving me that.

If I "str()" the request it will fix your problem and it will preserve backward compatibility.

I will create another issue to do something more intelligent with the request.

jrobichaud added a commit that referenced this issue Nov 18, 2021
@humitos
Copy link
Author

humitos commented Nov 18, 2021

Thanks for your quick reply and PR! I'll definitely use the new static method to be able to override the formatting 💪🏼 . We need the full URI because we have multiple domains, so the full path is not enough.

I will create another issue to do something more intelligent with the request.

Yeah, probably this can be split into method= and path= or similar, which will allow us to easily filter by a method when querying logs.

@jrobichaud
Copy link
Owner

2.2.0 is currently building. It should take about 15-20 minutes.

@jrobichaud
Copy link
Owner

@humitos I am open to suggestions. I love to have feedback of people using it.

@jrobichaud
Copy link
Owner

@humitos , have you considered using the signal to add what is missing?

https://django-structlog.readthedocs.io/en/latest/getting_started.html#extending-request-log-metadata

@humitos
Copy link
Author

humitos commented Nov 23, 2021

@humitos , have you considered using the signal to add what is missing?

Yes, I'm already using it to add other fields. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants