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

stacklevel incorrectly passed as extra in structlog.stdlib.render_to_log_kwargs (typo) #619

Closed
fraser-langton opened this issue May 15, 2024 · 2 comments · Fixed by #620
Closed

Comments

@fraser-langton
Copy link
Contributor

fraser-langton commented May 15, 2024

render_to_log_kwargs reserves stackLevel when it should be stacklevel

python stdlib kwargs https://github.com/python/cpython/blob/main/Lib/logging/__init__.py#L1632

import structlog

structlog.configure(
    processors=[
        structlog.stdlib.render_to_log_kwargs,
    ],
    cache_logger_on_first_use=True,
)

log = structlog.get_logger()

log.info("test", stacklevel=2)  # doesn't error, but doesn't pass stacklevel to stdlib

log.info("test", stackLevel=2)  # errors as it passes stackLevel to stdlib
Traceback (most recent call last):
  File "C:\Users\user\Path\to\file.py", line 12, in <module>
    log.info("test", stacklevel=2)
  File "C:\Users\user\VirtualEnv\project\lib\site-packages\structlog\_log_levels.py", line 157, in meth
    return self._proxy_to_logger(name, event, **kw)
  File "C:\Users\user\VirtualEnv\project\lib\site-packages\structlog\_base.py", line 206, in _proxy_to_logger
    return getattr(self._logger, method_name)(*args, **kw)
TypeError: msg() got an unexpected keyword argument 'msg'

related
#537
#424

@hynek
Copy link
Owner

hynek commented May 20, 2024

Ah shit thanks!

I'm trying to come up if there's a legitimate reason to add backwards-compatible change (i.e. support both, and translate the one to the other), or are the existing "use-cases" so thoroughly broken, that it just doesn't make any sense whatsoever?

edit removed a trace that I completely misread while being high on PyCon fumes please disregard

@fraser-langton
Copy link
Contributor Author

I do not think the backwards-compatible change should be supported no, given one case will cause an error (ie using stackLevel) and the other case (using stacklevel) needs to be interpreted correctly and passed to stdlib

Maybe there is an argument for passing both the argument to stdlib kwargs and in extra? Could turn the processor into a class so that could be parameterised, and the old one can be supported by render_to_log_kwargs = RenderToLogKwargs(pass_to_extra=True) - but I don't think it's worth adding the complexity to cover what I imagine is a vanishingly small use case

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