Skip to content

Commit b2413a5

Browse files
aslilacaslilac
authored
chore: reimplement activity status and autostop improvements (#12175)
1 parent 4e7beee commit b2413a5

File tree

17 files changed

+658
-173
lines changed

17 files changed

+658
-173
lines changed

‎coderd/agentapi/api.go

+1
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func New(opts Options) *API {
114114
api.StatsAPI = &StatsAPI{
115115
AgentFn: api.agent,
116116
Database: opts.Database,
117+
Pubsub: opts.Pubsub,
117118
Log: opts.Log,
118119
StatsBatcher: opts.StatsBatcher,
119120
TemplateScheduleStore: opts.TemplateScheduleStore,

‎coderd/agentapi/stats.go

+14
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import (
1616
"github.com/coder/coder/v2/coderd/autobuild"
1717
"github.com/coder/coder/v2/coderd/database"
1818
"github.com/coder/coder/v2/coderd/database/dbtime"
19+
"github.com/coder/coder/v2/coderd/database/pubsub"
1920
"github.com/coder/coder/v2/coderd/prometheusmetrics"
2021
"github.com/coder/coder/v2/coderd/schedule"
22+
"github.com/coder/coder/v2/codersdk"
2123
)
2224

2325
type StatsBatcher interface {
@@ -27,6 +29,7 @@ type StatsBatcher interface {
2729
type StatsAPI struct {
2830
AgentFn func(context.Context) (database.WorkspaceAgent, error)
2931
Database database.Store
32+
Pubsub pubsub.Pubsub
3033
Log slog.Logger
3134
StatsBatcher StatsBatcher
3235
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
@@ -130,5 +133,16 @@ func (a *StatsAPI) UpdateStats(ctx context.Context, req *agentproto.UpdateStatsR
130133
return nil, xerrors.Errorf("update stats in database: %w", err)
131134
}
132135

136+
// Tell the frontend about the new agent report, now that everything is updated
137+
a.publishWorkspaceAgentStats(ctx, workspace.ID)
138+
133139
return res, nil
134140
}
141+
142+
func (a *StatsAPI) publishWorkspaceAgentStats(ctx context.Context, workspaceID uuid.UUID) {
143+
err := a.Pubsub.Publish(codersdk.WorkspaceNotifyChannel(workspaceID), []byte{})
144+
if err != nil {
145+
a.Log.Warn(ctx, "failed to publish workspace agent stats",
146+
slog.F("workspace_id", workspaceID), slog.Error(err))
147+
}
148+
}

‎coderd/agentapi/stats_test.go

+31-4
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@ import (
1919
"github.com/coder/coder/v2/coderd/database"
2020
"github.com/coder/coder/v2/coderd/database/dbmock"
2121
"github.com/coder/coder/v2/coderd/database/dbtime"
22+
"github.com/coder/coder/v2/coderd/database/pubsub"
2223
"github.com/coder/coder/v2/coderd/prometheusmetrics"
2324
"github.com/coder/coder/v2/coderd/schedule"
25+
"github.com/coder/coder/v2/codersdk"
26+
"github.com/coder/coder/v2/testutil"
2427
)
2528

2629
type statsBatcher struct {
@@ -78,8 +81,10 @@ func TestUpdateStates(t *testing.T) {
7881
t.Parallel()
7982

8083
var (
81-
now = dbtime.Now()
82-
dbM = dbmock.NewMockStore(gomock.NewController(t))
84+
now = dbtime.Now()
85+
dbM = dbmock.NewMockStore(gomock.NewController(t))
86+
ps = pubsub.NewInMemory()
87+
8388
templateScheduleStore = schedule.MockTemplateScheduleStore{
8489
GetFn: func(context.Context, database.Store, uuid.UUID) (schedule.TemplateScheduleOptions, error) {
8590
panic("should not be called")
@@ -125,6 +130,7 @@ func TestUpdateStates(t *testing.T) {
125130
return agent, nil
126131
},
127132
Database: dbM,
133+
Pubsub: ps,
128134
StatsBatcher: batcher,
129135
TemplateScheduleStore: templateScheduleStorePtr(templateScheduleStore),
130136
AgentStatsRefreshInterval: 10 * time.Second,
@@ -164,6 +170,14 @@ func TestUpdateStates(t *testing.T) {
164170
// User gets fetched to hit the UpdateAgentMetricsFn.
165171
dbM.EXPECT().GetUserByID(gomock.Any(), user.ID).Return(user, nil)
166172

173+
// Ensure that pubsub notifications are sent.
174+
notifyDescription := make(chan []byte)
175+
ps.Subscribe(codersdk.WorkspaceNotifyChannel(workspace.ID), func(_ context.Context, description []byte) {
176+
go func() {
177+
notifyDescription <- description
178+
}()
179+
})
180+
167181
resp, err := api.UpdateStats(context.Background(), req)
168182
require.NoError(t, err)
169183
require.Equal(t, &agentproto.UpdateStatsResponse{
@@ -179,7 +193,13 @@ func TestUpdateStates(t *testing.T) {
179193
require.Equal(t, user.ID, batcher.lastUserID)
180194
require.Equal(t, workspace.ID, batcher.lastWorkspaceID)
181195
require.Equal(t, req.Stats, batcher.lastStats)
182-
196+
ctx := testutil.Context(t, testutil.WaitShort)
197+
select {
198+
case <-ctx.Done():
199+
t.Error("timed out while waiting for pubsub notification")
200+
case description := <-notifyDescription:
201+
require.Equal(t, description, []byte{})
202+
}
183203
require.True(t, updateAgentMetricsFnCalled)
184204
})
185205

@@ -189,6 +209,7 @@ func TestUpdateStates(t *testing.T) {
189209
var (
190210
now = dbtime.Now()
191211
dbM = dbmock.NewMockStore(gomock.NewController(t))
212+
ps = pubsub.NewInMemory()
192213
templateScheduleStore = schedule.MockTemplateScheduleStore{
193214
GetFn: func(context.Context, database.Store, uuid.UUID) (schedule.TemplateScheduleOptions, error) {
194215
panic("should not be called")
@@ -214,6 +235,7 @@ func TestUpdateStates(t *testing.T) {
214235
return agent, nil
215236
},
216237
Database: dbM,
238+
Pubsub: ps,
217239
StatsBatcher: batcher,
218240
TemplateScheduleStore: templateScheduleStorePtr(templateScheduleStore),
219241
AgentStatsRefreshInterval: 10 * time.Second,
@@ -245,6 +267,7 @@ func TestUpdateStates(t *testing.T) {
245267

246268
var (
247269
dbM = dbmock.NewMockStore(gomock.NewController(t))
270+
ps = pubsub.NewInMemory()
248271
req = &agentproto.UpdateStatsRequest{
249272
Stats: &agentproto.Stats{
250273
ConnectionsByProto: map[string]int64{}, // len() == 0
@@ -256,6 +279,7 @@ func TestUpdateStates(t *testing.T) {
256279
return agent, nil
257280
},
258281
Database: dbM,
282+
Pubsub: ps,
259283
StatsBatcher: nil, // should not be called
260284
TemplateScheduleStore: nil, // should not be called
261285
AgentStatsRefreshInterval: 10 * time.Second,
@@ -290,7 +314,9 @@ func TestUpdateStates(t *testing.T) {
290314
nextAutostart := now.Add(30 * time.Minute).UTC() // always sent to DB as UTC
291315

292316
var (
293-
dbM = dbmock.NewMockStore(gomock.NewController(t))
317+
dbM = dbmock.NewMockStore(gomock.NewController(t))
318+
ps = pubsub.NewInMemory()
319+
294320
templateScheduleStore = schedule.MockTemplateScheduleStore{
295321
GetFn: func(context.Context, database.Store, uuid.UUID) (schedule.TemplateScheduleOptions, error) {
296322
return schedule.TemplateScheduleOptions{
@@ -322,6 +348,7 @@ func TestUpdateStates(t *testing.T) {
322348
return agent, nil
323349
},
324350
Database: dbM,
351+
Pubsub: ps,
325352
StatsBatcher: batcher,
326353
TemplateScheduleStore: templateScheduleStorePtr(templateScheduleStore),
327354
AgentStatsRefreshInterval: 15 * time.Second,

‎site/src/hooks/useTime.ts

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { useEffect, useState } from "react";
2+
import { useEffectEvent } from "./hookPolyfills";
3+
4+
interface UseTimeOptions {
5+
/**
6+
* Can be set to `true` to disable checking for updates in circumstances where it is known
7+
* that there is no work to do.
8+
*/
9+
disabled?: boolean;
10+
11+
/**
12+
* The amount of time in milliseconds that should pass between checking for updates.
13+
*/
14+
interval?: number;
15+
}
16+
17+
/**
18+
* useTime allows a component to rerender over time without a corresponding state change.
19+
* An example could be a relative timestamp (eg. "in 5 minutes") that should count down as it
20+
* approaches.
21+
*/
22+
export function useTime<T>(func: () => T, options: UseTimeOptions = {}): T {
23+
const [computedValue, setComputedValue] = useState(() => func());
24+
const { disabled = false, interval = 1000 } = options;
25+
26+
const thunk = useEffectEvent(func);
27+
28+
useEffect(() => {
29+
if (disabled) {
30+
return;
31+
}
32+
33+
const handle = setInterval(() => {
34+
setComputedValue(() => thunk());
35+
}, interval);
36+
37+
return () => {
38+
clearInterval(handle);
39+
};
40+
}, [thunk, disabled, interval]);
41+
42+
return computedValue;
43+
}

‎site/src/modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip.tsx

+1-5
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,7 @@ export const WorkspaceOutdatedTooltipContent: FC<TooltipProps> = ({
8787
<div css={styles.bold}>Message</div>
8888
<div>
8989
{activeVersion ? (
90-
activeVersion.message === "" ? (
91-
"No message"
92-
) : (
93-
activeVersion.message
94-
)
90+
activeVersion.message || "No message"
9591
) : (
9692
<Skeleton variant="text" height={20} width={150} />
9793
)}

‎site/src/modules/workspaces/activity.ts

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import dayjs from "dayjs";
2+
import type { Workspace } from "api/typesGenerated";
3+
4+
export type WorkspaceActivityStatus =
5+
| "ready"
6+
| "connected"
7+
| "inactive"
8+
| "notConnected"
9+
| "notRunning";
10+
11+
export function getWorkspaceActivityStatus(
12+
workspace: Workspace,
13+
): WorkspaceActivityStatus {
14+
const builtAt = dayjs(workspace.latest_build.created_at);
15+
const usedAt = dayjs(workspace.last_used_at);
16+
const now = dayjs();
17+
18+
if (workspace.latest_build.status !== "running") {
19+
return "notRunning";
20+
}
21+
22+
// This needs to compare to `usedAt` instead of `now`, because the "grace period" for
23+
// marking a workspace as "Connected" is a lot longer. If you compared `builtAt` to `now`,
24+
// you could end up switching from "Ready" to "Connected" without ever actually connecting.
25+
const isBuiltRecently = builtAt.isAfter(usedAt.subtract(1, "second"));
26+
// By default, agents report connection stats every 30 seconds, so 2 minutes should be
27+
// plenty. Disconnection will be reflected relatively-quickly
28+
const isUsedRecently = usedAt.isAfter(now.subtract(2, "minute"));
29+
30+
// If the build is still "fresh", it'll be a while before the `last_used_at` gets bumped in
31+
// a significant way by the agent, so just label it as ready instead of connected.
32+
// Wait until `last_used_at` is after the time that the build finished, _and_ still
33+
// make sure to check that it's recent, so that we don't show "Ready" indefinitely.
34+
if (isUsedRecently && isBuiltRecently && workspace.health.healthy) {
35+
return "ready";
36+
}
37+
38+
if (isUsedRecently) {
39+
return "connected";
40+
}
41+
42+
// TODO: It'd be nice if we could differentiate between "connected but inactive" and
43+
// "not connected", but that will require some relatively substantial backend work.
44+
return "inactive";
45+
}

‎site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ describe("AccountPage", () => {
2929
Promise.resolve({
3030
id: userId,
3131
email: "user@coder.com",
32-
created_at: new Date().toString(),
32+
created_at: new Date().toISOString(),
3333
status: "active",
3434
organization_ids: ["123"],
3535
roles: [],
3636
avatar_url: "",
37-
last_seen_at: new Date().toString(),
37+
last_seen_at: new Date().toISOString(),
3838
login_type: "password",
3939
theme_preference: "",
4040
...data,

‎site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx

+19-12
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
import { render, screen } from "@testing-library/react";
2-
import { ThemeProvider } from "contexts/ThemeProvider";
2+
import userEvent from "@testing-library/user-event";
3+
import { type FC } from "react";
34
import { QueryClient, QueryClientProvider, useQuery } from "react-query";
4-
import { MockWorkspace } from "testHelpers/entities";
5-
import { WorkspaceScheduleControls } from "./WorkspaceScheduleControls";
6-
import { workspaceByOwnerAndName } from "api/queries/workspaces";
75
import { RouterProvider, createMemoryRouter } from "react-router-dom";
8-
import userEvent from "@testing-library/user-event";
9-
import { server } from "testHelpers/server";
10-
import { rest } from "msw";
116
import dayjs from "dayjs";
7+
import { rest } from "msw";
128
import * as API from "api/api";
9+
import { workspaceByOwnerAndName } from "api/queries/workspaces";
10+
import { ThemeProvider } from "contexts/ThemeProvider";
11+
import { MockTemplate, MockWorkspace } from "testHelpers/entities";
12+
import { server } from "testHelpers/server";
1313
import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
14+
import { WorkspaceScheduleControls } from "./WorkspaceScheduleControls";
1415

15-
const Wrapper = () => {
16+
const Wrapper: FC = () => {
1617
const { data: workspace } = useQuery(
1718
workspaceByOwnerAndName(MockWorkspace.owner_name, MockWorkspace.name),
1819
);
@@ -21,7 +22,13 @@ const Wrapper = () => {
2122
return null;
2223
}
2324

24-
return <WorkspaceScheduleControls workspace={workspace} canUpdateSchedule />;
25+
return (
26+
<WorkspaceScheduleControls
27+
workspace={workspace}
28+
template={MockTemplate}
29+
canUpdateSchedule
30+
/>
31+
);
2532
};
2633

2734
const BASE_DEADLINE = dayjs().add(3, "hour");
@@ -75,7 +82,7 @@ test("add 3 hours to deadline", async () => {
7582
await screen.findByText(
7683
"Workspace shutdown time has been successfully updated.",
7784
);
78-
expect(screen.getByText("Stop in 6 hours")).toBeInTheDocument();
85+
expect(await screen.findByText("Stop in 6 hours")).toBeInTheDocument();
7986

8087
// Mocks are used here because the 'usedDeadline' is a dayjs object, which
8188
// can't be directly compared.
@@ -87,7 +94,7 @@ test("add 3 hours to deadline", async () => {
8794
);
8895
});
8996

90-
test("remove 3 hours to deadline", async () => {
97+
test("remove 2 hours to deadline", async () => {
9198
const user = userEvent.setup();
9299
const updateDeadlineSpy = jest
93100
.spyOn(API, "putWorkspaceExtension")
@@ -103,7 +110,7 @@ test("remove 3 hours to deadline", async () => {
103110
await screen.findByText(
104111
"Workspace shutdown time has been successfully updated.",
105112
);
106-
expect(screen.getByText("Stop in an hour")).toBeInTheDocument();
113+
expect(await screen.findByText("Stop in an hour")).toBeInTheDocument();
107114

108115
// Mocks are used here because the 'usedDeadline' is a dayjs object, which
109116
// can't be directly compared.

0 commit comments

Comments
 (0)