diff options
author | Jonathan Amsterdam <jba@google.com> | 2024-05-16 08:14:52 -0400 |
---|---|---|
committer | Jonathan Amsterdam <jba@google.com> | 2024-05-16 16:12:08 +0000 |
commit | db7bb2742ce01601842e277e7808d225ff8390cd (patch) | |
tree | fe7f86b8743f3f91ed71082fa5fb840803f4fd85 /src/log | |
parent | 505000b2d38c4c93aa7d30e82c61b76a6925b4e9 (diff) | |
download | go-db7bb2742ce01601842e277e7808d225ff8390cd.tar.gz go-db7bb2742ce01601842e277e7808d225ff8390cd.zip |
log/slog: handle times with undefined UnixNanos
slog tries to represent a time.Time without allocations, which involves
storing its UnixNanos value. But UnixNanos is undefined for some valid
times. Provide a fallback representation for those times by storing them
in the `any` field of `Value`.
Fixes #65902.
Change-Id: I736c739a92f77d7b1122ea0831524acdd2c4703f
Reviewed-on: https://go-review.googlesource.com/c/go/+/585519
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Diffstat (limited to 'src/log')
-rw-r--r-- | src/log/slog/value.go | 38 | ||||
-rw-r--r-- | src/log/slog/value_test.go | 22 |
2 files changed, 46 insertions, 14 deletions
diff --git a/src/log/slog/value.go b/src/log/slog/value.go index d278d9b923..6b0768eb1d 100644 --- a/src/log/slog/value.go +++ b/src/log/slog/value.go @@ -90,7 +90,7 @@ func (v Value) Kind() Kind { return x case stringptr: return KindString - case timeLocation: + case timeLocation, timeTime: return KindTime case groupptr: return KindGroup @@ -139,9 +139,14 @@ func BoolValue(v bool) Value { return Value{num: u, any: KindBool} } -// Unexported version of *time.Location, just so we can store *time.Locations in -// Values. (No user-provided value has this type.) -type timeLocation *time.Location +type ( + // Unexported version of *time.Location, just so we can store *time.Locations in + // Values. (No user-provided value has this type.) + timeLocation *time.Location + + // timeTime is for times where UnixNano is undefined. + timeTime time.Time +) // TimeValue returns a [Value] for a [time.Time]. // It discards the monotonic portion. @@ -153,7 +158,15 @@ func TimeValue(v time.Time) Value { // mistaken for any other Value, time.Time or otherwise. return Value{any: timeLocation(nil)} } - return Value{num: uint64(v.UnixNano()), any: timeLocation(v.Location())} + nsec := v.UnixNano() + t := time.Unix(0, nsec) + if v.Equal(t) { + // UnixNano correctly represents the time, so use a zero-alloc representation. + return Value{num: uint64(nsec), any: timeLocation(v.Location())} + } + // Fall back to the general form. + // Strip the monotonic portion to match the other representation. + return Value{any: timeTime(v.Round(0))} } // DurationValue returns a [Value] for a [time.Duration]. @@ -368,12 +381,19 @@ func (v Value) Time() time.Time { return v.time() } +// See TimeValue to understand how times are represented. func (v Value) time() time.Time { - loc := v.any.(timeLocation) - if loc == nil { - return time.Time{} + switch a := v.any.(type) { + case timeLocation: + if a == nil { + return time.Time{} + } + return time.Unix(0, int64(v.num)).In(a) + case timeTime: + return time.Time(a) + default: + panic(fmt.Sprintf("bad time type %T", v.any)) } - return time.Unix(0, int64(v.num)).In(loc) } // LogValuer returns v's value as a LogValuer. It panics diff --git a/src/log/slog/value_test.go b/src/log/slog/value_test.go index 033f945407..3e191589c5 100644 --- a/src/log/slog/value_test.go +++ b/src/log/slog/value_test.go @@ -30,7 +30,10 @@ func TestValueEqual(t *testing.T) { BoolValue(true), BoolValue(false), TimeValue(testTime), + TimeValue(time.Time{}), TimeValue(time.Date(2001, 1, 2, 3, 4, 5, 0, time.UTC)), + TimeValue(time.Date(2300, 1, 1, 0, 0, 0, 0, time.UTC)), // overflows nanoseconds + TimeValue(time.Date(1715, 6, 13, 0, 25, 26, 290448384, time.UTC)), // overflowed value AnyValue(&x), AnyValue(&y), GroupValue(Bool("b", true), Int("i", 3)), @@ -229,11 +232,20 @@ func TestLogValue(t *testing.T) { } } -func TestZeroTime(t *testing.T) { - z := time.Time{} - got := TimeValue(z).Time() - if !got.IsZero() { - t.Errorf("got %s (%#[1]v), not zero time (%#v)", got, z) +func TestValueTime(t *testing.T) { + // Validate that all representations of times work correctly. + for _, tm := range []time.Time{ + time.Time{}, + time.Unix(0, 1e15), // UnixNanos is defined + time.Date(2300, 1, 1, 0, 0, 0, 0, time.UTC), // overflows UnixNanos + } { + got := TimeValue(tm).Time() + if !got.Equal(tm) { + t.Errorf("got %s (%#[1]v), want %s (%#[2]v)", got, tm) + } + if g, w := got.Location(), tm.Location(); g != w { + t.Errorf("%s: location: got %v, want %v", tm, g, w) + } } } |