Use RFC3339 in UTC as the default time format for log messages

[efd6]

This ensures that times that are logged in messages can be deserialised by the encoding/json package irrespective of the timezone that the time is in when logged; currently if the time is not in UTC, the ISO8601 format that is used is not accepted. Also provide a non-UTC formatter that can be used where the timezone is important.

[belimawr]

I'm just a bit confused why we need RFC3339UTCTimeEncoder and RFC3339TimeEncoder if only RFC3339UTCTimeEncoder is used directly and the only thing it does is to call RFC3339TimeEncoder with the time in UTC, which is the first thing RFC3339TimeEncoder does.

@efd6 could you clarify what is the effective difference between those two functions?

[efd6]

That is a bug.

[efd6]

Fixed. Thanks.

[kruskall]

LGTM!

Can you add a test ?

[efd6]

There are already tests for this.

[kruskall]

Sorry, I might be missing something

currently if the time is not in UTC, the ISO8601 format that is used is not accepted.

where is this tested ?

[efd6]

That's specified here.

[axw]

I think @kruskall is thinking of a test that the default timestamp encoder is able to accept non-UTC timestamps, for this library.

Put another way: can we add a test that would fail without the change in this PR?

[axw]

@efd6 how about this?

diff --git a/logger_test.go b/logger_test.go
index 5f08f74..c72335c 100644
--- a/logger_test.go
+++ b/logger_test.go
@@ -77,10 +77,13 @@ func (tw *testOutput) validate(t *testing.T, keys ...string) {
 			case "string":
 				_, ok = val.(string)
 			case "datetime":
-				var s string
-				s, ok = val.(string)
-				if _, err := time.Parse("2006-01-02T15:04:05.000Z0700", s); err == nil {
-					ok = true
+				if s, strOK := val.(string); strOK {
+					if _, err := time.Parse("2006-01-02T15:04:05.000Z0700", s); err == nil {
+						var t time.Time
+						if err := t.UnmarshalText([]byte(s)); err == nil {
+							ok = true
+						}
+					}
 				}
 			case "integer":
 				// json.Unmarshal unmarshals into float64 instead of int
@@ -160,3 +163,29 @@ func TestECSZapLogger(t *testing.T) {
 		})
 	}
 }
+
+func TestECSZapLoggerTimezone(t *testing.T) {
+	out := testOutput{}
+
+	eucla, err := time.LoadLocation("Australia/Eucla")
+	require.NoError(t, err)
+
+	core := NewCore(NewDefaultEncoderConfig(), &out, zap.DebugLevel)
+	logger := zap.New(core, zap.WithClock(&locationClock{
+		Clock:    zapcore.DefaultClock,
+		location: eucla,
+	}))
+	defer logger.Sync()
+
+	logger.Info("hello from UTC+8:45")
+	out.validate(t, "@timestamp")
+}
+
+type locationClock struct {
+	zapcore.Clock
+	location *time.Location
+}
+
+func (c *locationClock) Now() time.Time {
+	return c.Clock.Now().In(c.location)
+}

[efd6]

The time validation in validate is not right; it assumes the incorrect ISO8601 format. I've gone with a behaviour-based test that looks to whether json.Unmarshal cope with it only.

The config test is enough to show that the change is required.

[axw]

thanks for adding the test, looks good