From 7539c0804cafce7b7479b8c515f82ab876f62ee1 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Fri, 25 May 2018 12:25:33 +0900 Subject: [PATCH 1/2] Calculate t using time zone offsets --- rotatelogs.go | 5 +++-- rotatelogs_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/rotatelogs.go b/rotatelogs.go index e260a00..996c5c0 100644 --- a/rotatelogs.go +++ b/rotatelogs.go @@ -84,8 +84,9 @@ func New(p string, options ...Option) (*RotateLogs, error) { func (rl *RotateLogs) genFilename() string { now := rl.clock.Now() - diff := time.Duration(now.UnixNano()) % rl.rotationTime - t := now.Add(time.Duration(-1 * diff)) + _, offset := now.Zone() + base := now.Truncate(rl.rotationTime).Add(-1 * time.Duration(offset) * time.Second) + t := now.Add(base.Sub(now)) return rl.pattern.FormatString(t) } diff --git a/rotatelogs_test.go b/rotatelogs_test.go index 707e77d..d0bcfe3 100644 --- a/rotatelogs_test.go +++ b/rotatelogs_test.go @@ -12,6 +12,7 @@ import ( "github.com/jonboulle/clockwork" rotatelogs "github.com/lestrrat-go/file-rotatelogs" + "github.com/lestrrat-go/strftime" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -358,3 +359,46 @@ func TestRotationGenerationalNames(t *testing.T) { defer rl.Close() }) } + +func TestGHIssue23(t *testing.T) { + dir, err := ioutil.TempDir("", "file-rotatelogs-generational") + if !assert.NoError(t, err, `creating temporary directory should succeed`) { + return + } + defer os.RemoveAll(dir) + + t.Run("Set location to Asia/Tokyo", func(t *testing.T) { + loc, _ := time.LoadLocation("Asia/Tokyo") + rl, err := rotatelogs.New( + filepath.Join(dir, "asia_tokyo.%Y%m%d%H%M.log"), + rotatelogs.WithLocation(loc), + ) + if !assert.NoError(t, err, "rotatelogs.New should succeed") { + return + } + + // Timing sensitive... + + var now time.Time + for { + now = time.Now().In(loc) + if now.Hour() == 23 && now.Minute() >= 59 { + t.Logf("This test is sensitive to date changes. don't run this test after 23:59 Asia/Tokyo time") + time.Sleep(time.Minute) + continue + } + break + } + dt := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, loc) + expected, err := strftime.Format("asia_tokyo.%Y%m%d%H%M.log",dt) + if !assert.NoError(t, err, "strftime.Format should succeed") { + return + } + expected = filepath.Join(dir, expected) + + rl.Rotate() + if !assert.Equal(t, expected, rl.CurrentFileName(), "file names should match") { + return + } + }) +} From cb66a4779a9741b9a29b29d73f25fe84344a445f Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Fri, 1 Jun 2018 04:41:55 +0900 Subject: [PATCH 2/2] Hack around truncation, add more tests including negative offset timezones --- rotatelogs.go | 24 ++++++++++++--- rotatelogs_test.go | 76 ++++++++++++++++++++++++++-------------------- 2 files changed, 63 insertions(+), 37 deletions(-) diff --git a/rotatelogs.go b/rotatelogs.go index 996c5c0..e0bdd8d 100644 --- a/rotatelogs.go +++ b/rotatelogs.go @@ -84,10 +84,26 @@ func New(p string, options ...Option) (*RotateLogs, error) { func (rl *RotateLogs) genFilename() string { now := rl.clock.Now() - _, offset := now.Zone() - base := now.Truncate(rl.rotationTime).Add(-1 * time.Duration(offset) * time.Second) - t := now.Add(base.Sub(now)) - return rl.pattern.FormatString(t) + + // XXX HACK: Truncate only happens in UTC semantics, apparently. + // observed values for truncating given time with 86400 secs: + // + // before truncation: 2018/06/01 03:54:54 2018-06-01T03:18:00+09:00 + // after truncation: 2018/06/01 03:54:54 2018-05-31T09:00:00+09:00 + // + // This is really annoying when we want to truncate in local time + // so we hack: we take the apparent local time in the local zone, + // and pretend that it's in UTC. do our math, and put it back to + // the local zone + var base time.Time + if now.Location() != time.UTC { + base = time.Date(now.Year(), now.Month(), now.Day(), now.Hour(), now.Minute(), now.Second(), now.Nanosecond(), time.UTC) + base = base.Truncate(time.Duration(rl.rotationTime)) + base = time.Date(base.Year(), base.Month(), base.Day(), base.Hour(), base.Minute(), base.Second(), base.Nanosecond(), base.Location()) + } else { + base = now.Truncate(time.Duration(rl.rotationTime)) + } + return rl.pattern.FormatString(base) } // Write satisfies the io.Writer interface. It writes to the diff --git a/rotatelogs_test.go b/rotatelogs_test.go index d0bcfe3..1c57885 100644 --- a/rotatelogs_test.go +++ b/rotatelogs_test.go @@ -1,6 +1,7 @@ package rotatelogs_test import ( + "fmt" "io" "io/ioutil" "log" @@ -12,7 +13,6 @@ import ( "github.com/jonboulle/clockwork" rotatelogs "github.com/lestrrat-go/file-rotatelogs" - "github.com/lestrrat-go/strftime" "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -360,6 +360,12 @@ func TestRotationGenerationalNames(t *testing.T) { }) } +type ClockFunc func() time.Time + +func (f ClockFunc) Now() time.Time { + return f() +} + func TestGHIssue23(t *testing.T) { dir, err := ioutil.TempDir("", "file-rotatelogs-generational") if !assert.NoError(t, err, `creating temporary directory should succeed`) { @@ -367,38 +373,42 @@ func TestGHIssue23(t *testing.T) { } defer os.RemoveAll(dir) - t.Run("Set location to Asia/Tokyo", func(t *testing.T) { - loc, _ := time.LoadLocation("Asia/Tokyo") - rl, err := rotatelogs.New( - filepath.Join(dir, "asia_tokyo.%Y%m%d%H%M.log"), - rotatelogs.WithLocation(loc), - ) - if !assert.NoError(t, err, "rotatelogs.New should succeed") { - return - } - - // Timing sensitive... - - var now time.Time - for { - now = time.Now().In(loc) - if now.Hour() == 23 && now.Minute() >= 59 { - t.Logf("This test is sensitive to date changes. don't run this test after 23:59 Asia/Tokyo time") - time.Sleep(time.Minute) - continue - } - break - } - dt := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, loc) - expected, err := strftime.Format("asia_tokyo.%Y%m%d%H%M.log",dt) - if !assert.NoError(t, err, "strftime.Format should succeed") { - return + for _, locName := range []string{"Asia/Tokyo", "Pacific/Honolulu"} { + loc, _ := time.LoadLocation(locName) + tests := []struct { + Expected string + Clock rotatelogs.Clock + }{ + { + Expected: filepath.Join(dir, strings.ToLower(strings.Replace(locName, "/", "_", -1)) + ".201806010000.log"), + Clock: ClockFunc(func() time.Time { + return time.Date(2018, 6, 1, 3, 18, 0, 0, loc) + }), + }, + { + Expected: filepath.Join(dir, strings.ToLower(strings.Replace(locName, "/", "_", -1)) + ".201712310000.log"), + Clock: ClockFunc(func() time.Time { + return time.Date(2017, 12, 31, 23, 52, 0, 0, loc) + }), + }, } - expected = filepath.Join(dir, expected) - - rl.Rotate() - if !assert.Equal(t, expected, rl.CurrentFileName(), "file names should match") { - return + for _, test := range tests { + t.Run(fmt.Sprintf("location = %s, time = %s", locName, test.Clock.Now().Format(time.RFC3339)), func(t *testing.T) { + template := strings.ToLower(strings.Replace(locName, "/", "_", -1)) + ".%Y%m%d%H%M.log" + rl, err := rotatelogs.New( + filepath.Join(dir, template), + rotatelogs.WithClock(test.Clock), // we're not using WithLocation, but it's the same thing + ) + if !assert.NoError(t, err, "rotatelogs.New should succeed") { + return + } + + t.Logf("expected %s", test.Expected) + rl.Rotate() + if !assert.Equal(t, test.Expected, rl.CurrentFileName(), "file names should match") { + return + } + }) } - }) + } }