Don't show progress bar when data inconclusive

coder · ammario · Nov 17, 2022 · Oct 25, 2022 · Nov 7, 2022 · Nov 7, 2022
commit 4605fd08d503e03a71b1793a57a022d157260510
diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go
@@ -3,6 +3,7 @@ package databasefake
 import (
 	"context"
 	"database/sql"
+	"math"
 	"sort"
 	"strings"
 	"sync"
@@ -280,10 +281,31 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab
 		sort.Float64s(fs)
 		return fs[len(fs)/2]
 	}
+
+	tryStddev := func(fs []float64) float64 {
+		if len(fs) == 0 {
+			return -1
+		}
+		var sum float64
+		for _, f := range fs {
+			sum += f
+		}
+		mean := sum / float64(len(fs))
+
+		var sumSqDiff float64
+		for _, f := range fs {
+			sumSqDiff += math.Pow(mean-f, 2)
+		}
+		return sumSqDiff / float64(len(fs))
+	}
+
 	var row database.GetTemplateAverageBuildTimeRow
 	row.DeleteMedian = tryMedian(deleteTimes)
+	row.DeleteStddev = tryStddev(deleteTimes)
 	row.StopMedian = tryMedian(stopTimes)
+	row.StopStddev = tryStddev(stopTimes)
 	row.StartMedian = tryMedian(startTimes)
+	row.StartStddev = tryStddev(startTimes)
 	return row, nil
 }
 

diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql
@@ -144,6 +144,9 @@ SELECT
 	-- complexities.
 	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_median,
 	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_median,
