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

Allow configure log context for request_* events #484

Closed
shtoltz opened this issue Mar 13, 2024 · 3 comments · Fixed by #485
Closed

Allow configure log context for request_* events #484

shtoltz opened this issue Mar 13, 2024 · 3 comments · Fixed by #485

Comments

@shtoltz
Copy link

shtoltz commented Mar 13, 2024

Hello!

I need to configure context for request_started/request_finishedevents, for example to add additional headers or to split request context value into request_method and request_url. Or even log request and response body or headers in some cases. At the moment I can't do this without copy-paste and modify prepare and handle_response methods in my own class which inherits from RequestMiddleware.

I know about bind_extra_request_metadata and bind_extra_request_finished_metadata signals, but they add values to global context and I don't want these values to be global.

Maybe something like this:

class RequestMiddleware:

    def handle_response(self, request, response):
        if not hasattr(request, "_raised_exception"):
            ...

            log = self.bind_request_finish_context(logger, request, response)
            log.log(level, "request_finished")
            ...

    def prepare(self, request):
        ...

        log = self.bind_request_start_context(logger, request)
        log.info("request_started")

    def process_exception(self, request, exception):
        ...

        log = self.bind_request_failed_context(logger, request)
        log.exception("request_failed")

    # new methods
    def bind_request_start_context(self, _logger, request):
        return _logger.bind(
            request=self.format_request(request),
            user_agent=request.META.get("HTTP_USER_AGENT"),
        )

    def bind_request_finish_context(self, _logger, request, response):
        return _logger.bind(
            code=response.status_code,
            request=self.format_request(request),
        )

    def bind_request_failed_context(self, _logger, request):
        return _logger.bind(
            code=500,
            request=self.format_request(request),
        )

Then in my own class I will be able to do this:

from django_structlog.middlewares.request import RequestMiddleware as _RequestMiddleware

class RequestMiddleware(_RequestMiddleware):

    def bind_request_start_context(self, _logger, request: HttpRequest):
        return _logger.bind(
            request_url=request.get_full_path(),
            request_method=request.method,
            # add what you need
        )

    def bind_request_finish_context(self, _logger, request: HttpRequest, response: HttpResponse):
        return _logger.bind(
            response_status_code=response.status_code,
            # add what you need
        )
    ...

I can make a PR if you OK with this idea.
Thanks.

@jrobichaud
Copy link
Owner

You still could bind values for the finish and failure because the bind will persist only for the last log.

However it needs a solution for the request started.

I was not planning to support the subclassing of the RequestMiddleware yet but I will do it officially eventually. I want appropriate tests and a decent interface. Subclassing support means that any future change could break the setup of users. I don't like that aspect.

I don't want people to subclass it unless they need to do major advanced changes.

I'd like to find a solution for your use case with simpler implementation. Ex: adding an extra dict to the signals that will be added to the log of the event.

Any other suggestion?

@shtoltz
Copy link
Author

shtoltz commented Mar 14, 2024

Ex: adding an extra dict to the signals that will be added to the log of the event.

Sounds like a good idea. And if this extra dict would contains the current context from logging methods (e.g. request, user_agent from request_started), it could be easy to remove or modify.

@jrobichaud
Copy link
Owner

@shtoltz can you take a look at #485 please.

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