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

Cannot use PositionalArgumentsFormatter with FilteringBoundLogger #439

Closed
jerr0328 opened this issue Aug 31, 2022 · 7 comments · Fixed by #454
Closed

Cannot use PositionalArgumentsFormatter with FilteringBoundLogger #439

jerr0328 opened this issue Aug 31, 2022 · 7 comments · Fixed by #454

Comments

@jerr0328
Copy link
Contributor

I was switching to using make_filtering_bound_logger when I noticed that using a custom Gunicorn logger (that basically maps to structlog logging) wasn't working anymore. The issue is that the logger is doing something like log.info("Starting gunicorn %s", __version__) which then caused an error:

TypeError: _make_filtering_bound_logger.<locals>.make_method.<locals>.meth() takes 2 positional arguments but 3 were given

I can open a PR to simply pass through *args as well (in the level and log methods) which should clear up the error, though I don't know if this could cause downstream issues.

@jerr0328
Copy link
Contributor Author

This will need a bit more thought, perhaps make_filtering_bound_logger should move to stdlib so it can create a subclass of BoundLogger, rather than BoundLoggerBase? The BoundLogger class handles positional arguments properly so it can be used with PositionalArgumentsFormatter. It might make sense to move that code out of _log_levels.py anyways to avoid circular imports.

@hynek
Copy link
Owner

hynek commented Sep 10, 2022

I think I need more context here – how did you manage to make gunicorn use make_filtering_bound_logger? The whole point of it is to have very fast level filters if you don't use stdlib, which has level filters built in?

@jerr0328
Copy link
Contributor Author

Gunicorn allows you to specify a logger class to use: https://docs.gunicorn.org/en/stable/settings.html#logger-class, so by adapting it similar to what I found in this gist, I was able to have Gunicorn log with a structlog logger. However, I can't control how gunicorn calls their logger, and they still interface with it as if it was a stdlib logger.

@hynek
Copy link
Owner

hynek commented Oct 7, 2022

hey sorry for the delay – would you mind checking if #454 fixes this?

@jerr0328
Copy link
Contributor Author

Thanks, I haven't had a chance to fully test it but looking at the code, it looks like it should work! 🎉

@sshishov
Copy link

sshishov commented Nov 1, 2022

@hynek Can I ask one small question? When this changes will be available? Just approximately 😉

@hynek
Copy link
Owner

hynek commented Nov 2, 2022

The 22.2 is largely feature-complete, but I need to tie up a few strings in documentation (notably add a Celery recipe) and a bit of admin work. Shouldn't be too long, since I want it out too, but can't commit to a date.

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.

3 participants