-
Notifications
You must be signed in to change notification settings - Fork 323
fix(promslog): always use UTC for time #735
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
Conversation
@SuperQ and I have both called this out in a few places; finally remembering to fix it. Prometheus has a UTC policy for logging because UTC is also used for metrics and so they should correlate. Before: ``` === RUN TestDynamicLevels/slog_log_style time=2024-12-02T13:51:08.463-05:00 level=INFO source=slog_test.go:120 msg=info hello=world time=2024-12-02T13:51:08.463-05:00 level=DEBUG source=slog_test.go:121 msg=debug hello=world ``` After: ``` === RUN TestDynamicLevels/slog_log_style time=2024-12-04T19:51:59.813Z level=INFO source=slog_test.go:120 msg=info hello=world time=2024-12-04T19:51:59.813Z level=DEBUG source=slog_test.go:121 msg=debug hello=world ``` Signed-off 8000 -by: TJ Hoplock <t.hoplock@gmail.com>
I don't think we should revert the behavior as it is done here. We had a bunch of feature requests to use the timezone configured in the environment for logging. I closed them as completed once the slog transition was done. Log format is exempt from the stability guarantees, so we are technically allowed to change it without a major version bump. But we should still only do it as an informed decision. At the very least, we should give users an option to use the default slog behavior again. But I would even argue that the current behavior (prior to this PR) is fine. Those people that have seen the light will configure UTC on their servers and get UTC logging. Those that set a local timezone on their server apparently want it that way, so why log in UTC? I also don't think that using UTC consistently for time series means we have to only use UTC timestamps for logs. The reasoning is quite different for each case. |
This reverts commit 145b50a. See discussion on Slack. We should first decide which log format we want before changing it from what is already released in prometheus/prometheus.
This reverts commit 145b50a. See discussion on Slack. We should first decide which log format we want before changing it from what is already released in prometheus/prometheus. Signed-off-by: beorn7 <beorn@grafana.com>
👍 seems ok to me. Sometimes when I'm just testing something locally it's actually nicer to have local timestamps, as it's easier for me to tell when something happened. But I don't mind much either way. |
As per the dev-summit discussion, we want to use the default timezone as selected by slog after all rather than a hardcoded UTC. (This was accidentally introduced earlier when moving from gokit to slog, but then switched back to hardcoded UTC in PR #735.) Environments that have configured UTC anyway won't see any difference, but those with other timezones configured will see the configured timezone for the timestamps in their logs. This is in line with how slog behaves by default. Example timestamp _before_ this change and also _after_ this change with UTC configured in the environment via a suitable way (e.g. setting `TZ=UTC` on Linux): time=2025-04-10T12:00:38.179Z Example timestamp _after_ this change with location Europe/Berlin, i.e. timezone is CET or CEST: time=2025-04-10T14:00:03.120+02:00 Note that the precise delta to UTC is in the timestamp. So this change will also be transparent to the usual logs processing tools. Signed-off-by: beorn7 <beorn@grafana.com>
@SuperQ and I have both called this out in a few places; finally remembering to fix it.
Prometheus has a UTC policy for logging because UTC is also used for metrics and so they should correlate.
Before:
After: