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

render_to_log_kwargs does not handle all kwargs #424

Closed
last-partizan opened this issue Jun 23, 2022 · 3 comments · Fixed by #427
Closed

render_to_log_kwargs does not handle all kwargs #424

last-partizan opened this issue Jun 23, 2022 · 3 comments · Fixed by #427

Comments

@last-partizan
Copy link
Contributor

last-partizan commented Jun 23, 2022

Hi, i want to use standard logging for formatting, to use formatting from https://github.com/elastic/ecs-logging-python

But, following configuration:

structlog.configure(
    processors=[
        # Transform event dict into `logging.Logger` method arguments.
        # "event" becomes "msg" and the rest is passed as a dict in
        # "extra". IMPORTANT: This means that the standard library MUST
        # render "extra" for the context to appear in log entries! See
        # warning below.
        structlog.stdlib.render_to_log_kwargs,
    ],
    logger_factory=structlog.stdlib.LoggerFactory(),
    wrapper_class=structlog.stdlib.BoundLogger,
    cache_logger_on_first_use=True,
)

Causes errors when using structlog in the following way:

logger = structlog.getLogger()
try:
    assert False
except:
    logger.error("Error happened", exc_info=True)

... 
KeyError: "Attempt to overwrite 'exc_info' in LogRecord"

This is because not all kwargs to logging should go to extra.

I fixed this by rewriting render_to_log_kwargs in the following way:

def _render_to_log_kwargs(_, __, event_dict: EventDict) -> EventDict:
    required = {
        "msg": event_dict.pop("event"),
    }
    optional = {
        k: event_dict.pop(k)
        for k in
        ["exc_info", "stack_info", "stackLevel"]
        if k in event_dict
    }
    return required | optional | {
        "extra": event_dict,
    }

But i think this should be merged upstream. I can make PR if you want, or you can add those changes yourself.

@eoinmorgan
Copy link

+1, I also hit the exact same issue. I'm trying to use the stdlib.LoggerFactory and structlog.stdlib.ProcessorFormatter to defer all of the log record manipulation into the stdlib logging, but using the structlog processors (so that 3rd party libs that use logging.getLogger() have consistent formatting with my own code's logs). The stdlib.BoundLogger() calls with exc_info in the extra kwarg :/ Thanks for your help

@hynek
Copy link
Owner

hynek commented Jul 6, 2022

Hm this never popped up, because you'd usually format exceptions using format_exc_info before handing them off to standard library.

I guess we could sieve out keywords that must not be passed on, are there more than exc_info?

@last-partizan
Copy link
Contributor Author

Yeah, adding format_exc_info fixes this.

I don't think sieving out is correct, we need to pass actual arguments. Three of them: "exc_info", "stack_info", "stackLevel".

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