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

TimeStamper ignores utc flag for custom format string #712

Open
m-endra opened this issue Mar 14, 2025 · 2 comments
Open

TimeStamper ignores utc flag for custom format string #712

m-endra opened this issue Mar 14, 2025 · 2 comments

Comments

@m-endra
Copy link

m-endra commented Mar 14, 2025

Hello,

There may be a regression in Structlog 25.2.0 as a result of #709. My unit tests found that it always displays time in local timezone, even if you set flag utc=True. For ISO format it works fine, but for custom format, it strips the UTC timezone and applies a local one.

Easiest way to reproduce is:

event_dict = {}
time_stamper_local = structlog.processors.TimeStamper(fmt="%Y-%m-%d %H:%M:%S %Z", utc=False)
print(time_stamper_local(None, None, event_dict))
time_stamper_utc = structlog.processors.TimeStamper(fmt="%Y-%m-%d %H:%M:%S %Z", utc=True)
print(time_stamper_utc(None, None, event_dict))

# prints
{'timestamp': '2025-03-14 13:33:03 CET'}
{'timestamp': '2025-03-14 13:33:03 CET'}



In structlog/src/structlog/processors.py
TimeStamper(fmt="%Y-%m-%d %H:%M:%S %Z", utc=True) effectively does:

datetime.datetime.now(tz=datetime.timezone.utc).astimezone()

Before applying astimezone() the object has UTC timezone, then it is replaced by local timezone. As per datetime documentation:

If called without arguments (or with tz=None) the system local time zone is assumed for the target time zone.




Not sure if this is intentional, but this behavior changed from Structlog 25.1.0. How about handling UTC separately, the same way it is done for ISO format?

diff --git a/structlog/.venv/lib/python3.10/site-packages/structlog/processors.py b/structlog/.venv/lib/python3.10/site-packages/structlog/processors.py
index 4ef7578..39e1532 100644
--- a/structlog/.venv/lib/python3.10/site-packages/structlog/processors.py
+++ b/structlog/.venv/lib/python3.10/site-packages/structlog/processors.py
@@ -553,12 +553,18 @@ def _make_stamper(
 
         return stamper_iso_local
 
-    def stamper_fmt(event_dict: EventDict) -> EventDict:
+    def stamper_fmt_local(event_dict: EventDict) -> EventDict:
         event_dict[key] = now().astimezone().strftime(fmt)
+        return event_dict
 
+    def stamper_fmt_utc(event_dict: EventDict) -> EventDict:
+        event_dict[key] = now().strftime(fmt)
         return event_dict
 
-    return stamper_fmt
+    if utc:
+        return stamper_fmt_utc
+
+    return stamper_fmt_local
 
 
 class MaybeTimeStamper:

Let me know if this makes sense or I misconfigured something,

Thanks,
Marcin Endraszka

@hynek
Copy link
Owner

hynek commented Mar 15, 2025

Unfortunately your bug report came ~24h too late and I’m now on vacation for three weeks. Skimming your report looks like you’re right, but I don’t think it warrants a yank on PyPI?

I would welcome a PR with an appropriate new test. The approach to have separate stamper()s makes sense.

@m-endra
Copy link
Author

m-endra commented Mar 18, 2025

Sure, why not. Testing this is not very easy, because freezegun does not handle timezones correctly. At least not in a way that could guarantee the same test results when the tests themselves are executed across many timezones.

Please enjoy your vacation and take a look when you're back. No need to hurry.
Pull request #713

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

No branches or pull requests

2 participants