fix: Use app slugs instead of the display name to report health (#4944)

All applications without display names were reporting broken health.

agent/apphealth.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"golang.org/x/xerrors"
+	"github.com/google/uuid"
 
 	"cdr.dev/slog"
 	"github.com/coder/coder/codersdk"
@@ -31,9 +32,9 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
 		}
 
 		hasHealthchecksEnabled := false
-		health := make(map[string]codersdk.WorkspaceAppHealth, 0)
+		health := make(map[uuid.UUID]codersdk.WorkspaceAppHealth, 0)
 		for _, app := range apps {
-			health[app.DisplayName] = app.Health
+			health[app.ID] = app.Health
 			if !hasHealthchecksEnabled && app.Health != codersdk.WorkspaceAppHealthDisabled {
 				hasHealthchecksEnabled = true
 			}
@@ -46,7 +47,7 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
 
 		// run a ticker for each app health check.
 		var mu sync.RWMutex
-		failures := make(map[string]int, 0)
+		failures := make(map[uuid.UUID]int, 0)
 		for _, nextApp := range apps {
 			if !shouldStartTicker(nextApp) {
 				continue
@@ -85,21 +86,21 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, apps []codersdk.Workspace
 					}()
 					if err != nil {
 						mu.Lock()
-						if failures[app.DisplayName] < int(app.Healthcheck.Threshold) {
+						if failures[app.ID] < int(app.Healthcheck.Threshold) {
 							// increment the failure count and keep status the same.
 							// we will change it when we hit the threshold.
-							failures[app.DisplayName]++
+							failures[app.ID]++
 						} else {
 							// set to unhealthy if we hit the failure threshold.
 							// we stop incrementing at the threshold to prevent the failure value from increasing forever.
-							health[app.DisplayName] = codersdk.WorkspaceAppHealthUnhealthy
+							health[app.ID] = codersdk.WorkspaceAppHealthUnhealthy
 						}
 						mu.Unlock()
 					} else {
 						mu.Lock()
 						// we only need one successful health check to be considered healthy.
-						health[app.DisplayName] = codersdk.WorkspaceAppHealthHealthy
+						health[app.ID] = codersdk.WorkspaceAppHealthHealthy
-						failures[app.DisplayName] = 0
+						failures[app.ID] = 0
 						mu.Unlock()
 					}
 
@@ -155,7 +156,7 @@ func shouldStartTicker(app codersdk.WorkspaceApp) bool {
 	return app.Healthcheck.URL != "" && app.Healthcheck.Interval > 0 && app.Healthcheck.Threshold > 0
 }
 
-func healthChanged(old map[string]codersdk.WorkspaceAppHealth, new map[string]codersdk.WorkspaceAppHealth) bool {
+func healthChanged(old map[uuid.UUID]codersdk.WorkspaceAppHealth, new map[uuid.UUID]codersdk.WorkspaceAppHealth) bool {
 	for name, newValue := range new {
 		oldValue, found := old[name]
 		if !found {
@@ -169,8 +170,8 @@ func healthChanged(old map[string]codersdk.WorkspaceAppHealth, new map[string]co
 	return false
 }
 
-func copyHealth(h1 map[string]codersdk.WorkspaceAppHealth) map[string]codersdk.WorkspaceAppHealth {
+func copyHealth(h1 map[uuid.UUID]codersdk.WorkspaceAppHealth) map[uuid.UUID]codersdk.WorkspaceAppHealth {
-	h2 := make(map[string]codersdk.WorkspaceAppHealth, 0)
+	h2 := make(map[uuid.UUID]codersdk.WorkspaceAppHealth, 0)
 	for k, v := range h1 {
 		h2[k] = v
 	}

agent/apphealth_test.go

@@ -27,12 +27,12 @@ func TestAppHealth(t *testing.T) {
 		defer cancel()
 		apps := []codersdk.WorkspaceApp{
 			{
-				DisplayName: "app1",
+				Slug:        "app1",
 				Healthcheck: codersdk.Healthcheck{},
 				Health:      codersdk.WorkspaceAppHealthDisabled,
 			},
 			{
-				DisplayName: "app2",
+				Slug: "app2",
 				Healthcheck: codersdk.Healthcheck{
 					// URL: We don't set the URL for this test because the setup will
 					// create a httptest server for us and set it for us.
@@ -69,7 +69,7 @@ func TestAppHealth(t *testing.T) {
 		defer cancel()
 		apps := []codersdk.WorkspaceApp{
 			{
-				DisplayName: "app2",
+				Slug: "app2",
 				Healthcheck: codersdk.Healthcheck{
 					// URL: We don't set the URL for this test because the setup will
 					// create a httptest server for us and set it for us.
@@ -102,7 +102,7 @@ func TestAppHealth(t *testing.T) {
 		defer cancel()
 		apps := []codersdk.WorkspaceApp{
 			{
-				DisplayName: "app2",
+				Slug: "app2",
 				Healthcheck: codersdk.Healthcheck{
 					// URL: We don't set the URL for this test because the setup will
 					// create a httptest server for us and set it for us.
@@ -137,7 +137,7 @@ func TestAppHealth(t *testing.T) {
 		defer cancel()
 		apps := []codersdk.WorkspaceApp{
 			{
-				DisplayName: "app2",
+				Slug: "app2",
 				Healthcheck: codersdk.Healthcheck{
 					// URL: We don't set the URL for this test because the setup will
 					// create a httptest server for us and set it for us.
@@ -185,9 +185,9 @@ func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.Workspa
 	}
 	postWorkspaceAgentAppHealth := func(_ context.Context, req codersdk.PostWorkspaceAppHealthsRequest) error {
 		mu.Lock()
-		for name, health := range req.Healths {
+		for id, health := range req.Healths {
 			for i, app := range apps {
-				if app.DisplayName != name {
+				if app.ID != id {
 					continue
 				}
 				app.Health = health

coderd/workspaceagents.go

@@ -913,10 +913,10 @@ func (api *API) postWorkspaceAppHealth(rw http.ResponseWriter, r *http.Request)
 	}
 
 	var newApps []database.WorkspaceApp
-	for name, newHealth := range req.Healths {
+	for id, newHealth := range req.Healths {
 		old := func() *database.WorkspaceApp {
 			for _, app := range apps {
-				if app.DisplayName == name {
+				if app.ID == id {
 					return &app
 				}
 			}
@@ -926,7 +926,7 @@ func (api *API) postWorkspaceAppHealth(rw http.ResponseWriter, r *http.Request)
 		if old == nil {
 			httpapi.Write(r.Context(), rw, http.StatusNotFound, codersdk.Response{
 				Message: "Error setting workspace app health",
-				Detail:  xerrors.Errorf("workspace app name %s not found", name).Error(),
+				Detail:  xerrors.Errorf("workspace app name %s not found", id).Error(),
 			})
 			return
 		}
@@ -934,7 +934,7 @@ func (api *API) postWorkspaceAppHealth(rw http.ResponseWriter, r *http.Request)
 		if old.HealthcheckUrl == "" {
 			httpapi.Write(r.Context(), rw, http.StatusNotFound, codersdk.Response{
 				Message: "Error setting workspace app health",
-				Detail:  xerrors.Errorf("health checking is disabled for workspace app %s", name).Error(),
+				Detail:  xerrors.Errorf("health checking is disabled for workspace app %s", id).Error(),
 			})
 			return
 		}

coderd/workspaceagents_test.go

@@ -555,9 +555,8 @@ func TestWorkspaceAgentListeningPorts(t *testing.T) {
 			// should not exist in the response.
 			_, appLPort := generateUnfilteredPort(t)
 			app := &proto.App{
-				Slug:        "test-app",
+				Slug: "test-app",
-				DisplayName: "test-app",
+				Url:  fmt.Sprintf("http://localhost:%d", appLPort),
-				Url:         fmt.Sprintf("http://localhost:%d", appLPort),
 			}
 
 			// Generate a filtered port that should not exist in the response.
@@ -624,11 +623,10 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
 	authToken := uuid.NewString()
 	apps := []*proto.App{
 		{
-			Slug:        "code-server",
+			Slug:    "code-server",
-			DisplayName: "code-server",
+			Command: "some-command",
-			Command:     "some-command",
+			Url:     "http://localhost:3000",
-			Url:         "http://localhost:3000",
+			Icon:    "/code.svg",
-			Icon:        "/code.svg",
 		},
 		{
 			Slug:        "code-server-2",
@@ -683,31 +681,24 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
 	// empty
 	err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{})
 	require.Error(t, err)
-	// invalid name
+	// healthcheck disabled
 	err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
-		Healths: map[string]codersdk.WorkspaceAppHealth{
+		Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
-			"bad-name": codersdk.WorkspaceAppHealthDisabled,
+			metadata.Apps[0].ID: codersdk.WorkspaceAppHealthInitializing,
-		},
-	})
-	require.Error(t, err)
-	// healcheck disabled
-	err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
-		Healths: map[string]codersdk.WorkspaceAppHealth{
-			"code-server": codersdk.WorkspaceAppHealthInitializing,
 		},
 	})
 	require.Error(t, err)
 	// invalid value
 	err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
-		Healths: map[string]codersdk.WorkspaceAppHealth{
+		Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
-			"code-server-2": codersdk.WorkspaceAppHealth("bad-value"),
+			metadata.Apps[1].ID: codersdk.WorkspaceAppHealth("bad-value"),
 		},
 	})
 	require.Error(t, err)
 	// update to healthy
 	err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
-		Healths: map[string]codersdk.WorkspaceAppHealth{
+		Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
-			"code-server-2": codersdk.WorkspaceAppHealthHealthy,
+			metadata.Apps[1].ID: codersdk.WorkspaceAppHealthHealthy,
 		},
 	})
 	require.NoError(t, err)
@@ -716,8 +707,8 @@ func TestWorkspaceAgentAppHealth(t *testing.T) {
 	require.EqualValues(t, codersdk.WorkspaceAppHealthHealthy, metadata.Apps[1].Health)
 	// update to unhealthy
 	err = agentClient.PostWorkspaceAgentAppHealth(ctx, codersdk.PostWorkspaceAppHealthsRequest{
-		Healths: map[string]codersdk.WorkspaceAppHealth{
+		Healths: map[uuid.UUID]codersdk.WorkspaceAppHealth{
-			"code-server-2": codersdk.WorkspaceAppHealthUnhealthy,
+			metadata.Apps[1].ID: codersdk.WorkspaceAppHealthUnhealthy,
 		},
 	})
 	require.NoError(t, err)

codersdk/workspaceapps.go

@@ -54,5 +54,5 @@ type Healthcheck struct {
 // @typescript-ignore PostWorkspaceAppHealthsRequest
 type PostWorkspaceAppHealthsRequest struct {
 	// Healths is a map of the workspace app name and the health of the app.
-	Healths map[string]WorkspaceAppHealth
+	Healths map[uuid.UUID]WorkspaceAppHealth
 }