From af7791206f0cea1e987e3165186b5afc2466d712 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Jan 2024 10:37:50 -0600 Subject: [PATCH 1/5] fix: strip timezone information from a date in dau response Timezone information is lost, so do not forward it to the client. --- coderd/insights_test.go | 12 ++++++++---- coderd/metricscache/metricscache.go | 4 +++- coderd/templates_test.go | 2 +- codersdk/deployment.go | 6 ++++-- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/coderd/insights_test.go b/coderd/insights_test.go index 07461fc26719e..1e8beaf19adc9 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -39,6 +39,9 @@ import ( func TestDeploymentInsights(t *testing.T) { t.Parallel() + clientTz, err := time.LoadLocation("America/Chicago") + require.NoError(t, err) + client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, AgentStatsRefreshInterval: time.Millisecond * 100, @@ -64,7 +67,7 @@ func TestDeploymentInsights(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - daus, err := client.DeploymentDAUs(context.Background(), codersdk.TimezoneOffsetHour(time.UTC)) + daus, err := client.DeploymentDAUs(context.Background(), codersdk.TimezoneOffsetHour(clientTz)) require.NoError(t, err) res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) @@ -84,22 +87,23 @@ func TestDeploymentInsights(t *testing.T) { _ = sshConn.Close() wantDAUs := &codersdk.DAUsResponse{ + TZHourOffset: codersdk.TimezoneOffsetHour(clientTz), Entries: []codersdk.DAUEntry{ { - Date: time.Now().UTC().Truncate(time.Hour * 24), + Date: time.Now().In(clientTz).Format("2006-01-02"), Amount: 1, }, }, } require.Eventuallyf(t, func() bool { - daus, err = client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(time.UTC)) + daus, err = client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(clientTz)) require.NoError(t, err) return len(daus.Entries) > 0 }, testutil.WaitShort, testutil.IntervalFast, "deployment daus never loaded", ) - gotDAUs, err := client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(time.UTC)) + gotDAUs, err := client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(clientTz)) require.NoError(t, err) require.Equal(t, gotDAUs, wantDAUs) diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 256564b671183..c485517ee97fc 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -166,7 +166,9 @@ func convertDAUResponse[T dauRow](rows []T, tzOffset int) codersdk.DAUsResponse var resp codersdk.DAUsResponse for _, date := range fillEmptyDays(dates) { resp.Entries = append(resp.Entries, codersdk.DAUEntry{ - Date: date, + // This date is truncated to 00:00:00 of the given day, so only + // return date information. + Date: date.Format("2006-01-02"), Amount: len(respMap[date]), }) } diff --git a/coderd/templates_test.go b/coderd/templates_test.go index fc3e8d07c3c97..269dae6138fbb 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -1405,7 +1405,7 @@ func TestTemplateMetrics(t *testing.T) { wantDAUs := &codersdk.DAUsResponse{ Entries: []codersdk.DAUEntry{ { - Date: time.Now().UTC().Truncate(time.Hour * 24), + Date: time.Now().UTC().Truncate(time.Hour * 24).Format("2006-01-02"), Amount: 1, }, }, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index c02eea595924f..d3d5a9ba504ba 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2168,8 +2168,10 @@ type DAUsResponse struct { } type DAUEntry struct { - Date time.Time `json:"date" format:"date-time"` - Amount int `json:"amount"` + // Date is a string formatted as 2024-01-31. + // Timezone and time information is not included. + Date string `json:"date"` + Amount int `json:"amount"` } type DAURequest struct { From bc3088fe04a0ca04f8976311cabe6e4c7ba6493e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Jan 2024 10:41:40 -0600 Subject: [PATCH 2/5] Make gen --- coderd/apidoc/docs.go | 4 ++-- coderd/apidoc/swagger.json | 4 ++-- docs/api/insights.md | 2 +- docs/api/schemas.md | 12 ++++++------ docs/api/templates.md | 2 +- site/src/testHelpers/entities.ts | 12 ++++++------ 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index e4bb0c628dfc9..7ee176f411626 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -8877,8 +8877,8 @@ const docTemplate = `{ "type": "integer" }, "date": { - "type": "string", - "format": "date-time" + "description": "Date is a string formatted as 2024-01-31.\nTimezone and time information is not included.", + "type": "string" } } }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 0e8460d4eda94..69f0479cb55c3 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7913,8 +7913,8 @@ "type": "integer" }, "date": { - "type": "string", - "format": "date-time" + "description": "Date is a string formatted as 2024-01-31.\nTimezone and time information is not included.", + "type": "string" } } }, diff --git a/docs/api/insights.md b/docs/api/insights.md index bfa1fcd380d5d..fd2ca79592b35 100644 --- a/docs/api/insights.md +++ b/docs/api/insights.md @@ -22,7 +22,7 @@ curl -X GET http://coder-server:8080/api/v2/insights/daus \ "entries": [ { "amount": 0, - "date": "2019-08-24T14:15:22Z" + "date": "string" } ], "tz_hour_offset": 0 diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 909158f2ebdc8..c20453ae6b28a 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1931,16 +1931,16 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ```json { "amount": 0, - "date": "2019-08-24T14:15:22Z" + "date": "string" } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| -------- | ------- | -------- | ------------ | ----------- | -| `amount` | integer | false | | | -| `date` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| -------- | ------- | -------- | ------------ | ---------------------------------------------------------------------------------------- | +| `amount` | integer | false | | | +| `date` | string | false | | Date is a string formatted as 2024-01-31. Timezone and time information is not included. | ## codersdk.DAUsResponse @@ -1949,7 +1949,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "entries": [ { "amount": 0, - "date": "2019-08-24T14:15:22Z" + "date": "string" } ], "tz_hour_offset": 0 diff --git a/docs/api/templates.md b/docs/api/templates.md index 63420f76a0d35..1ea355164af32 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -842,7 +842,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/daus \ "entries": [ { "amount": 0, - "date": "2019-08-24T14:15:22Z" + "date": "string" } ], "tz_hour_offset": 0 diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 11d2ba5a9c215..4436bcf690f03 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -21,17 +21,17 @@ export const MockOrganization: TypesGen.Organization = { export const MockTemplateDAUResponse: TypesGen.DAUsResponse = { tz_hour_offset: 0, entries: [ - { date: "2022-08-27T00:00:00Z", amount: 1 }, - { date: "2022-08-29T00:00:00Z", amount: 2 }, - { date: "2022-08-30T00:00:00Z", amount: 1 }, + { date: "2022-08-27", amount: 1 }, + { date: "2022-08-29", amount: 2 }, + { date: "2022-08-30", amount: 1 }, ], }; export const MockDeploymentDAUResponse: TypesGen.DAUsResponse = { tz_hour_offset: 0, entries: [ - { date: "2022-08-27T00:00:00Z", amount: 10 }, - { date: "2022-08-29T00:00:00Z", amount: 22 }, - { date: "2022-08-30T00:00:00Z", amount: 14 }, + { date: "2022-08-27", amount: 10 }, + { date: "2022-08-29", amount: 22 }, + { date: "2022-08-30", amount: 14 }, ], }; export const MockSessionToken: TypesGen.LoginWithPasswordResponse = { From 033fd4e4402383d7b60e5c8ca53d63a1d19c1555 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Jan 2024 13:53:28 -0600 Subject: [PATCH 3/5] fix unit tests --- coderd/metricscache/metricscache.go | 6 +++- coderd/metricscache/metricscache_test.go | 38 ++++++++++++------------ 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index c485517ee97fc..eee86fcb333f8 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -22,6 +22,10 @@ import ( "github.com/coder/retry" ) +func OnlyDate(t time.Time) string { + return t.Format("2006-01-02") +} + // deploymentTimezoneOffsets are the timezones that are cached and supported. // Any non-listed timezone offsets will need to use the closest supported one. var deploymentTimezoneOffsets = []int{ @@ -168,7 +172,7 @@ func convertDAUResponse[T dauRow](rows []T, tzOffset int) codersdk.DAUsResponse resp.Entries = append(resp.Entries, codersdk.DAUEntry{ // This date is truncated to 00:00:00 of the given day, so only // return date information. - Date: date.Format("2006-01-02"), + Date: OnlyDate(date), Amount: len(respMap[date]), }) } diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 30325e2e88c23..a362a6a86df6c 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -67,19 +67,19 @@ func TestCache_TemplateUsers(t *testing.T) { }, tplWant: want{[]codersdk.DAUEntry{ { - Date: date(2022, 8, 27), + Date: metricscache.OnlyDate(date(2022, 8, 27)), Amount: 1, }, { - Date: date(2022, 8, 28), + Date: metricscache.OnlyDate(date(2022, 8, 28)), Amount: 0, }, { - Date: date(2022, 8, 29), + Date: metricscache.OnlyDate(date(2022, 8, 29)), Amount: 0, }, { - Date: date(2022, 8, 30), + Date: metricscache.OnlyDate(date(2022, 8, 30)), Amount: 1, }, }, 1}, @@ -95,15 +95,15 @@ func TestCache_TemplateUsers(t *testing.T) { }, tplWant: want{[]codersdk.DAUEntry{ { - Date: date(2022, 8, 27), + Date: metricscache.OnlyDate(date(2022, 8, 27)), Amount: 1, }, { - Date: date(2022, 8, 28), + Date: metricscache.OnlyDate(date(2022, 8, 28)), Amount: 1, }, { - Date: date(2022, 8, 29), + Date: metricscache.OnlyDate(date(2022, 8, 29)), Amount: 1, }, }, 1}, @@ -121,31 +121,31 @@ func TestCache_TemplateUsers(t *testing.T) { }, tplWant: want{[]codersdk.DAUEntry{ { - Date: date(2022, 1, 1), + Date: metricscache.OnlyDate(date(2022, 1, 1)), Amount: 2, }, { - Date: date(2022, 1, 2), + Date: metricscache.OnlyDate(date(2022, 1, 2)), Amount: 0, }, { - Date: date(2022, 1, 3), + Date: metricscache.OnlyDate(date(2022, 1, 3)), Amount: 0, }, { - Date: date(2022, 1, 4), + Date: metricscache.OnlyDate(date(2022, 1, 4)), Amount: 1, }, { - Date: date(2022, 1, 5), + Date: metricscache.OnlyDate(date(2022, 1, 5)), Amount: 0, }, { - Date: date(2022, 1, 6), + Date: metricscache.OnlyDate(date(2022, 1, 6)), Amount: 0, }, { - Date: date(2022, 1, 7), + Date: metricscache.OnlyDate(date(2022, 1, 7)), Amount: 2, }, }, 2}, @@ -164,17 +164,17 @@ func TestCache_TemplateUsers(t *testing.T) { }, tplWant: want{[]codersdk.DAUEntry{ { - Date: date(2022, 1, 2), + Date: metricscache.OnlyDate(date(2022, 1, 2)), Amount: 2, }, }, 2}, dauWant: []codersdk.DAUEntry{ { - Date: date(2022, 1, 1), + Date: metricscache.OnlyDate(date(2022, 1, 1)), Amount: 2, }, { - Date: date(2022, 1, 2), + Date: metricscache.OnlyDate(date(2022, 1, 2)), Amount: 2, }, }, @@ -192,13 +192,13 @@ func TestCache_TemplateUsers(t *testing.T) { }, dauWant: []codersdk.DAUEntry{ { - Date: date(2022, 1, 1), + Date: metricscache.OnlyDate(date(2022, 1, 1)), Amount: 2, }, }, tplWant: want{[]codersdk.DAUEntry{ { - Date: date(2022, 1, 2), + Date: metricscache.OnlyDate(date(2022, 1, 2)), Amount: 2, }, }, 2}, From dab4394e1d14e64b0e0364628a1f49091ebccab0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Jan 2024 15:06:01 -0600 Subject: [PATCH 4/5] fix: timezone offset should be flipped --- codersdk/deployment.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index d3d5a9ba504ba..c94dd427d98b8 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2186,14 +2186,18 @@ func (d DAURequest) asRequestOption() RequestOption { } } +// TimezoneOffsetHour is implemented to match the javascript 'getTimezoneOffset()' function. +// This is the amount of time between this date evaluated in UTC and evaluated in the 'loc' +// The trivial case of times being on the same day is: +// 'time.Now().UTC().Hour() - time.Now().In(loc).Hour()' func TimezoneOffsetHour(loc *time.Location) int { if loc == nil { // Default to UTC time to be consistent across all callers. loc = time.UTC } _, offsetSec := time.Now().In(loc).Zone() - // Convert to hours - return offsetSec / 60 / 60 + // Convert to hours and flip the sign + return -1 * offsetSec / 60 / 60 } func (c *Client) DeploymentDAUsLocalTZ(ctx context.Context) (*DAUsResponse, error) { From 62a22e7286f89b61a156e8f3a3522d3096f87041 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 31 Jan 2024 15:17:14 -0600 Subject: [PATCH 5/5] Make tests deterministic --- codersdk/deployment.go | 10 +++++-- codersdk/deployment_test.go | 60 ++++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index c94dd427d98b8..c4744e544886e 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2186,20 +2186,24 @@ func (d DAURequest) asRequestOption() RequestOption { } } -// TimezoneOffsetHour is implemented to match the javascript 'getTimezoneOffset()' function. +// TimezoneOffsetHourWithTime is implemented to match the javascript 'getTimezoneOffset()' function. // This is the amount of time between this date evaluated in UTC and evaluated in the 'loc' // The trivial case of times being on the same day is: // 'time.Now().UTC().Hour() - time.Now().In(loc).Hour()' -func TimezoneOffsetHour(loc *time.Location) int { +func TimezoneOffsetHourWithTime(now time.Time, loc *time.Location) int { if loc == nil { // Default to UTC time to be consistent across all callers. loc = time.UTC } - _, offsetSec := time.Now().In(loc).Zone() + _, offsetSec := now.In(loc).Zone() // Convert to hours and flip the sign return -1 * offsetSec / 60 / 60 } +func TimezoneOffsetHour(loc *time.Location) int { + return TimezoneOffsetHourWithTime(time.Now(), loc) +} + func (c *Client) DeploymentDAUsLocalTZ(ctx context.Context) (*DAUsResponse, error) { return c.DeploymentDAUs(ctx, TimezoneOffsetHour(time.Local)) } diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 97cd2ce82bfce..250be46461b66 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -205,6 +205,7 @@ func TestTimezoneOffsets(t *testing.T) { testCases := []struct { Name string + Now time.Time Loc *time.Location ExpectedOffset int }{ @@ -213,29 +214,52 @@ func TestTimezoneOffsets(t *testing.T) { Loc: time.UTC, ExpectedOffset: 0, }, - // The following test cases are broken re: daylight savings - //{ - // Name: "Eastern", - // Loc: must(time.LoadLocation("America/New_York")), - // ExpectedOffset: -4, - // }, - //{ - // Name: "Central", - // Loc: must(time.LoadLocation("America/Chicago")), - // ExpectedOffset: -5, - // }, - //{ - // Name: "Ireland", - // Loc: must(time.LoadLocation("Europe/Dublin")), - // ExpectedOffset: 1, - // }, + + { + Name: "Eastern", + Now: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), + Loc: must(time.LoadLocation("America/New_York")), + ExpectedOffset: 5, + }, + { + // Daylight savings is on the 14th of March to Nov 7 in 2021 + Name: "EasternDaylightSavings", + Now: time.Date(2021, 3, 16, 0, 0, 0, 0, time.UTC), + Loc: must(time.LoadLocation("America/New_York")), + ExpectedOffset: 4, + }, + { + Name: "Central", + Now: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), + Loc: must(time.LoadLocation("America/Chicago")), + ExpectedOffset: 6, + }, + { + Name: "CentralDaylightSavings", + Now: time.Date(2021, 3, 16, 0, 0, 0, 0, time.UTC), + Loc: must(time.LoadLocation("America/Chicago")), + ExpectedOffset: 5, + }, + { + Name: "Ireland", + Now: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC), + Loc: must(time.LoadLocation("Europe/Dublin")), + ExpectedOffset: 0, + }, + { + Name: "IrelandDaylightSavings", + Now: time.Date(2021, 4, 3, 0, 0, 0, 0, time.UTC), + Loc: must(time.LoadLocation("Europe/Dublin")), + ExpectedOffset: -1, + }, { Name: "HalfHourTz", + Now: time.Date(2024, 1, 20, 6, 0, 0, 0, must(time.LoadLocation("Asia/Yangon"))), // This timezone is +6:30, but the function rounds to the nearest hour. // This is intentional because our DAUs endpoint only covers 1-hour offsets. // If the user is in a non-hour timezone, they get the closest hour bucket. Loc: must(time.LoadLocation("Asia/Yangon")), - ExpectedOffset: 6, + ExpectedOffset: -6, }, } @@ -244,7 +268,7 @@ func TestTimezoneOffsets(t *testing.T) { t.Run(c.Name, func(t *testing.T) { t.Parallel() - offset := codersdk.TimezoneOffsetHour(c.Loc) + offset := codersdk.TimezoneOffsetHourWithTime(c.Now, c.Loc) require.Equal(t, c.ExpectedOffset, offset) }) }