search

aws / amazon-ssm-agent

An agent to enable remote management of your EC2 instances, on-premises servers, or virtual machines (VMs).
https://aws.amazon.com/systems-manager/
Apache License 2.0
1.03k stars 323 forks source link

Change date formats in logging to use RFC3339 #434

Open dBotLFG opened 2 years ago

dBotLFG commented 2 years ago

Issue #, if available:

Description of changes: Change date formats in logging to use UTC

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

jkroepke commented 2 years ago

Hi @dBotLFG,

what the reason to do this? Is it not enough to set the system time to UTC?

dBotLFG commented 2 years ago

If the system time is set to local, the logs will use local time, but the timestamp will not reflect that it is local time (no offset indicator). By using a UTC timestamp by default, there will be no ambiguity in the time of the logged event. At a minimum, there should be a timezone offset used by default. As the code currently stands, it would be very difficult to correlate events between instances located in different regions with different timezones. Thank you for your consideration.

jkroepke commented 2 years ago

If the system time is set to local, the logs will use local time,

In a default configuration, I always expect this. Each system component is logging in localtime. Use UTC by default would confuse a lot of people here. For example current logs from journalctl:

Apr 07 15:19:58 i-0b1615f6a27caa92f amazon-ssm-agent[372718]: 2022-04-07 15:19:57 INFO [amazon-ssm-agent] [LongRunningWorkerContainer] Monitor long running worker health every 60 seconds

In case, logs are UTC

Apr 07 15:19:58 i-0b1615f6a27caa92f amazon-ssm-agent[372718]: 2022-04-07 13:19:57 INFO [amazon-ssm-agent] [LongRunningWorkerContainer] Monitor long running worker health every 60 seconds

or in winter time

Apr 07 15:19:58 i-0b1615f6a27caa92f amazon-ssm-agent[372718]: 2022-04-07 14:19:57 INFO [amazon-ssm-agent] [LongRunningWorkerContainer] Monitor long running worker health every 60 seconds

Having different times on a single line looks very odd.

I agree with you. UTC is removes a lot trouble, for example daylight saving time and helps if operations is done in multiple timezones. In conclusion, the system time should be also UTC. Consider to set CRON_TZ to local time, if cronjobs on the system should be run in a localtime.

but the timestamp will not reflect that it is local time (no offset indicator)

From my point of view, this is the real issue here. Using RFC3339 ("2006-01-02T15:04:05Z07:00") should solve this.

dBotLFG commented 2 years ago

Agreed, switched to RFC3339