Skip to content

Commit 39b846f

Browse files
johnstcnjohnstcn
committed
fix(coderd): workspaceapps: update last_used_at when workspace app reports stat
1 parent 5bfbf9f commit 39b846f

File tree

13 files changed

+156
-6
lines changed

13 files changed

+156
-6
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,15 @@ func (q *querier) ArchiveUnusedTemplateVersions(ctx context.Context, arg databas
695695
return q.db.ArchiveUnusedTemplateVersions(ctx, arg)
696696
}
697697

698+
func (q *querier) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg database.BatchUpdateWorkspaceLastUsedAtParams) error {
699+
// Could be any workspace and checking auth to each workspace is overkill for the purpose
700+
// of this function.
701+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceWorkspace.All()); err != nil {
702+
return err
703+
}
704+
return q.db.BatchUpdateWorkspaceLastUsedAt(ctx, arg)
705+
}
706+
698707
func (q *querier) CleanTailnetCoordinators(ctx context.Context) error {
699708
if err := q.authorizeContext(ctx, rbac.ActionDelete, rbac.ResourceTailnetCoordinator); err != nil {
700709
return err

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,6 +1549,13 @@ func (s *MethodTestSuite) TestWorkspace() {
15491549
ID: ws.ID,
15501550
}).Asserts(ws, rbac.ActionUpdate).Returns()
15511551
}))
1552+
s.Run("BatchUpdateWorkspaceLastUsedAt", s.Subtest(func(db database.Store, check *expects) {
1553+
ws1 := dbgen.Workspace(s.T(), db, database.Workspace{})
1554+
ws2 := dbgen.Workspace(s.T(), db, database.Workspace{})
1555+
check.Args(database.BatchUpdateWorkspaceLastUsedAtParams{
1556+
IDs: []uuid.UUID{ws1.ID, ws2.ID},
1557+
}).Asserts(rbac.ResourceWorkspace.All(), rbac.ActionUpdate).Returns()
1558+
}))
15521559
s.Run("UpdateWorkspaceTTL", s.Subtest(func(db database.Store, check *expects) {
15531560
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
15541561
check.Args(database.UpdateWorkspaceTTLParams{

‎coderd/database/dbmem/dbmem.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,34 @@ func (q *FakeQuerier) ArchiveUnusedTemplateVersions(_ context.Context, arg datab
963963
return archived, nil
964964
}
965965

966+
func (q *FakeQuerier) BatchUpdateWorkspaceLastUsedAt(ctx context.Context, arg database.BatchUpdateWorkspaceLastUsedAtParams) error {
967+
err := validateDatabaseType(arg)
968+
if err != nil {
969+
return err
970+
}
971+
972+
q.mutex.Lock()
973+
defer q.mutex.Unlock()
974+
975+
// temporary map to avoid O(q.workspaces*arg.workspaceIds)
976+
m := make(map[uuid.UUID]struct{})
977+
for _, id := range arg.IDs {
978+
m[id] = struct{}{}
979+
}
980+
n := 0
981+
for i := 0; i < len(q.workspaces); i++ {
982+
if _, found := m[q.workspaces[i].ID]; !found {
983+
continue
984+
}
985+
q.workspaces[i].LastUsedAt = arg.LastUsedAt
986+
n++
987+
}
988+
if n == 0 {
989+
return sql.ErrNoRows
990+
}
991+
return nil
992+
}
993+
966994
func (*FakeQuerier) CleanTailnetCoordinators(_ context.Context) error {
967995
return ErrUnimplemented
968996
}

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,14 @@ SET
357357
WHERE
358358
id = $1;
359359

360+
-- name: BatchUpdateWorkspaceLastUsedAt :exec
361+
UPDATE
362+
workspaces
363+
SET
364+
last_used_at = @last_used_at
365+
WHERE
366+
id = ANY(@ids :: uuid[]);
367+
360368
-- name: GetDeploymentWorkspaceStats :one
361369
WITH workspaces_with_jobs AS (
362370
SELECT

‎coderd/httpapi/websocket.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import (
44
"context"
55
"time"
66

7-
"cdr.dev/slog"
87
"nhooyr.io/websocket"
8+
9+
"cdr.dev/slog"
910
)
1011

1112
// Heartbeat loops to ping a WebSocket to keep it alive.

‎coderd/workspaceapps/apptest/apptest.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
6464
// reconnecting-pty proxy server we want to test is mounted.
6565
client := appDetails.AppClient(t)
6666
testReconnectingPTY(ctx, t, client, appDetails.Agent.ID, "")
67+
assertWorkspaceLastUsedAtUpdated(t, client, appDetails)
6768
})
6869

6970
t.Run("SignedTokenQueryParameter", func(t *testing.T) {
@@ -92,6 +93,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
9293
// Make an unauthenticated client.
9394
unauthedAppClient := codersdk.New(appDetails.AppClient(t).URL)
9495
testReconnectingPTY(ctx, t, unauthedAppClient, appDetails.Agent.ID, issueRes.SignedToken)
96+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
9597
})
9698
})
9799

@@ -117,6 +119,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
117119
body, err := io.ReadAll(resp.Body)
118120
require.NoError(t, err)
119121
require.Contains(t, string(body), "Path-based applications are disabled")
122+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
120123
})
121124