-	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median
+	coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median,
+	coalesce((stddev(exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_stddev,
+	coalesce((stddev(exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_stddev,
+	coalesce((stddev(exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_stddev
 FROM build_times
 ;
diff --git a/coderd/metricscache/buildtime.go b/coderd/metricscache/buildtime.go
@@ -0,0 +1 @@
+package metricscache
diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go
@@ -242,7 +242,11 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) {
 }
 
 func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeStats {
-	var unknown codersdk.TemplateBuildTimeStats
+	unknown := codersdk.TemplateBuildTimeStats{
+		codersdk.WorkspaceTransitionStart:  {},
+		codersdk.WorkspaceTransitionStop:   {},
+		codersdk.WorkspaceTransitionDelete: {},
+	}
 
 	m := c.templateAverageBuildTime.Load()
 	if m == nil {
@@ -256,7 +260,7 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS
 		return unknown
 	}
 
-	convertMedian := func(m float64) *int64 {
+	convertMillis := func(m float64) *int64 {
 		if m <= 0 {
 			return nil
 		}
@@ -265,8 +269,17 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS
 	}
 
 	return codersdk.TemplateBuildTimeStats{
-		StartMillis:  convertMedian(resp.StartMedian),
-		StopMillis:   convertMedian(resp.StopMedian),
-		DeleteMillis: convertMedian(resp.DeleteMedian),
+		codersdk.WorkspaceTransitionStart: {
+			Median: convertMillis(resp.StartMedian),
+			Stddev: convertMillis(resp.StartStddev),
+		},
+		codersdk.WorkspaceTransitionStop: {
+			Median: convertMillis(resp.StopMedian),
+			Stddev: convertMillis(resp.StopStddev),
+		},
+		codersdk.WorkspaceTransitionDelete: {
+			Median: convertMillis(resp.DeleteMedian),
+			Stddev: convertMillis(resp.DeleteStddev),
+		},
 	}
 }
diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go
@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"cdr.dev/slog/sloggers/slogtest"
@@ -204,6 +203,12 @@ func clockTime(t time.Time, hour, minute, sec int) time.Time {
 	return time.Date(t.Year(), t.Month(), t.Day(), hour, minute, sec, t.Nanosecond(), t.Location())
 }
 
+func requireBuildTimeStatsEmpty(t *testing.T, stats codersdk.TemplateBuildTimeStats) {
+	require.Empty(t, stats[codersdk.WorkspaceTransitionStart])
+	require.Empty(t, stats[codersdk.WorkspaceTransitionStop])
+	require.Empty(t, stats[codersdk.WorkspaceTransitionDelete])
+}
+
 func TestCache_BuildTime(t *testing.T) {
 	t.Parallel()
 
@@ -296,7 +301,7 @@ func TestCache_BuildTime(t *testing.T) {
 			require.NoError(t, err)
 
 			gotStats := cache.TemplateBuildTimeStats(template.ID)
-			require.Empty(t, gotStats, "should not have loaded yet")
+			requireBuildTimeStatsEmpty(t, gotStats)
 
 			for _, row := range tt.args.rows {
 				_, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{
@@ -328,34 +333,27 @@ func TestCache_BuildTime(t *testing.T) {
 			}
 
 			if tt.want.loads {
+				wantTransition := codersdk.WorkspaceTransition(tt.args.transition)
 				require.Eventuallyf(t, func() bool {
 					stats := cache.TemplateBuildTimeStats(template.ID)
-					return stats != codersdk.TemplateBuildTimeStats{}
+					return stats[wantTransition] != codersdk.TransitionStats{}
 				}, testutil.WaitLong, testutil.IntervalMedium,
 					"BuildTime never populated",
 				)
 
 				gotStats = cache.TemplateBuildTimeStats(template.ID)
-
-				if tt.args.transition == database.WorkspaceTransitionDelete {
-					require.Nil(t, gotStats.StopMillis)
-					require.Nil(t, gotStats.StartMillis)
-					require.Equal(t, tt.want.buildTimeMs, *gotStats.DeleteMillis)
-				}
-				if tt.args.transition == database.WorkspaceTransitionStart {
-					require.Nil(t, gotStats.StopMillis)
-					require.Nil(t, gotStats.DeleteMillis)
-					require.Equal(t, tt.want.buildTimeMs, *gotStats.StartMillis)
-				}
-				if tt.args.transition == database.WorkspaceTransitionStop {
-					require.Nil(t, gotStats.StartMillis)
-					require.Nil(t, gotStats.DeleteMillis)
-					require.Equal(t, tt.want.buildTimeMs, *gotStats.StopMillis)
+				for transition, stats := range gotStats {
+					if transition == wantTransition {
+						require.Equal(t, tt.want.buildTimeMs, *stats.Median)
+					} else {
+						require.Empty(t, stats)
+					}
 				}
 			} else {
 				require.Never(t, func() bool {
 					stats := cache.TemplateBuildTimeStats(template.ID)
-					return !assert.Empty(t, stats)
+					requireBuildTimeStatsEmpty(t, stats)
+					return t.Failed()
 				}, testutil.WaitShort/2, testutil.IntervalMedium,
 					"BuildTimeStats populated",
 				)

diff --git a/codersdk/templates.go b/codersdk/templates.go
@@ -33,12 +33,12 @@ type Template struct {
 	CreatedByName              string                 `json:"created_by_name"`
 }
 
-type TemplateBuildTimeStats struct {
-	StartMillis  *int64 `json:"start_ms"`
-	StopMillis   *int64 `json:"stop_ms"`
-	DeleteMillis *int64 `json:"delete_ms"`
+type TransitionStats struct {
+	Median *int64
+	Stddev *int64
 }
 
+type TemplateBuildTimeStats map[WorkspaceTransition]TransitionStats
 type UpdateActiveTemplateVersion struct {
 	ID uuid.UUID `json:"id" validate:"required"`
 }

diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts
@@ -559,11 +559,10 @@ export interface TemplateACL {
 }
 
 // From codersdk/templates.go
-export interface TemplateBuildTimeStats {
-  readonly start_ms?: number
-  readonly stop_ms?: number
-  readonly delete_ms?: number
-}
+export type TemplateBuildTimeStats = Record<
+  WorkspaceTransition,
+  TransitionStats
+>
 
 // From codersdk/templates.go
 export interface TemplateDAUsResponse {
@@ -599,6 +598,12 @@ export interface TemplateVersionsByTemplateRequest extends Pagination {
   readonly template_id: string
 }
 
+// From codersdk/templates.go
+export interface TransitionStats {
+  readonly Median?: number
+  readonly Stddev?: number
+}
+
 // From codersdk/templates.go
 export interface UpdateActiveTemplateVersion {
   readonly id: string

diff --git a/site/src/components/Workspace/Workspace.tsx b/site/src/components/Workspace/Workspace.tsx
@@ -118,10 +118,10 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
     />
   )
 
-  let buildTimeEstimate: number | undefined = undefined
+  let transitionStats: TypesGen.TransitionStats | undefined = undefined
   let isTransitioning: boolean | undefined = undefined
   if (template !== undefined) {
-    ;[buildTimeEstimate, isTransitioning] = EstimateTransitionTime(
+    ;[transitionStats, isTransitioning] = EstimateTransitionTime(
       template,
       workspace,
     )
@@ -201,7 +201,7 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
         {isTransitioning !== undefined && isTransitioning && (
           <WorkspaceBuildProgress
             workspace={workspace}
-            buildEstimate={buildTimeEstimate}
+            transitionStats={transitionStats}
           />
         )}
 

diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx
@@ -21,7 +21,7 @@ const Template: Story<WorkspaceBuildProgressProps> = (args) => (
 
 export const Starting = Template.bind({})
 Starting.args = {
-  buildEstimate: 10000,
+  transitionStats: 10000,
   workspace: {
     ...MockStartingWorkspace,
     latest_build: {
@@ -39,11 +39,11 @@ Starting.args = {
 export const StartingUnknown = Template.bind({})
 StartingUnknown.args = {
   ...Starting.args,
-  buildEstimate: undefined,
+  transitionStats: undefined,
 }
 
 export const StartingPassedEstimate = Template.bind({})
 StartingPassedEstimate.args = {
   ...Starting.args,
-  buildEstimate: 1000,
+  transitionStats: 1000,
 }
diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx
@@ -1,6 +1,6 @@
 import LinearProgress from "@material-ui/core/LinearProgress"
 import makeStyles from "@material-ui/core/styles/makeStyles"
-import { Template, Workspace } from "api/typesGenerated"
+import { TransitionStats, Template, Workspace, WorkspaceTransition } from "api/typesGenerated"
 import dayjs, { Dayjs } from "dayjs"
 import { FC, useEffect, useState } from "react"
 import { MONOSPACE_FONT_FAMILY } from "theme/constants"
@@ -30,31 +30,27 @@ const estimateFinish = (
 
 export interface WorkspaceBuildProgressProps {
   workspace: Workspace
-  buildEstimate?: number
+  transitionStats?: TransitionStats
 }
 
 // EstimateTransitionTime gets the build estimate for the workspace,
 // if it is in a transition state.
 export const EstimateTransitionTime = (
   template: Template,
   workspace: Workspace,
-): [number | undefined, boolean] => {
-  switch (workspace.latest_build.status) {
-    case "starting":
-      return [template.build_time_stats.start_ms, true]
-    case "stopping":
-      return [template.build_time_stats.stop_ms, true]
-    case "deleting":
-      return [template.build_time_stats.delete_ms, true]
-    default:
-      // Not in a transition state
-      return [undefined, false]
+): [TransitionStats | undefined, boolean] => {
+  const transition = workspace.latest_build.status
+
+  if (!["starting", "stopping", "deleting"].includes(transition)) {
+    return [undefined, false]
   }
+
+  return [template.build_time_stats[transition as WorkspaceTransition], true]
 }
 
 export const WorkspaceBuildProgress: FC<WorkspaceBuildProgressProps> = ({
   workspace,
-  buildEstimate,
+  transitionStats: transitionStats,
 }) => {
   const styles = useStyles()
   const job = workspace.latest_build.job
@@ -65,15 +61,15 @@ export const WorkspaceBuildProgress: FC<WorkspaceBuildProgressProps> = ({
   // stutter in all cases.
   useEffect(() => {
     const updateProgress = () => {
-      if (job.status !== "running" || buildEstimate === undefined) {
+      if (job.status !== "running" || transitionStats === undefined) {
         setProgressValue(undefined)
         return
       }
-      const est = estimateFinish(dayjs(job.started_at), buildEstimate)[0]
+      const est = estimateFinish(dayjs(job.started_at), transitionStats)[0]
       setProgressValue(est)
     }
     setTimeout(updateProgress, 5)
-  }, [progressValue, job, buildEstimate])
+  }, [progressValue, job, transitionStats])
 
   return (
     <div className={styles.stack}>
@@ -86,7 +82,7 @@ export const WorkspaceBuildProgress: FC<WorkspaceBuildProgressProps> = ({
           // perceives the bar jumping from 100% to 0%.
           progressValue !== undefined &&
           progressValue < 100 &&
-          buildEstimate !== undefined
+          transitionStats !== undefined
             ? "determinate"
             : "indeterminate"
         }
@@ -101,8 +97,8 @@ export const WorkspaceBuildProgress: FC<WorkspaceBuildProgressProps> = ({
           {(() => {
             if (job.status !== "running") {
               return ""
-            } else if (buildEstimate !== undefined) {
-              return estimateFinish(dayjs(job.started_at), buildEstimate)[1]
+            } else if (transitionStats !== undefined) {
+              return estimateFinish(dayjs(job.started_at), transitionStats)[1]
             } else {
               return "Unknown ETA"
             }