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

The time value is wrong when outputting the log to console. #483

Closed
bagualing opened this issue Sep 23, 2022 · 5 comments
Closed

The time value is wrong when outputting the log to console. #483

bagualing opened this issue Sep 23, 2022 · 5 comments

Comments

@bagualing
Copy link

bagualing commented Sep 23, 2022

Code

package main

import (
	"fmt"
	"os"
	"time"

	"github.com/rs/zerolog"
	"github.com/rs/zerolog/log"
)

func main() {
	timeFormat := "2006-01-02 15:04:05"

	t := time.Now()
	fmt.Print("local zone: ")
	fmt.Println(t.Local().Zone())
	fmt.Println("def:", t.Format(timeFormat))
	fmt.Println("loc:", t.Local().Format(timeFormat))
	fmt.Println("utc:", t.UTC().Format(timeFormat))

	zerolog.TimeFieldFormat = timeFormat
	log.Print("hello world")

	log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: timeFormat})
	log.Info().Msg("Wrong time")
}

Output:

local zone: CST 28800
def: 2022-09-23 21:19:29
loc: 2022-09-23 21:19:29
utc: 2022-09-23 13:19:29
{"level":"debug","time":"2022-09-23 21:19:29","message":"hello world"}
2022-09-24 05:19:29 INF Wrong time

The local time is 2022-09-23 21:19:29 and the UTC time is 2022-09-23 13:19:29, but the console output time is 2022-09-24 05:19:29 .
This issue is caused by #415 .

@pscheid92
Copy link
Contributor

pscheid92 commented Oct 6, 2022

Ok, I think I understood the code base and how the behaviour above occurs. And I think it is an unfortunate case of "works as designed by Go". I have no idea if there is a need for a change; or how to fix this holistically.

Inner workings of ConsoleWriter

A ConsoleWriter takes the structured log output of zerolog as an argument and uses a json.Decoder to recreate a key-value map from it.

zerolog/console.go

Lines 118 to 125 in 55aaf04

var evt map[string]interface{}
p = decodeIfBinaryToBytes(p)
d := json.NewDecoder(bytes.NewReader(p))
d.UseNumber()
err = d.Decode(&evt)
if err != nil {
return n, fmt.Errorf("cannot decode event: %s", err)
}

It extracts the time field as a string in the format specified by zerolog.TimeFieldFormat. For instance, this byte array is copied from a debug session (using the default time format).

{"level":"info","time":"2022-10-06T20:04:38+02:00","message":"wrong time default"}

Later, zerolog uses the closure as returned by consoleDefaultFormatTimestamp() to parse & format the extracted time.

zerolog/console.go

Lines 339 to 345 in 55aaf04

case string:
ts, err := time.Parse(TimeFieldFormat, tt)
if err != nil {
t = tt
} else {
t = ts.Local().Format(timeFormat)
}

It uses zerolog.TimeFieldFormat to parse the extracted time from string to time.Time. That is rightfully so because that is the exact time layout used to format the time in the first place.

Then it uses the time format layout given to the ConsoleWriter on creation by TimeFormat:

zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: "<here time format for console writer output>"}

So we take the parsed time.Time, convert it into our local time zone and format it as specified by TimeFormat:

ts.Local().Format(timeFormat) 

The result is the time string, as we see it printed later.


Reason for the observed behaviour

The time.Parse() method handles time layouts without a time zone indicator by assuming UTC.

Source: Go documentation for time package

In the absence of a time zone indicator, time.Parse() returns a time in UTC.


Remember above that we use zerolog.TimeFieldFormat to format and then parse the time again?
In your example, the format is "2006-01-02 15:04:05" ... no time zone indicator.

Let's say ConsoleWriter sees the following log:

{"level":"info","time":"2022-10-06 20:54:52","message":"wrong time default"}

Parsing it again leads to "2022-10-06 20:54:52 UTC" (due to the missing time zone indicator).
The formatting with ts.Local().Format(timeFormat) then converts from UTC to your local time zone again.
Adding or subtracting the hours accordingly. Leading to the wrong output.


Solution

Add a time zone indicator to your layout so Go doesn't end up assuming UTC.

timeFormat := "2006-01-02 15:04:05 MST"

Maybe a diagram is more accessible than my kind of unstructured thoughts.

This happens when we use a time format without a time zone indicator. Go assumes UTC, and we falsely convert from UTC to our time zone again.

image

On the other hand: If we use a time format with a time zone indicator, Go can restore the time zone, doesn't convert, and the result is correct.

image

@bagualing
Copy link
Author

@pscheid92 Thanks for your detailed answer, it's very clear.

I have a question: Is it possible to replace time.Parse() with time.ParseInLocation() to solve this problem?

zerolog/console.go

		case string:
			//ts, err := time.Parse(TimeFieldFormat, tt)
			ts, err := time.ParseInLocation(TimeFieldFormat, tt, time.Local)
			if err != nil {
				t = tt
			} else {
				t = ts.Local().Format(timeFormat)
			}

@pscheid92
Copy link
Contributor

🤔 Could be a solution I think.

Go Documentation

ParseInLocation is like Parse but differs in two important ways. First, in the absence of time zone information, Parse interprets a time as UTC; ParseInLocation interprets the time as in the given location.

But I still try to wrap my head around the second remark:

Second, when given a zone offset or abbreviation, Parse tries to match it against the Local location; ParseInLocation uses the given location.

WDYT @rs?

@pscheid92
Copy link
Contributor

Playing around with it locally looked good to me 🤔

@rs
Copy link
Owner

rs commented Oct 19, 2022

I'm fine with a local tz default on console writer.

rs pushed a commit that referenced this issue Oct 21, 2022
)

Closes issue #483

Before:
We use time.Parse which defaults to TZ UTC if there is no time zone
indicator specified in the time layout string. During the reparsing in
ConsoleWriter we therefore added the TZ difference to UTC twice.

After:
We use time.ParseInLocal where we need to provide a dedicated fallback
TZ as a fallback. Since we now fallback to the local TZ, we don't add
the TZ difference to UTC twice.
pablitoc pushed a commit to nxcr-org/zerolog that referenced this issue Apr 7, 2023
…s#497)

Closes issue rs#483

Before:
We use time.Parse which defaults to TZ UTC if there is no time zone
indicator specified in the time layout string. During the reparsing in
ConsoleWriter we therefore added the TZ difference to UTC twice.

After:
We use time.ParseInLocal where we need to provide a dedicated fallback
TZ as a fallback. Since we now fallback to the local TZ, we don't add
the TZ difference to UTC twice.
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

3 participants