122125
t.Run("LoginWithoutAuthOnPrimary", func(t *testing.T) {
@@ -142,6 +145,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
142145
require.NoError(t, err)
143146
require.True(t, loc.Query().Has("message"))
144147
require.True(t, loc.Query().Has("redirect"))
148+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
145149
})
146150

147151
t.Run("LoginWithoutAuthOnProxy", func(t *testing.T) {
@@ -179,6 +183,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
179183
// request is getting stripped.
180184
require.Equal(t, u.Path, redirectURI.Path+"/")
181185
require.Equal(t, u.RawQuery, redirectURI.RawQuery)
186+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
182187
})
183188

184189
t.Run("NoAccessShould404", func(t *testing.T) {
@@ -195,6 +200,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
195200
require.NoError(t, err)
196201
defer resp.Body.Close()
197202
require.Equal(t, http.StatusNotFound, resp.StatusCode)
203+
assertWorkspaceLastUsedAtNotUpdated(t, appDetails.AppClient(t), appDetails)
198204
})
199205

200206
t.Run("RedirectsWithSlash", func(t *testing.T) {
@@ -209,6 +215,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
209215
require.NoError(t, err)
210216
defer resp.Body.Close()
211217
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
218+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
212219
})
213220

214221
t.Run("RedirectsWithQuery", func(t *testing.T) {
@@ -226,6 +233,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
226233
loc, err := resp.Location()
227234
require.NoError(t, err)
228235
require.Equal(t, proxyTestAppQuery, loc.RawQuery)
236+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
229237
})
230238

231239
t.Run("Proxies", func(t *testing.T) {
@@ -267,6 +275,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
267275
require.NoError(t, err)
268276
require.Equal(t, proxyTestAppBody, string(body))
269277
require.Equal(t, http.StatusOK, resp.StatusCode)
278+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
270279
})
271280

272281
t.Run("ProxiesHTTPS", func(t *testing.T) {
@@ -312,6 +321,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
312321
require.NoError(t, err)
313322
require.Equal(t, proxyTestAppBody, string(body))
314323
require.Equal(t, http.StatusOK, resp.StatusCode)
324+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
315325
})
316326

317327
t.Run("BlocksMe", func(t *testing.T) {
@@ -331,6 +341,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
331341
body, err := io.ReadAll(resp.Body)
332342
require.NoError(t, err)
333343
require.Contains(t, string(body), "must be accessed with the full username, not @me")
344+
assertWorkspaceLastUsedAtNotUpdated(t, appDetails.AppClient(t), appDetails)
334345
})
335346

336347
t.Run("ForwardsIP", func(t *testing.T) {
@@ -349,6 +360,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
349360
require.Equal(t, proxyTestAppBody, string(body))
350361
require.Equal(t, http.StatusOK, resp.StatusCode)
351362
require.Equal(t, "1.1.1.1,127.0.0.1", resp.Header.Get("X-Forwarded-For"))
363+
assertWorkspaceLastUsedAtUpdated(t, appDetails.AppClient(t), appDetails)
352364
})
353365

354366
t.Run("ProxyError", func(t *testing.T) {
@@ -361,6 +373,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
361373
require.NoError(t, err)
362374
defer resp.Body.Close()
363375
require.Equal(t, http.StatusBadGateway, resp.StatusCode)
376+
assertWorkspaceLastUsedAtNotUpdated(t, appDetails.AppClient(t), appDetails)
364377
})
365378

366379
t.Run("NoProxyPort", func(t *testing.T) {
@@ -375,6 +388,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
375388
// TODO(@deansheather): This should be 400. There's a todo in the
376389
// resolve request code to fix this.
377390
require.Equal(t, http.StatusInternalServerError, resp.StatusCode)
391+
assertWorkspaceLastUsedAtNotUpdated(t, appDetails.AppClient(t), appDetails)
378392
})
379393
})
380394

