[QUESTION] Is merge_contextvars required in the processor chain for AsyncBoundLogger or not?

[SanchithHegde]

From AsyncBoundLogger's docstring (FYI, the warning is not rendered on the API reference page):

.. warning: Since the processor pipeline runs in a separate thread,
    `structlog.contextvars.merge_contextvars` does **nothing** and should
    be removed from you processor chain.

Following the aforementioned warning, if I remove structlog.contextvars.merge_contextvars from my processor chain, none of the context variables (added using structlog.contextvars.bind_contextvars) are displayed in the log records. Only if I add it to the processor chain, context variables are displayed. Am I doing something wrong?

My structlog related code:

shared_processors: List[structlog.types.Processor] = [
    structlog.stdlib.add_logger_name,
    structlog.stdlib.add_log_level,
    add_request_id,  # Custom processor to add request ID to `event_dict` using `starlette_context`
    structlog.processors.TimeStamper(fmt="iso", utc=True),
    structlog.processors.StackInfoRenderer(),
    structlog.processors.format_exc_info,
]

structlog.configure(
    processors=[
        # structlog.contextvars.merge_contextvars,
        structlog.stdlib.filter_by_level,
        *shared_processors,
        structlog.stdlib.PositionalArgumentsFormatter(),
        structlog.stdlib.ProcessorFormatter.wrap_for_formatter,
    ],
    context_class=dict,
    logger_factory=structlog.stdlib.LoggerFactory(),
    wrapper_class=structlog.stdlib.AsyncBoundLogger,
    cache_logger_on_first_use=True,
)

Python version: 3.9.6
structlog version: 21.1.0

If any more info is required from my end, let me know, I'll provide it.

[hynek]

Wow, this is super interesting and most likely related to #302! It's kinda hilarious that the screwed up markup probably saved us from many broken deployments! 😅

It would be great if @eladnam could weight in real quick, because I understand why

structlog/src/structlog/stdlib.py

Lines 478 to 482 in 4c19f0c

	ctx = contextvars.copy_context()
	await self._loop.run_in_executor(
	self._executor, partial(ctx.run, partial(meth, event, *args, **kw))
	)

doesn't work anymore, but I find it entirely baffling why merge_contextvars works now when it didn't before!? 🤯

[eladnam]

Wow, this is super interesting and most likely related to #302! It's kinda hilarious that the screwed up markup probably saved us from many broken deployments! 😅

It would be great if @eladnam could weight in real quick, because I understand why

structlog/src/structlog/stdlib.py

Lines 478 to 482 in 4c19f0c

	ctx = contextvars.copy_context()
	await self._loop.run_in_executor(
	self._executor, partial(ctx.run, partial(meth, event, *args, **kw))
	)

doesn't work anymore, but I find it entirely baffling why merge_contextvars works now when it didn't before!? 🤯

Part of my fix was to send the context to the thread pool executor used in the async logger (the ctx.run call does the trick), so that merge_contextvars should work perfectly with the async logger.

@hynek i think you can simply remove the outdated warning,
and @SanchithHegde you can add the merge_contextvars function to your processor chain, there is no reason why it won't work

[SanchithHegde]

@hynek and @eladnam, Thank you, both for your quick responses! I'll close this issue.

[hynek]

Wonderful, thank you @eladnam!