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>

diff --git a/src/log/slog/value.go b/src/log/slog/value.go
index d278d9b923559fb5aaa7015961d7b2f581b5a246..6b0768eb1dea1de0bfbb53806be73173e3f2653e 100644 (file)
--- 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 033f945407ef8ec18d162593ee58d01b900c4fe9..3e191589c5f6ae81fc640b308e60e2310ccfe032 100644 (file)
--- 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)
+               }
        }
 }