@@ -548,6 +562,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
548562
require.NoError(t, err)
549563
resp.Body.Close()
550564
require.Equal(t, http.StatusOK, resp.StatusCode)
565+
assertWorkspaceLastUsedAtUpdated(t, appClient, appDetails)
551566
})
552567
}
553568
})
@@ -1549,3 +1564,23 @@ func testReconnectingPTY(ctx context.Context, t *testing.T, client *codersdk.Cli
15491564
// Ensure the connection closes.
15501565
require.ErrorIs(t, tr.ReadUntil(ctx, nil), io.EOF)
15511566
}
1567+
1568+
// Accessing an app should update the workspace's LastUsedAt.
1569+
func assertWorkspaceLastUsedAtUpdated(t testing.TB, client *codersdk.Client, details *Details) {
1570+
t.Helper()
1571+
1572+
details.FlushStats()
1573+
ws, err := client.Workspace(context.Background(), details.Workspace.ID)
1574+
require.NoError(t, err)
1575+
require.Greater(t, ws.UpdatedAt, details.Workspace.UpdatedAt, "workspace last used at not updated when it should have been")
1576+
}
1577+
1578+
// Except when it sometimes shouldn't (e.g. no access)
1579+
func assertWorkspaceLastUsedAtNotUpdated(t testing.TB, client *codersdk.Client, details *Details) {
1580+
t.Helper()
1581+
1582+
details.FlushStats()
1583+
ws, err := client.Workspace(context.Background(), details.Workspace.ID)
1584+
require.NoError(t, err)
1585+
require.Equal(t, ws.UpdatedAt, details.Workspace.UpdatedAt, "workspace last used at updated when it shouldn't have been")
1586+
}

‎coderd/workspaceapps/apptest/setup.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ type Deployment struct {
7171
SDKClient *codersdk.Client
7272
FirstUser codersdk.CreateFirstUserResponse
7373
PathAppBaseURL *url.URL
74+
FlushStats func()
7475
}
7576

7677
// DeploymentFactory generates a deployment with an API client, a path base URL,

‎coderd/workspaceapps/stats.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/coder/coder/v2/coderd/database"
1414
"github.com/coder/coder/v2/coderd/database/dbauthz"
1515
"github.com/coder/coder/v2/coderd/database/dbtime"
16+
"github.com/coder/coder/v2/coderd/util/slice"
1617
)
1718

1819
const (
@@ -117,11 +118,21 @@ func (r *StatsDBReporter) Report(ctx context.Context, stats []StatsReport) error
117118
batch.Requests = batch.Requests[:0]
118119
}
119120
}
120-
if len(batch.UserID) > 0 {
121-
err := tx.InsertWorkspaceAppStats(ctx, batch)
122-
if err != nil {
123-
return err
124-
}
121+
if len(batch.UserID) == 0 {
122+
return nil
123+
}
124+
125+
if err := tx.InsertWorkspaceAppStats(ctx, batch); err != nil {
126+
return err
127+
}
128+
129+
// TODO: There should be a better way to do this.
130+
uniqueIDs := slice.Unique(batch.WorkspaceID)
131+
if err := tx.BatchUpdateWorkspaceLastUsedAt(ctx, database.BatchUpdateWorkspaceLastUsedAtParams{
132+
IDs: uniqueIDs,
133+
LastUsedAt: dbtime.Now(), // This isn't 100% accurate, but it's good enough.
134+
}); err != nil {
135+
return err
125136
}
126137

127138
return nil
@@ -234,6 +245,7 @@ func (sc *StatsCollector) Collect(report StatsReport) {
234245
}
235246
delete(sc.statsBySessionID, report.SessionID)
236247
}
248+
sc.opts.Logger.Debug(sc.ctx, "collected workspace app stats", slog.F("report", report))
237249
}
238250

239251
// rollup performs stats rollup for sessions that fall within the

‎coderd/workspaceapps_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,13 @@ func TestWorkspaceApps(t *testing.T) {
262262
opts.AppHost = ""
263263
}
264264

265+
statsFlushCh := make(chan chan<- struct{}, 1)
266+
opts.StatsCollectorOptions.Flush = statsFlushCh
267+
flushStats := func() {
268+
flushDone := make(chan struct{})
269+
statsFlushCh <- flushDone
270+
<-flushDone
271+
}
265272
client := coderdtest.New(t, &coderdtest.Options{
266273
DeploymentValues: deploymentValues,
267274
AppHostname: opts.AppHost,
@@ -285,6 +292,7 @@ func TestWorkspaceApps(t *testing.T) {
285292
SDKClient: client,
286293
FirstUser: user,
287294
PathAppBaseURL: client.URL,
295+
FlushStats: flushStats,
288296
}
289297
})
290298
}

0 commit comments

Comments
 (0)