Skip to content

Commit ac64155

Browse files
EmyrkEmyrk
authored
fix: strip timezone information from a date in dau response (#11962)
* fix: strip timezone information from a date in dau response Timezone information is lost, so do not forward it to the client. * fix: timezone offset should be flipped * Make tests deterministic
1 parent 76e7328 commit ac64155

File tree

12 files changed

+111
-67
lines changed

12 files changed

+111
-67
lines changed

‎coderd/apidoc/docs.go

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/insights_test.go

+8-4
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ import (
3939
func TestDeploymentInsights(t *testing.T) {
4040
t.Parallel()
4141

42+
clientTz, err := time.LoadLocation("America/Chicago")
43+
require.NoError(t, err)
44+
4245
client := coderdtest.New(t, &coderdtest.Options{
4346
IncludeProvisionerDaemon: true,
4447
AgentStatsRefreshInterval: time.Millisecond * 100,
@@ -64,7 +67,7 @@ func TestDeploymentInsights(t *testing.T) {
6467
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
6568
defer cancel()
6669

67-
daus, err := client.DeploymentDAUs(context.Background(), codersdk.TimezoneOffsetHour(time.UTC))
70+
daus, err := client.DeploymentDAUs(context.Background(), codersdk.TimezoneOffsetHour(clientTz))
6871
require.NoError(t, err)
6972

7073
res, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{})
@@ -84,22 +87,23 @@ func TestDeploymentInsights(t *testing.T) {
8487
_ = sshConn.Close()
8588

8689
wantDAUs := &codersdk.DAUsResponse{
90+
TZHourOffset: codersdk.TimezoneOffsetHour(clientTz),
8791
Entries: []codersdk.DAUEntry{
8892
{
89-
Date: time.Now().UTC().Truncate(time.Hour * 24),
93+
Date: time.Now().In(clientTz).Format("2006-01-02"),
9094
Amount: 1,
9195
},
9296
},
9397
}
9498
require.Eventuallyf(t, func() bool {
95-
daus, err = client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(time.UTC))
99+
daus, err = client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(clientTz))
96100
require.NoError(t, err)
97101
return len(daus.Entries) > 0
98102
},
99103
testutil.WaitShort, testutil.IntervalFast,
100104
"deployment daus never loaded",
101105
)
102-
gotDAUs, err := client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(time.UTC))
106+
gotDAUs, err := client.DeploymentDAUs(ctx, codersdk.TimezoneOffsetHour(clientTz))
103107
require.NoError(t, err)
104108
require.Equal(t, gotDAUs, wantDAUs)
105109

‎coderd/metricscache/metricscache.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ import (
2222
"github.com/coder/retry"
2323
)
2424

25+
func OnlyDate(t time.Time) string {
26+
return t.Format("2006-01-02")
27+
}
28+
2529
// deploymentTimezoneOffsets are the timezones that are cached and supported.
2630
// Any non-listed timezone offsets will need to use the closest supported one.
2731
var deploymentTimezoneOffsets = []int{
@@ -166,7 +170,9 @@ func convertDAUResponse[T dauRow](rows []T, tzOffset int) codersdk.DAUsResponse
166170
var resp codersdk.DAUsResponse
167171
for _, date := range fillEmptyDays(dates) {
168172
resp.Entries = append(resp.Entries, codersdk.DAUEntry{
169-
Date: date,
173+
// This date is truncated to 00:00:00 of the given day, so only
174+
// return date information.
175+
Date: OnlyDate(date),
170176
Amount: len(respMap[date]),
171177
})
172178
}

‎coderd/metricscache/metricscache_test.go

+19-19
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,19 @@ func TestCache_TemplateUsers(t *testing.T) {
6767
},
6868
tplWant: want{[]codersdk.DAUEntry{
6969
{
70-
Date: date(2022, 8, 27),
70+
Date: metricscache.OnlyDate(date(2022, 8, 27)),
7171
Amount: 1,
7272
},
7373
{
74-
Date: date(2022, 8, 28),
74+
Date: metricscache.OnlyDate(date(2022, 8, 28)),
7575
Amount: 0,
7676
},
7777
{
78-
Date: date(2022, 8, 29),
78+
Date: metricscache.OnlyDate(date(2022, 8, 29)),
7979
Amount: 0,
8080
},
8181
{
82-
Date: date(2022, 8, 30),
82+
Date: metricscache.OnlyDate(date(2022, 8, 30)),
8383
Amount: 1,
8484
},
8585
}, 1},
@@ -95,15 +95,15 @@ func TestCache_TemplateUsers(t *testing.T) {
9595
},
9696
tplWant: want{[]codersdk.DAUEntry{
9797
{
98-
Date: date(2022, 8, 27),
98+
Date: metricscache.OnlyDate(date(2022, 8, 27)),
9999
Amount: 1,
100100
},
101101
{
102-
Date: date(2022, 8, 28),
102+
Date: metricscache.OnlyDate(date(2022, 8, 28)),
103103
Amount: 1,
104104
},
105105
{
106-
Date: date(2022, 8, 29),
106+
Date: metricscache.OnlyDate(date(2022, 8, 29)),
107107
Amount: 1,
108108
},
109109
}, 1},
@@ -121,31 +121,31 @@ func TestCache_TemplateUsers(t *testing.T) {
121121
},
122122
tplWant: want{[]codersdk.DAUEntry{
123123
{
124-
Date: date(2022, 1, 1),
124+
Date: metricscache.OnlyDate(date(2022, 1, 1)),
125125
Amount: 2,
126126
},
127127
{
128-
Date: date(2022, 1, 2),
128+
Date: metricscache.OnlyDate(date(2022, 1, 2)),
129129
Amount: 0,
130130
},
131131
{
132-
Date: date(2022, 1, 3),
132+
Date: metricscache.OnlyDate(date(2022, 1, 3)),
133133
Amount: 0,
134134
},
135135
{
136-
Date: date(2022, 1, 4),
136+
Date: metricscache.OnlyDate(date(2022, 1, 4)),
137137
Amount: 1,
138138
},
139139
{
140-
Date: date(2022, 1, 5),
140+
Date: metricscache.OnlyDate(date(2022, 1, 5)),
141141
Amount: 0,
142142
},
143143
{
144-
Date: date(2022, 1, 6),
144+
Date: metricscache.OnlyDate(date(2022, 1, 6)),
145145
Amount: 0,
146146
},
147147
{
148-
Date: date(2022, 1, 7),
148+
Date: metricscache.OnlyDate(date(2022, 1, 7)),
149149
Amount: 2,
150150
},
151151
}, 2},
@@ -164,17 +164,17 @@ func TestCache_TemplateUsers(t *testing.T) {
164164
},
165165
tplWant: want{[]codersdk.DAUEntry{
166166
{
167-
Date: date(2022, 1, 2),
167+
Date: metricscache.OnlyDate(date(2022, 1, 2)),
168168
Amount: 2,
169169
},
170170
}, 2},
171171
dauWant: []codersdk.DAUEntry{
172172
{
173-
Date: date(2022, 1, 1),
173+
Date: metricscache.OnlyDate(date(2022, 1, 1)),
174174
Amount: 2,
175175
},
176176
{
177-
Date: date(2022, 1, 2),
177+
Date: metricscache.OnlyDate(date(2022, 1, 2)),
178178
Amount: 2,
179179
},
180180
},
@@ -192,13 +192,13 @@ func TestCache_TemplateUsers(t *testing.T) {
192192
},
193193
dauWant: []codersdk.DAUEntry{
194194
{
195-
Date: date(2022, 1, 1),
195+
Date: metricscache.OnlyDate(date(2022, 1, 1)),
196196
Amount: 2,
197197
},
198198
},
199199
tplWant: want{[]codersdk.DAUEntry{
200200
{
201-
Date: date(2022, 1, 2),
201+
Date: metricscache.OnlyDate(date(2022, 1, 2)),
202202
Amount: 2,
203203
},
204204
}, 2},

‎coderd/templates_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1405,7 +1405,7 @@ func TestTemplateMetrics(t *testing.T) {
14051405
wantDAUs := &codersdk.DAUsResponse{
14061406
Entries: []codersdk.DAUEntry{
14071407
{
1408-
Date: time.Now().UTC().Truncate(time.Hour * 24),
1408+
Date: time.Now().UTC().Truncate(time.Hour * 24).Format("2006-01-02"),
14091409
Amount: 1,
14101410
},
14111411
},

‎codersdk/deployment.go

+16-6
Original file line numberDiff line numberDiff line change
@@ -2168,8 +2168,10 @@ type DAUsResponse struct {
21682168
}
21692169

21702170
type DAUEntry struct {
2171-
Date time.Time `json:"date" format:"date-time"`
2172-
Amount int `json:"amount"`
2171+
// Date is a string formatted as 2024-01-31.
2172+
// Timezone and time information is not included.
2173+
Date string `json:"date"`
2174+
Amount int `json:"amount"`
21732175
}
21742176

21752177
type DAURequest struct {
@@ -2184,14 +2186,22 @@ func (d DAURequest) asRequestOption() RequestOption {
21842186
}
21852187
}
21862188

2187-
func TimezoneOffsetHour(loc *time.Location) int {
2189+
// TimezoneOffsetHourWithTime is implemented to match the javascript 'getTimezoneOffset()' function.
2190+
// This is the amount of time between this date evaluated in UTC and evaluated in the 'loc'
2191+
// The trivial case of times being on the same day is:
2192+
// 'time.Now().UTC().Hour() - time.Now().In(loc).Hour()'
2193+
func TimezoneOffsetHourWithTime(now time.Time, loc *time.Location) int {
21882194
if loc == nil {
21892195
// Default to UTC time to be consistent across all callers.
21902196
loc = time.UTC
21912197
}
2192-
_, offsetSec := time.Now().In(loc).Zone()
2193-
// Convert to hours
2194-
return offsetSec / 60 / 60
2198+
_, offsetSec := now.In(loc).Zone()
2199+
// Convert to hours and flip the sign
2200+
return -1 * offsetSec / 60 / 60
2201+
}
2202+
2203+
func TimezoneOffsetHour(loc *time.Location) int {
2204+
return TimezoneOffsetHourWithTime(time.Now(), loc)
21952205
}
21962206

21972207
func (c *Client) DeploymentDAUsLocalTZ(ctx context.Context) (*DAUsResponse, error) {

‎codersdk/deployment_test.go

+42-18
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ func TestTimezoneOffsets(t *testing.T) {
205205

206206
testCases := []struct {
207207
Name string
208+
Now time.Time
208209
Loc *time.Location
209210
ExpectedOffset int
210211
}{
@@ -213,29 +214,52 @@ func TestTimezoneOffsets(t *testing.T) {
213214
Loc: time.UTC,
214215
ExpectedOffset: 0,
215216
},
216-
// The following test cases are broken re: daylight savings
217-
//{
218-
// Name: "Eastern",
219-
// Loc: must(time.LoadLocation("America/New_York")),
220-
// ExpectedOffset: -4,
221-
// },
222-
//{
223-
// Name: "Central",
224-
// Loc: must(time.LoadLocation("America/Chicago")),
225-
// ExpectedOffset: -5,
226-
// },
227-
//{
228-
// Name: "Ireland",
229-
// Loc: must(time.LoadLocation("Europe/Dublin")),
230-
// ExpectedOffset: 1,
231-
// },
217+
218+
{
219+
Name: "Eastern",
220+
Now: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC),
221+
Loc: must(time.LoadLocation("America/New_York")),
222+
ExpectedOffset: 5,
223+
},
224+
{
225+
// Daylight savings is on the 14th of March to Nov 7 in 2021
226+
Name: "EasternDaylightSavings",
227+
Now: time.Date(2021, 3, 16, 0, 0, 0, 0, time.UTC),
228+
Loc: must(time.LoadLocation("America/New_York")),
229+
ExpectedOffset: 4,
230+
},
231+
{
232+
Name: "Central",
233+
Now: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC),
234+
Loc: must(time.LoadLocation("America/Chicago")),
235+
ExpectedOffset: 6,
236+
},
237+
{
238+
Name: "CentralDaylightSavings",
239+
Now: time.Date(2021, 3, 16, 0, 0, 0, 0, time.UTC),
240+
Loc: must(time.LoadLocation("America/Chicago")),
241+
ExpectedOffset: 5,
242+
},
243+
{
244+
Name: "Ireland",
245+
Now: time.Date(2021, 2, 1, 0, 0, 0, 0, time.UTC),
246+
Loc: must(time.LoadLocation("Europe/Dublin")),
247+
ExpectedOffset: 0,
248+
},
249+
{
250+
Name: "IrelandDaylightSavings",
251+
Now: time.Date(2021, 4, 3, 0, 0, 0, 0, time.UTC),
252+
Loc: must(time.LoadLocation("Europe/Dublin")),
253+
ExpectedOffset: -1,
254+
},
232255
{
233256
Name: "HalfHourTz",
257+
Now: time.Date(2024, 1, 20, 6, 0, 0, 0, must(time.LoadLocation("Asia/Yangon"))),
234258
// This timezone is +6:30, but the function rounds to the nearest hour.
235259
// This is intentional because our DAUs endpoint only covers 1-hour offsets.
236260
// If the user is in a non-hour timezone, they get the closest hour bucket.
237261
Loc: must(time.LoadLocation("Asia/Yangon")),
238-
ExpectedOffset: 6,
262+
ExpectedOffset: -6,
239263
},
240264
}
241265

@@ -244,7 +268,7 @@ func TestTimezoneOffsets(t *testing.T) {
244268
t.Run(c.Name, func(t *testing.T) {
245269
t.Parallel()
246270

247-
offset := codersdk.TimezoneOffsetHour(c.Loc)
271+
offset := codersdk.TimezoneOffsetHourWithTime(c.Now, c.Loc)
248272
require.Equal(t, c.ExpectedOffset, offset)
249273
})
250274
}

‎docs/api/insights.md

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/api/schemas.md

+6-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/api/templates.md

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)