temporalio / sdk-python

Temporal Python SDK
MIT License
457 stars 68 forks source link

[Bug] Activity logger context not populated when `activity_info_on_message` False #592

Closed TheCodeWrangler closed 2 months ago

TheCodeWrangler commented 2 months ago

What are you really trying to do?

I would like to have my logs include activity info on extra but not in the message

activity.logger.activity_info_on_message = False
activity.logger.activity_info_on_extra = True

However in practice after enabling a debugger when https://github.com/temporalio/sdk-python/blob/main/temporalio/activity.py#L462 is reached the value of context._logger_details is None/null

Running the same debug again with

activity.logger.activity_info_on_message = True
activity.logger.activity_info_on_extra = True

results in the debugger showing that context._logger_details is populated

Describe the bug

It should be possible to include extra context on activities without including them in the string of the message however in practice it does not seem to work.

Minimal Reproduction

Prior to running hello world on running the workers

from temporal import activity

activity.logger.activity_info_on_message = False
activity.logger.activity_info_on_extra = True

Run an activity which inclues activity.logger.info("some message") and observe that the logger results in "temporal_activity": null

cretz commented 2 months ago

Thanks for the report! This is a bug. We need to change https://github.com/temporalio/sdk-python/blob/c4b1a019ad2627edb20a954edfd57c9ba414f3d5/temporalio/activity.py#L462 to access logger_details instead of _logger_details since the former will lazily populate it (which was occurring when activity_info_on_message was True so the bug wasn't obvious before).

TheCodeWrangler commented 2 months ago

Thank you for quickly (nearly immediately) identifying the fix!

I posted a PR https://github.com/temporalio/sdk-python/pull/593