feat(site): add workspace timings by BrunoQuaresma · Pull Request #15068 · coder/coder

BrunoQuaresma · 2024-10-14T19:31:10Z

Demo:

Screen.Recording.2024-10-18.at.15.48.28.mp4

…ce-timings-ui

BrunoQuaresma · 2024-10-18T19:01:43Z

dannykopping

Fantastic work! I'll leave the frontend review to the folks who know best. I'm really excited to see this be added.

Question: when a script occurs, does the error indicator sit to the left of it in the timeline overview view, or is it always aligned to the far left?

dannykopping · 2024-10-18T19:09:09Z

site/src/modules/workspaces/WorkspaceTiming/ScriptsChart.tsx

+									<Tooltip
+										title={
+											<TooltipTitle>
+												Script exited with {t.exitCode} code


Suggested change

Script exited with {t.exitCode} code

Script exited with code {t.exitCode}

Is there a way to make the exit code more visually distinct? Perhaps bold, with a different colour, monospace font, etc?

code-asher

This is really cool!!

code-asher · 2024-10-18T20:09:21Z

site/src/api/queries/workspaces.ts

+
+export const timings = (workspaceId: string) => {
+	return {
+		queryKey: ["workspaces", workspaceId, "timings"],


Should this include the build ID or something? I think this means if a workspace gets rebuilt it could cache the same timings because the workspace ID is the same, and I suppose you might have to refresh the page to get the new timings?

Somewhat related, when I create a new workspace I have to refresh or navigate away and back again before the timings load.

Good catch!

code-asher · 2024-10-18T20:37:45Z

site/src/modules/workspaces/WorkspaceTiming/Chart/XAxis.tsx

+	scale: number;
+};
+
+export const XAxis: FC<XAxisProps> = ({ ticks, scale, ...htmlProps }) => {


I am not sure if this component is related, but I found that if I:

Stop a running workspace, wait for it to stop

Click "Workspaces"

Go back into my now-stopped workspace

Then the timings run off edge of the component and I am not able to see them. It seems to fix itself if I navigate away and come back again. Weirdly, the same issue does not appear to happen if I start a stopped workspace then navigate away and back again, it only happens when I stop my workspace.

Another thing is if I resize my screen then the timings and legend get cut off, idk if it is worth adding resizing logic but maybe we can just throw in a horizontal scrollbar.

code-asher · 2024-10-18T20:42:50Z

site/src/modules/workspaces/WorkspaceTiming/Chart/XAxis.tsx

+	// 	The X axis should occupy all available space. If there is extra space,
+	// 	increase the column width accordingly. Use a CSS variable to propagate the
+	// 	value to the child components.


Not important, but this comment has a bunch of space at the beginning 😄

code-asher · 2024-10-18T20:46:05Z

site/src/modules/workspaces/WorkspaceTiming/Chart/Bar.tsx

+		minWidth: 24,
+		// Increase hover area
+		position: "relative",
+		"&::after": {


Neat trick!

code-asher · 2024-10-18T20:48:58Z

site/src/modules/workspaces/WorkspaceTiming/Chart/YAxis.tsx

+			overflow: "hidden",
+			textOverflow: "ellipsis",


Could we add a hover, maybe a tooltip, in case someone wants to see the full name?

The user can already do it by hovering over the bar.

Ohhh I see. My instinct was to hover the name.

code-asher · 2024-10-18T21:01:37Z

site/src/modules/workspaces/WorkspaceTiming/Chart/utils.ts

+const scales = [100, 500, 5_000];
+
+const pickScale = (totalTime: number): number => {
+	const reversedScales = scales.slice().reverse();


Looks like this is the only place where scales is used? We could just define scales already reversed.

code-asher · 2024-10-18T21:06:13Z

site/src/modules/workspaces/WorkspaceTiming/Chart/utils.ts

+	return reversedScales[0];
+};
+
+export const makeTicks = (time: number) => {


Do we need to be worried about small screens? These ticks can get really cramped in a small screen. I wonder if we should just set a minimum width and add horizontal scrollbars.

Hmm, it should use horizontal scrollbars already, but I will investigate that. We should be concerned about tablet viewports.

Found the issue and it is fixed!

code-asher · 2024-10-18T21:19:21Z

site/src/modules/workspaces/WorkspaceTiming/Chart/Blocks.tsx

+			{Array.from({ length: nOfBlocks }, (_, i) => i + 1).map((n) => (
+				<div key={n} css={styles.block} style={{ minWidth: blockSize }} />
+			))}


Hopefully this is the right spot, but one thing that threw me off is that my template has five resources, and I saw five blue blocks, but only three of them show up in the chart when I click into the bar. Edit: I think because we filter Coder resources in the sub-chart but not the top-level chart.

It was also a little surprising that the blue blocks were all even width instead of matching up with their timings.

code-asher · 2024-10-18T21:26:59Z

site/src/modules/workspaces/WorkspaceTiming/Chart/Bar.tsx

+	 * Color scheme for the bar. If not passed the default gray color will be
+	 * used.


I kept thinking the gray box was a checkbox 😆

I wonder if we could change the shape or default color a bit, but then again I might be the only one dumb enough not to realize it was the legend color.

lol, @chrifro wdyt?

You are referring to the "state refresh" color right?

We could totally change that color. Either to orange if we also want to draw more attention to the "state refresh"
orange-bg: #431407
orange-stroke: #FDBA74 (we use the same color already on different views)

or if we want to keep it more subtle to grey
grey-bg: #18181B
grey-stroke: #71717A

something to keep in mind: the colors will likely slightly change as part of #14780. Then the difference between the newly introduced grey colors and the bg will be more obvious (see screenshot).

Sorry for the late reply, yes that is what I meant! When there are two or more, it is pretty obvious they are a legend, but with just one I thought it was supposed to be a checkbox, in particular the dark or grey ones. This might just be me being dumb though.

code-asher · 2024-10-18T21:30:03Z

site/src/modules/workspaces/WorkspaceTiming/StagesChart.tsx

+										>
+											{/** We only want to expand stages with more than one resource */}
+											{t.resources > 1 ? (
+												<ClickableBar


I am seeing a case where my plan stage is clickable, but there are no timings or rows when I click in. The resources are data.coder_provisioner.me, data.coder_workspace_owner.me, and data.coder_workspace.me.

I think because we are filtering out Coder resources in the resource chart, but not in the top-level chart. Not sure if that means we should also filter them out in the top-level, or if we should include rows for them.

…ce-timings-ui

chrifro

Looks great @BrunoQuaresma 👏 ✨

One comment non-blocking:

what do you think of adjusting the height of the whole drop-down to fit the content? Currently, there is a lot more bottom spacing on the initial view vs the different stages (see screenshots).
Consequences: either the box height gets smaller for the stage views as well or the box height changes for the stage views. Thoughts?

BrunoQuaresma · 2024-10-23T12:38:35Z

@chrifro Hm... I see what you’re trying to achieve, but I usually prefer to have elements with dynamic data and unpredictable sizes set to a fixed height to avoid layout shifting. In my opinion, we could release the component as it is and see if we receive any feedback from users on this. Does that sound like a good plan?

dannykopping · 2024-10-23T13:57:03Z

site/src/modules/workspaces/WorkspaceTiming/WorkspaceTimings.tsx

+				) : (
+					<KeyboardArrowDown css={{ fontSize: 16, marginRight: 16 }} />
+				)}
+				<span>Provisioning time</span>


Nit: this is not just provisioning time since it also includes agent timings.
We should use a more descriptive name, like "Build timeline".

BrunoQuaresma added 30 commits September 19, 2024 18:19

Add base components for the chart

5268b1e

Improve spacing calc

4d509f9

Make bars clickable

d48624b

Refactor code to allow multiple views

62152ce

Add basic view and breadcrumbs

fd84ed9

Add resource filtering

f7f09ff

Find the right tick spacings

2ffc75a

Add colors to the bars

a8372e1

Do not display Coder resource

f7c7488

Add legends

0868185

Handle empty search

54d13c8

Improve coder resource filter and adjust bar hover

714e37b

Only scroll the chart

a2dd126

Add tooltip

0b4747e

Refactor code and improve legends

49d3a72

Adjust columns to fit the space

647635d

Customize scroll

6c742aa

Add info tooltip

9a8bb59

Fix fmt

4139151

Fix nblock gen

bcff9c6

Fix key

97b25d9

Debug on chromatic

6d5c344

Another debug image

c4bd74e

Try with useEffect

939ec9a

Fix labels alignment

49c69e0

Increase border radius tooltip

61008a3

Add scroll mask

f969ef2

Merge branch 'main' of https://github.com/coder/coder into bq/workspa…

f55033a

…ce-timings-ui

Include agent scripts

00616d3

Add failed icon

8cc7eef

BrunoQuaresma marked this pull request as ready for review October 18, 2024 19:00

dannykopping reviewed Oct 18, 2024

View reviewed changes

code-asher reviewed Oct 18, 2024

View reviewed changes

BrunoQuaresma added 8 commits October 21, 2024 19:13

Merge branch 'main' of https://github.com/coder/coder into bq/workspa…

a1ee002

…ce-timings-ui

Fix code verbiage

75ea1e5

Fetch timings from build and not workspace

edb885c

Don't could coder resources for blocks

f8bb22c

Fix timings getting fetched before build getting done

1ff245c

Fix comment spacing

541ccba

Revert scales on def

b3f0351

Fix columnd calcl

65ca087

BrunoQuaresma requested review from a team, code-asher, dannykopping and jaaydenh and removed request for a team and code-asher October 21, 2024 20:11

Fix fmt

f288169

chrifro approved these changes Oct 23, 2024

View reviewed changes

Add extra tests

08874a3

BrunoQuaresma merged commit d89eceb into main Oct 23, 2024

BrunoQuaresma deleted the bq/workspace-timings-ui branch October 23, 2024 13:09

github-actions bot locked and limited conversation to collaborators Oct 23, 2024

dannykopping reviewed Oct 23, 2024

View reviewed changes

	Script exited with {t.exitCode} code
	Script exited with code {t.exitCode}

		* Color scheme for the bar. If not passed the default gray color will be
		* used.

Conversation

BrunoQuaresma commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BrunoQuaresma commented Oct 18, 2024

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrifro left a comment

Choose a reason for hiding this comment

Uh oh!

BrunoQuaresma commented Oct 23, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

BrunoQuaresma commented Oct 14, 2024 •

edited

Loading

code-asher Oct 18, 2024 •

edited

Loading

code-asher Oct 29, 2024 •

edited

Loading