From d33cea1c0dd1e9647ad33df1595d7701a885d958 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 May 2023 15:09:16 -0400 Subject: [PATCH 1/2] chore: Revert to only using 1 timezone support for template DAUs Keeping the logic to support more in case we optimize later --- coderd/metricscache/metricscache.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index e70f193ee0c3c..d25b716f2df15 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -21,14 +21,23 @@ import ( "github.com/coder/retry" ) -// timezoneOffsets are the timezones that are cached and supported. +// deploymentTimezoneOffsets are the timezones that are cached and supported. // Any non-listed timezone offsets will need to use the closest supported one. -var timezoneOffsets = []int{ +var deploymentTimezoneOffsets = []int{ 0, // UTC - is listed first intentionally. -12, -11, -10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, } +// templateTimezoneOffsets are the timezones each template will use for it's DAU +// calculations. This is expensive as each template needs to do each timezone, so keep this list +// very small. +var templateTimezoneOffsets = []int{ + // Only do one for now. If people request more accurate template DAU, we can + // fix this. But it adds too much cost, so optimization is needed first. + 0, // UTC - is listed first intentionally. +} + // Cache holds the template metrics. // The aggregation queries responsible for these values can take up to a minute // on large deployments. Even in small deployments, aggregation queries can @@ -166,7 +175,7 @@ func (c *Cache) refreshDeploymentDAUs(ctx context.Context) error { ctx = dbauthz.AsSystemRestricted(ctx) deploymentDAUs := make(map[int]codersdk.DAUsResponse) - for _, tzOffset := range timezoneOffsets { + for _, tzOffset := range deploymentTimezoneOffsets { rows, err := c.database.GetDeploymentDAUs(ctx, int32(tzOffset)) if err != nil { return err @@ -199,7 +208,7 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error { } for _, template := range templates { - for _, tzOffset := range timezoneOffsets { + for _, tzOffset := range templateTimezoneOffsets { rows, err := c.database.GetTemplateDAUs(ctx, database.GetTemplateDAUsParams{ TemplateID: template.ID, TzOffset: int32(tzOffset), From 4202f43e54989e753a123452cb445eeca6349c2c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 30 May 2023 15:17:32 -0400 Subject: [PATCH 2/2] Fix unit tests --- coderd/metricscache/metricscache_test.go | 47 +++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index ee6c9e8e54884..4bc6c445e7174 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -48,12 +48,14 @@ func TestCache_TemplateUsers(t *testing.T) { uniqueUsers int } tests := []struct { - name string - args args - want want + name string + args args + tplWant want + // dauWant is optional + dauWant []codersdk.DAUEntry tzOffset int }{ - {name: "empty", args: args{}, want: want{nil, 0}}, + {name: "empty", args: args{}, tplWant: want{nil, 0}}, { name: "one hole", args: args{ @@ -62,7 +64,7 @@ func TestCache_TemplateUsers(t *testing.T) { statRow(zebra, dateH(2022, 8, 30, 0)), }, }, - want: want{[]codersdk.DAUEntry{ + tplWant: want{[]codersdk.DAUEntry{ { Date: date(2022, 8, 27), Amount: 1, @@ -90,7 +92,7 @@ func TestCache_TemplateUsers(t *testing.T) { statRow(zebra, dateH(2022, 8, 29, 0)), }, }, - want: want{[]codersdk.DAUEntry{ + tplWant: want{[]codersdk.DAUEntry{ { Date: date(2022, 8, 27), Amount: 1, @@ -116,7 +118,7 @@ func TestCache_TemplateUsers(t *testing.T) { statRow(tiger, dateH(2022, 1, 7, 0)), }, }, - want: want{[]codersdk.DAUEntry{ + tplWant: want{[]codersdk.DAUEntry{ { Date: date(2022, 1, 1), Amount: 2, @@ -159,7 +161,13 @@ func TestCache_TemplateUsers(t *testing.T) { statRow(tiger, dateH(2022, 1, 2, 0)), }, }, - want: want{[]codersdk.DAUEntry{ + tplWant: want{[]codersdk.DAUEntry{ + { + Date: date(2022, 1, 2), + Amount: 2, + }, + }, 2}, + dauWant: []codersdk.DAUEntry{ { Date: date(2022, 1, 1), Amount: 2, @@ -168,7 +176,7 @@ func TestCache_TemplateUsers(t *testing.T) { Date: date(2022, 1, 2), Amount: 2, }, - }, 2}, + }, }, { name: "tzOffsetPreviousDay", @@ -181,11 +189,17 @@ func TestCache_TemplateUsers(t *testing.T) { statRow(tiger, dateH(2022, 1, 2, 0)), }, }, - want: want{[]codersdk.DAUEntry{ + dauWant: []codersdk.DAUEntry{ { Date: date(2022, 1, 1), Amount: 2, }, + }, + tplWant: want{[]codersdk.DAUEntry{ + { + Date: date(2022, 1, 2), + Amount: 2, + }, }, 2}, }, } @@ -223,11 +237,18 @@ func TestCache_TemplateUsers(t *testing.T) { gotUniqueUsers, ok := cache.TemplateUniqueUsers(template.ID) require.True(t, ok) + if tt.dauWant != nil { + _, dauResponse, ok := cache.DeploymentDAUs(tt.tzOffset) + require.True(t, ok) + require.Equal(t, tt.dauWant, dauResponse.Entries) + } + offset, gotEntries, ok := cache.TemplateDAUs(template.ID, tt.tzOffset) require.True(t, ok) - require.Equal(t, offset, tt.tzOffset) - require.Equal(t, tt.want.entries, gotEntries.Entries) - require.Equal(t, tt.want.uniqueUsers, gotUniqueUsers) + // Template only supports 0 offset. + require.Equal(t, 0, offset) + require.Equal(t, tt.tplWant.entries, gotEntries.Entries) + require.Equal(t, tt.tplWant.uniqueUsers, gotUniqueUsers) }) } }