diff --git a/temporalio/activity.py b/temporalio/activity.py index 417195d59..4e632701e 100644 --- a/temporalio/activity.py +++ b/temporalio/activity.py @@ -513,15 +513,14 @@ def process( if context: if self.activity_info_on_message: msg = f"{msg} ({context.logger_details})" - if self.activity_info_on_extra: - # Extra can be absent or None, this handles both - extra = kwargs.get("extra", None) or {} - extra["temporal_activity"] = context.logger_details - kwargs["extra"] = extra - if self.full_activity_info_on_extra: - # Extra can be absent or None, this handles both - extra = kwargs.get("extra", None) or {} - extra["activity_info"] = context.info() + if self.activity_info_on_extra or self.full_activity_info_on_extra: + # Copy any caller-provided extra rather than mutating it in + # place (see https://github.com/temporalio/sdk-python/issues/503). + extra = dict(kwargs.get("extra") or {}) + if self.activity_info_on_extra: + extra["temporal_activity"] = context.logger_details + if self.full_activity_info_on_extra: + extra["activity_info"] = context.info() kwargs["extra"] = extra return (msg, kwargs) diff --git a/tests/testing/test_activity.py b/tests/testing/test_activity.py index 2acf93639..71ba7f590 100644 --- a/tests/testing/test_activity.py +++ b/tests/testing/test_activity.py @@ -167,3 +167,47 @@ def my_activity() -> None: env = ActivityEnvironment(client=Mock(spec=Client)) env.run(my_activity) assert saw_error + + +async def test_activity_logger_does_not_mutate_caller_extra(): + """Regression test for https://github.com/temporalio/sdk-python/issues/503. + + The activity LoggerAdapter must not mutate the ``extra`` dict provided by + the caller; it should merge its temporal context into a fresh dict. + """ + import logging + import logging.handlers + import queue + + handler = logging.handlers.QueueHandler(queue.Queue()) + activity.logger.base_logger.addHandler(handler) + previous_level = activity.logger.base_logger.level + activity.logger.base_logger.setLevel(logging.INFO) + previous_full = activity.logger.full_activity_info_on_extra + activity.logger.full_activity_info_on_extra = True + + try: + + def log_with_extra() -> dict: + caller_extra = {"request_id": "req-1"} + activity.logger.info("hi", extra=caller_extra) + return caller_extra + + env = ActivityEnvironment() + caller_extra = env.run(log_with_extra) + finally: + activity.logger.base_logger.removeHandler(handler) + activity.logger.base_logger.setLevel(previous_level) + activity.logger.full_activity_info_on_extra = previous_full + + # The caller's dict must be untouched. + assert caller_extra == {"request_id": "req-1"} + + # But the emitted record should still carry the temporal-injected fields + # alongside the caller-provided one. + records: list[logging.LogRecord] = list(handler.queue.queue) # type: ignore[attr-defined] + assert records, "expected one log record" + record = records[-1] + assert record.__dict__["request_id"] == "req-1" + assert record.__dict__["temporal_activity"]["activity_type"] == "unknown" + assert "activity_info" in record.__dict__