From 6067e102b664b6219847876939857590ca2f10f9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 2 Dec 2024 14:22:06 +0000 Subject: [PATCH 1/5] Improve comments --- .../workspaces/WorkspaceTiming/WorkspaceTimings.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx index 47873f8aaaede..b239a0aaf66af 100644 --- a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx +++ b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx @@ -58,15 +58,18 @@ export const WorkspaceTimings: FC = ({ ].sort((a, b) => { return new Date(a.started_at).getTime() - new Date(b.started_at).getTime(); }); + const [isOpen, setIsOpen] = useState(defaultIsOpen); const isLoading = timings.length === 0; - // All stages + // Each agent connection timing is a stage in the timeline to make it easier + // to users to see the timing for connection and the other scripts. const agentStageLabels = Array.from( new Set( agentConnectionTimings.map((t) => `agent (${t.workspace_agent_name})`), ), ); + const stages = [ ...provisioningStages, ...agentStageLabels.flatMap((a) => agentStages(a)), @@ -120,7 +123,8 @@ export const WorkspaceTimings: FC = ({ : mergeTimeRanges(stageTimings.map(toTimeRange)); // Prevent users from inspecting internal coder resources in - // provisioner timings. + // provisioner timings because they were not useful to the + // user and would add noise. const visibleResources = stageTimings.filter((t) => { const isProvisionerTiming = "resource" in t; return isProvisionerTiming From 2181bec72b5334e5e15dc575d2234860be02920b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 2 Dec 2024 17:23:29 +0000 Subject: [PATCH 2/5] Refetch timings until script timings are present --- site/src/api/queries/workspaceBuilds.ts | 2 -- .../WorkspacePage/WorkspaceReadyPage.tsx | 25 +++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/site/src/api/queries/workspaceBuilds.ts b/site/src/api/queries/workspaceBuilds.ts index 45f7ac3bb7fe6..f79f4518f44a3 100644 --- a/site/src/api/queries/workspaceBuilds.ts +++ b/site/src/api/queries/workspaceBuilds.ts @@ -60,14 +60,12 @@ export const infiniteWorkspaceBuilds = ( // We use readyAgentsCount to invalidate the query when an agent connects export const workspaceBuildTimings = ( workspaceBuildId: string, - readyAgentsCount: number, ) => { return { queryKey: [ "workspaceBuilds", workspaceBuildId, "timings", - { readyAgentsCount }, ], queryFn: () => API.workspaceBuildTimings(workspaceBuildId), }; diff --git a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx index cdb47f86f508c..f28e36b958879 100644 --- a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx @@ -157,13 +157,28 @@ export const WorkspaceReadyPage: FC = ({ // Cancel build const cancelBuildMutation = useMutation(cancelBuild(workspace, queryClient)); - // Build Timings. Fetch build timings only when the build job is completed. - const readyAgents = workspace.latest_build.resources - .flatMap((r) => r.agents) - .filter((a) => a && a.lifecycle_state !== "starting"); + // Workspace Timings. const timingsQuery = useQuery({ - ...workspaceBuildTimings(workspace.latest_build.id, readyAgents.length), + ...workspaceBuildTimings(workspace.latest_build.id), + + // Fetch build timings only when the build job is completed. enabled: Boolean(workspace.latest_build.job.completed_at), + + // Sometimes, the timings can be fetched before the agent script timings are + // done or saved in the database so we need to conditionally refetch the + // timings. To refetch the timings, I found the best way was to compare the + // expected amount of script timings with the current amount of script + // timings returned in the response. + refetchInterval: (data) => { + const expectedScriptTimingsCount = workspace.latest_build.resources + .flatMap((r) => r.agents) + .flatMap((a) => a?.scripts) + .filter((s) => s !== undefined).length; + const currentScriptTimingsCount = data?.agent_script_timings?.length ?? 0; + return expectedScriptTimingsCount === currentScriptTimingsCount + ? false + : 1_000; + }, }); const runLastBuild = ( From b16fad165c8733c60150b0dfce5df73ab2669e36 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 3 Dec 2024 12:10:39 +0000 Subject: [PATCH 3/5] Stay on loading state when agent script timings are empty --- .../WorkspaceTiming/WorkspaceTimings.stories.tsx | 8 ++++++++ .../workspaces/WorkspaceTiming/WorkspaceTimings.tsx | 9 ++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.stories.tsx b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.stories.tsx index 02a544ea9a718..0210353488257 100644 --- a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.stories.tsx +++ b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.stories.tsx @@ -118,3 +118,11 @@ export const DuplicatedScriptTiming: Story = { ], }, }; + +// Loading when agent script timings are empty +// Test case for https://github.com/coder/coder/issues/15273 +export const LoadingWhenAgentScriptTimingsAreEmpty: Story = { + args: { + agentScriptTimings: undefined, + }, +}; diff --git a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx index b239a0aaf66af..63fc03ad2a3de 100644 --- a/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx +++ b/site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx @@ -60,7 +60,14 @@ export const WorkspaceTimings: FC = ({ }); const [isOpen, setIsOpen] = useState(defaultIsOpen); - const isLoading = timings.length === 0; + + // If any of the timings are empty, we are still loading the data. They can be + // filled in different moments. + const isLoading = [ + provisionerTimings, + agentScriptTimings, + agentConnectionTimings, + ].some((t) => t.length === 0); // Each agent connection timing is a stage in the timeline to make it easier // to users to see the timing for connection and the other scripts. From 53f267b3f82648c00d7049cf4011aec1903e7406 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 3 Dec 2024 12:20:11 +0000 Subject: [PATCH 4/5] Fix fmt --- site/src/api/queries/workspaceBuilds.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/site/src/api/queries/workspaceBuilds.ts b/site/src/api/queries/workspaceBuilds.ts index f79f4518f44a3..a537cbed092e3 100644 --- a/site/src/api/queries/workspaceBuilds.ts +++ b/site/src/api/queries/workspaceBuilds.ts @@ -58,15 +58,9 @@ export const infiniteWorkspaceBuilds = ( }; // We use readyAgentsCount to invalidate the query when an agent connects -export const workspaceBuildTimings = ( - workspaceBuildId: string, -) => { +export const workspaceBuildTimings = (workspaceBuildId: string) => { return { - queryKey: [ - "workspaceBuilds", - workspaceBuildId, - "timings", - ], + queryKey: ["workspaceBuilds", workspaceBuildId, "timings"], queryFn: () => API.workspaceBuildTimings(workspaceBuildId), }; }; From 0c7abeeeb93cc61af97986428c17a237ed82053c Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 3 Dec 2024 14:54:27 +0000 Subject: [PATCH 5/5] Apply review suggestions --- site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx index f28e36b958879..62e1b5a6c6dd5 100644 --- a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx @@ -172,8 +172,7 @@ export const WorkspaceReadyPage: FC = ({ refetchInterval: (data) => { const expectedScriptTimingsCount = workspace.latest_build.resources .flatMap((r) => r.agents) - .flatMap((a) => a?.scripts) - .filter((s) => s !== undefined).length; + .flatMap((a) => a?.scripts ?? []).length; const currentScriptTimingsCount = data?.agent_script_timings?.length ?? 0; return expectedScriptTimingsCount === currentScriptTimingsCount ? false