-
Notifications
You must be signed in to change notification settings - Fork 33
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
user_id always None for DRF requests when using RequestMiddleware #37
Comments
Can this be because of the order of the middlewares in |
In the demo of django-structlog it is last, after the authentication middleware. |
I'm afraid it's not that simple, the middleware is actually already one of the last ones (only followed by the celery middleware):
Maybe something else that I didn't make clear is that I'm using DRF's |
I found people encountering the same DRF issue but in a different context here: jpadilla/django-rest-framework-jwt#45 (comment) As far as I can find |
DRF token authentication is not tested in django-structlog and now that you mention this issue I just remembered DRF handle the authentication in its view not in middleware. It does work with DRF and django session authentication but it is not how drf is used normally. 🤦 I will investigate this and try to figure out a workaround. But a quick workaround would be to implement a mixin or an intermediary view wrapping DRF authentication then bind/log the user. It is not ideal but it would work. 🤷 |
Ok, thanks for clarifying. The problem with an intermediary view is that the code would be executed after Alternatively, I could implement a middleware that executes after |
There are no optimal solution until DRF fix the issue you mentioned. You have to choose your poison until then. I will update the documentation to reflect this issue. I would appreciate any pull requests or just code snippets to help working around this. |
I wouldn't really know how to integrate this properly with the existing code base, but I can share the solution I've implemented. I went for the mixin approach, wrapping each of DRF's APIView and subclasses. I added this to my app's views.py file:
Then, instead of using DRF's
becomes
Then you also need to make sure that the threadlocal context is cleared after every request because threads might be reused for different HTTP request. I did this by adding a middleware:
I add this right before the first middleware that logs information (in my case this is django-structlog's request middleware):
Finally, I also added
Note that this still has the downside of logging |
In django-structlog we could just log the user_id in the request finish as well (or with a setting). You would not have do anything fancy. What do you think? |
Hmm, I don't know. That would mean that user_id is always None except for the On the other hand you could argue that this isn't django-structlog's responsibility because DRF doesn't support hooking into the view layer to support cases like these (afaik). In that case, binding a user_id a second time is probably the only thing you can do. |
@jrobichaud How do we log any variable (eg: user_id) in the |
It is not supported at the moment, you can create an issue for this. |
I made the workaround we discussed, a setting to add the user_id in the request finished. |
In 1.5.3 I added a setting to add |
Have you considered evaluating DRF authentication classes in the middleware and then setting the user based on that? That way all log events will contain the Here is a simplified version: @receiver([bind_extra_request_metadata, bind_extra_request_finished_metadata, bind_extra_request_failed_metadata])
def bind_user(request, logger, **kwargs):
"""
Bind Django/REST Framework user to the logger.
"""
# Retrieve DRF user from auth headers if it is not already cached.
token_user: Optional[TokenUser] = getattr(request, "_drf_token_user", None)
if not hasattr(request, "_drf_token_user"):
try:
auth = JWTTokenUserAuthentication().authenticate(request)
if auth:
token_user = auth[0]
except (TokenError, AuthenticationFailed):
pass
# Cache token user to prevent unnecessary JWT decode
request._drf_token_user = token_user
# Bind DRF token user if available.
if token_user is not None:
logger.bind(user_id=token_user.pk)
# Fallback to django auth
elif request.user:
logger.bind(user_id=request.user.pk) To make it more generic we can read from A downside of this is that it is somewhat opinionated and adds overhead of processing authentication twice. |
It is an interesting solution, however views may redefine the authentication class. |
@v1k45 could you try to the class from the view? I would add this workaround in the documentation or in a helper module. |
In my Django application I'm using the
django_structlog.middlewares.RequestMiddleware
to log all of the requests to my application.I noticed that
user_id
is alwaysNone
for all the calls made to my Django Rest Framework (DRF) urls.Now, I know this is because
RequestMiddleware
tries to accessrequest.user
, which is only filled in when you're going through the Django authentication mechanism, which DRF bypasses completely. Instead, it performs authentication in the view layer, so after all middleware code has run.I guess other people have encountered this too so I was wondering if there is any workaround so that I can track which user performed which request from "the start" until "the end" of the request?
I tried several things, but all of them feel like I'm fighting with DRF to get it to do something it doesn't support.
The text was updated successfully, but these errors were encountered: