Skip to content

Commit 776998d

Browse files
Do not mutate caller's extra dict in activity LoggerAdapter (#1513)
The activity LoggerAdapter mutated the caller-provided ``extra`` mapping in place when injecting ``temporal_activity`` and ``activity_info``. If the caller reused the dict across log calls (or held a reference to it elsewhere), Temporal context would leak back into their state. Copy the caller's extra into a fresh dict before adding our keys. This matches what the workflow LoggerAdapter already does via ``_build_log_context``. Fixes #503 Signed-off-by: 1fanwang <1fannnw@gmail.com> Co-authored-by: tconley1428 <tconley1428@gmail.com>
1 parent b5777e9 commit 776998d

2 files changed

Lines changed: 52 additions & 9 deletions

File tree

temporalio/activity.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -513,15 +513,14 @@ def process(
513513
if context:
514514
if self.activity_info_on_message:
515515
msg = f"{msg} ({context.logger_details})"
516-
if self.activity_info_on_extra:
517-
# Extra can be absent or None, this handles both
518-
extra = kwargs.get("extra", None) or {}
519-
extra["temporal_activity"] = context.logger_details
520-
kwargs["extra"] = extra
521-
if self.full_activity_info_on_extra:
522-
# Extra can be absent or None, this handles both
523-
extra = kwargs.get("extra", None) or {}
524-
extra["activity_info"] = context.info()
516+
if self.activity_info_on_extra or self.full_activity_info_on_extra:
517+
# Copy any caller-provided extra rather than mutating it in
518+
# place (see https://github.com/temporalio/sdk-python/issues/503).
519+
extra = dict(kwargs.get("extra") or {})
520+
if self.activity_info_on_extra:
521+
extra["temporal_activity"] = context.logger_details
522+
if self.full_activity_info_on_extra:
523+
extra["activity_info"] = context.info()
525524
kwargs["extra"] = extra
526525
return (msg, kwargs)
527526

tests/testing/test_activity.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,3 +167,47 @@ def my_activity() -> None:
167167
env = ActivityEnvironment(client=Mock(spec=Client))
168168
env.run(my_activity)
169169
assert saw_error
170+
171+
172+
async def test_activity_logger_does_not_mutate_caller_extra():
173+
"""Regression test for https://github.com/temporalio/sdk-python/issues/503.
174+
175+
The activity LoggerAdapter must not mutate the ``extra`` dict provided by
176+
the caller; it should merge its temporal context into a fresh dict.
177+
"""
178+
import logging
179+
import logging.handlers
180+
import queue
181+
182+
handler = logging.handlers.QueueHandler(queue.Queue())
183+
activity.logger.base_logger.addHandler(handler)
184+
previous_level = activity.logger.base_logger.level
185+
activity.logger.base_logger.setLevel(logging.INFO)
186+
previous_full = activity.logger.full_activity_info_on_extra
187+
activity.logger.full_activity_info_on_extra = True
188+
189+
try:
190+
191+
def log_with_extra() -> dict:
192+
caller_extra = {"request_id": "req-1"}
193+
activity.logger.info("hi", extra=caller_extra)
194+
return caller_extra
195+
196+
env = ActivityEnvironment()
197+
caller_extra = env.run(log_with_extra)
198+
finally:
199+
activity.logger.base_logger.removeHandler(handler)
200+
activity.logger.base_logger.setLevel(previous_level)
201+
activity.logger.full_activity_info_on_extra = previous_full
202+
203+
# The caller's dict must be untouched.
204+
assert caller_extra == {"request_id": "req-1"}
205+
206+
# But the emitted record should still carry the temporal-injected fields
207+
# alongside the caller-provided one.
208+
records: list[logging.LogRecord] = list(handler.queue.queue) # type: ignore[attr-defined]
209+
assert records, "expected one log record"
210+
record = records[-1]
211+
assert record.__dict__["request_id"] == "req-1"
212+
assert record.__dict__["temporal_activity"]["activity_type"] == "unknown"
213+
assert "activity_info" in record.__dict__

0 commit comments

Comments
 (0)