Use ISO8601 extended format with time zone for log timestamps

[altendky]

https://en.wikipedia.org/wiki/ISO_8601

Existing: 2024-09-20T08:10:07.194 2.4.4.dev130 full_node full_node_server        : INFO
New     : 2024-09-20T08:10:07.194-04:00 2.4.4.dev130 full_node full_node_server        : INFO

Closes https://github.com/Chia-Network/chia-blockchain/issues/8802.

[altendky]

I went ahead and made the code, but I don't know how much we want to consider backwards compatibility for log-parsing tools with this sort of change. Also, this makes the code slightly more complicated, which isn't a big deal, but if we were willing to have microseconds instead of milliseconds I think it would actually make the new code a bit simpler than the original. Also I'll note that https://en.wikipedia.org/wiki/ISO_8601 seems to suggest that there... was a preference for a , fractional separator. Or maybe still is "within International Standards", whatever exactly that means. As coded, it still uses a ..

[arvidn]

I would think a better direction to go in would be to print the date every time it changes, and on other log lines only include the 24 hour wall clock time (without date). A large portion of the screen space is wasted on repeating the same date over and over again now, when looking at logs.

[arvidn]

the time zone doesn't change either, so it seems quite unnecessary to print it on every line

[emlowe]

Not printing it on every line does defeat the original purpose of the change though - which is to support streaming the logs to some log ingester like grafana (#8802) - which presumably needs each line to be completely self-contained.

It's fairly typical to either include the timezone on the line (Common Log Format) - or to always use a standard TZ like GMC/UTC.

There are probably other ways for the end-user to solve this though, so I'm not sure about making the change

[altendky]

Time zones can be changed. If referring to the offset, then for lots of places the laws change it a couple times a year. Not having this information is a pain when trying to do many things other than just glancing at your own log. For example, when I share logs with a pool and compare with other user's logs I have to remind them to check and share their system's offset.

[altendky]

@wjblanke, you tend to be sensibly concerned about backwards compatibility. Thoughts here? Would this need to be configurable?

[arvidn]

I've we're mostly concerned about this being machine-readable, using posix-time as timestamp would probably be best. No need to worry about time-zones or other head-aches that come with civic time, and it's short. Humans reading the logs will appreciate the screen-space.

The timestamp wouldn't be very human-friendly though.

[paulhainsworth-chia]

Are we ready to merge this so we can close out #8802

[altendky]

Kick CI for a fresh merge/build.

[github-actions[bot]]

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

[trepca]

any idea if this breaks tools that rely on parsing our logs, like chiadog?

[altendky]

I do not know.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[jinnatar]

I've been mucking deep in the chiadog codebase to introduce new health checks. What I've written now for the wallet works both with timezone aware and naive ISO8601 timestamps, of course with the caveat that with naive we need to assume the producing and consuming of logs are happening on the same TZ.

There's at least a handful of parsers that are still TZ naive:

• non_skipped_signage_points
• time_since_last_farm_event
• signage_point_stats

With that said, they're still parsing for an older log format and actually dropping the date, but not actually using the timestamp for anything as far as I can tell, ergo why the timestamp format change to ISO8601 in the first place hasn't tripped anything.

@martomi probably knows better than me how the event timestamp fields are used in practice, but from what I'm seeing adding the TZ offset shouldn't explode anything too horribly, and if it does at least it can be easily fixed to make the problem go away for good by parsing them as TZ aware datetimes.

[altendky]

Thanks for the feedback there. Maybe I'll try to revive this. :]