search

phuslu / log

Fastest structured logging
MIT License
672 stars 44 forks source link

Switch to allow header timestamps to be UTC #65

Closed fireflycons closed 6 months ago

fireflycons commented 6 months ago

Perhaps I'm missing something, but I could not see how to get the header timestamp to be always UTC instead of system local time.

This PR adds a boolean field to Logger so that UTC may be enabled, and the logic for this in header()

Additionally fix a typo in Logger comments and disable running of syslog tests on Windows.

phuslu commented 6 months ago

thanks! will go through it today!

fireflycons commented 6 months ago

Out of interest, what reference did you use to craft the ASM to get the go-id?

phuslu commented 6 months ago

I added comment to goid.s, using pure asm to impalement this function can minimalized the function call overhead in go, I think.

phuslu commented 6 months ago

After consideration I thin it's no problem for adding the "TimeUTC" field.

  1. Is there a better name for it? (If currently no, let us merge it first)
  2. Maybe the code change could be simplify/tidy. (maybe I'm wrong, but it will not be a blocker)
phuslu commented 6 months ago

Let me merge it first, thanks.

phuslu commented 6 months ago

Another option is change TimeUTC bool to TimeLocation *time.Location.

If user give a non-nil value(e.g. Time.UTC), we will call Time.In for the timestamp, and we also need implement a fast path for time.UTC.

phuslu commented 6 months ago

@fireflycons please take a look this approach https://github.com/phuslu/log/pull/66 if you have time, thanks.