A bunch of minor touch ups

coder · ammario · Sep 1, 2022 · Aug 29, 2022 · Aug 29, 2022 · Sep 1, 2022
commit b14a077e4787fa6432099a07ff4f7533c9cf388c
diff --git a/agent/stats.go b/agent/stats.go
@@ -33,12 +33,11 @@ var _ net.Conn = new(StatsConn)
 
 // Stats records the Agent's network connection statistics for use in
 // user-facing metrics and debugging.
+// Each member value must be written and read with atomic.
 type Stats struct {
 	NumConns int64 `json:"num_comms"`
-	// RxBytes must be read with atomic.
-	RxBytes int64 `json:"rx_bytes"`
-	// TxBytes must be read with atomic.
-	TxBytes int64 `json:"tx_bytes"`
+	RxBytes  int64 `json:"rx_bytes"`
+	TxBytes  int64 `json:"tx_bytes"`
 }
 
 func (s *Stats) Copy() *Stats {

diff --git a/cli/server.go b/cli/server.go
@@ -841,6 +841,9 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command {
 	_ = root.Flags().MarkHidden("spooky")
 	cliflag.BoolVarP(root.Flags(), &verbose, "verbose", "v", "CODER_VERBOSE", false, "Enables verbose logging.")
 
+	// These metrics flags are for manually testing the metric system.
+	// The defaults should be acceptable for any Coder deployment of any
+	// reasonable size.
 	cliflag.DurationVarP(root.Flags(), &metricsCacheRefreshInterval, "metrics-cache-refresh-interval", "", "CODER_METRICS_CACHE_REFRESH_INTERVAL", time.Hour, "How frequently metrics are refreshed")
 	_ = root.Flags().MarkHidden("metrics-cache-refresh-interval")
 	cliflag.DurationVarP(root.Flags(), &agentStatRefreshInterval, "agent-stats-refresh-interval", "", "CODER_AGENT_STATS_REFRESH_INTERVAL", time.Minute*10, "How frequently agent stats are recorded")

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -78,7 +78,6 @@ type Options struct {
 	TailnetCoordinator *tailnet.Coordinator
 	DERPMap            *tailcfg.DERPMap
 
-	// Metrics related intervals.
 	MetricsCacheRefreshInterval time.Duration
 	AgentStatsRefreshInterval   time.Duration
 }

diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go
@@ -15,10 +15,11 @@ import (
 	"github.com/coder/retry"
 )
 
-// Cache holds the DAU cache.
-// The aggregation queries responsible for these values
-// can take up to a minute on large deployments, but the cache has near zero
-// effect on most deployments.
+// Cache holds the template DAU cache.
+// The aggregation queries responsible for these values can take up to a minute
+// on large deployments. Even in small deployments, aggregation queries can
+// take a few hundred milliseconds, which would ruin page load times and
+// database performance if in the hot path.
 type Cache struct {
 	database database.Store
 	log      slog.Logger
@@ -153,8 +154,9 @@ func (c *Cache) Close() error {
 	return nil
 }
 
-// DAUs returns the DAUs or nil if they aren't ready yet.
-func (c *Cache) DAUs(id uuid.UUID) codersdk.TemplateDAUsResponse {
+// TemplateDAUs returns an empty response if the template doesn't have users
+// or is loading for the first time.
+func (c *Cache) TemplateDAUs(id uuid.UUID) codersdk.TemplateDAUsResponse {
 	m := c.templateDAUResponses.Load()
 	if m == nil {
 		// Data loading.

diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go
@@ -175,7 +175,7 @@ func TestCache(t *testing.T) {
 			var got codersdk.TemplateDAUsResponse
 
 			require.Eventuallyf(t, func() bool {
-				got = cache.DAUs(templateID)
+				got = cache.TemplateDAUs(templateID)
 				return reflect.DeepEqual(got.Entries, tt.want)
 			}, testutil.WaitShort, testutil.IntervalFast,
 				"GetDAUs() = %v, want %v", got, tt.want,

diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go
@@ -124,10 +124,6 @@ var (
 	ResourceLicense = Object{
 		Type: "license",
 	}
-
-	ResourceMetrics = Object{
-		Type: "metrics",
-	}
 )
 
 // Object is used to create objects for authz checks when you have none in

diff --git a/coderd/templates.go b/coderd/templates.go
@@ -524,7 +524,7 @@ func (api *API) templateDAUs(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	resp := api.metricsCache.DAUs(template.ID)
+	resp := api.metricsCache.TemplateDAUs(template.ID)
 	if resp.Entries == nil {
 		resp.Entries = []codersdk.DAUEntry{}
 	}

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -821,6 +821,9 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques
 		}
 
 		// Avoid inserting duplicate rows to preserve DB space.
+		// We will see duplicate reports when on idle connections
+		// (e.g. web terminal left open) or when there are no connections at
+		// all.
 		var insert = !reflect.DeepEqual(lastReport, rep)
 
 		api.Logger.Debug(ctx, "read stats report",

diff --git a/codersdk/client.go b/codersdk/client.go
@@ -219,8 +219,8 @@ func (e *Error) Error() string {
 	return builder.String()
 }
 
-type CloseFunc func() error
+type closeFunc func() error
 
-func (c CloseFunc) Close() error {
+func (c closeFunc) Close() error {
 	return c()
 }
diff --git a/codersdk/metrics.go b/codersdk/metrics.go
diff --git a/codersdk/templates.go b/codersdk/templates.go
@@ -133,3 +133,44 @@ func (c *Client) TemplateVersionByName(ctx context.Context, template uuid.UUID,
 	var templateVersion TemplateVersion
 	return templateVersion, json.NewDecoder(res.Body).Decode(&templateVersion)
 }
+
+type DAUEntry struct {
+	Date time.Time `json:"date"`
+	DAUs int       `json:"daus"`
+}
+
+type TemplateDAUsResponse struct {
+	Entries []DAUEntry `json:"entries"`
+}
+
+func (c *Client) TemplateDAUs(ctx context.Context, templateID uuid.UUID) (*TemplateDAUsResponse, error) {
+	res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/daus", templateID), nil)
+	if err != nil {
+		return nil, xerrors.Errorf("execute request: %w", err)
+	}
+	defer res.Body.Close()
+
+	if res.StatusCode != http.StatusOK {
+		return nil, readBodyAsError(res)
+	}
+
+	var resp TemplateDAUsResponse
+	return &resp, json.NewDecoder(res.Body).Decode(&resp)
+}
+
+// @typescript-ignore AgentStatsReportRequest
+
+// AgentStatsReportRequest is a WebSocket request by coderd
+// to the agent for stats.
+type AgentStatsReportRequest struct {
+}
+
+// AgentStatsReportResponse is returned for each report
+// request by the agent.
+type AgentStatsReportResponse struct {
+	NumConns int64 `json:"num_comms"`
+	// RxBytes is the number of received bytes.
+	RxBytes int64 `json:"rx_bytes"`
+	// TxBytes is the number of received bytes.
+	TxBytes int64 `json:"tx_bytes"`
+}
diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go
@@ -19,6 +19,7 @@ import (
 	"golang.org/x/net/proxy"
 	"golang.org/x/xerrors"
 	"nhooyr.io/websocket"
+	"nhooyr.io/websocket/wsjson"
 	"tailscale.com/tailcfg"
 
 	"cdr.dev/slog"
@@ -528,3 +529,87 @@ func (c *Client) turnProxyDialer(ctx context.Context, httpClient *http.Client, p
 		return websocket.NetConn(ctx, conn, websocket.MessageBinary), nil
 	})
 }
+
+// AgentReportStats begins a stat streaming connection with the Coder server.
+// It is resilient to network failures and intermittent coderd issues.
+func (c *Client) AgentReportStats(
+	ctx context.Context,
+	log slog.Logger,
+	stats func() *agent.Stats,
+) (io.Closer, error) {
+	serverURL, err := c.URL.Parse("/api/v2/workspaceagents/me/report-stats")
+	if err != nil {
+		return nil, xerrors.Errorf("parse url: %w", err)
+	}
+
+	jar, err := cookiejar.New(nil)
+	if err != nil {
+		return nil, xerrors.Errorf("create cookie jar: %w", err)
+	}
+
+	jar.SetCookies(serverURL, []*http.Cookie{{
+		Name:  SessionTokenKey,
+		Value: c.SessionToken,
+	}})
+
+	httpClient := &http.Client{
+		Jar: jar,
+	}
+
+	doneCh := make(chan struct{})
+	ctx, cancel := context.WithCancel(ctx)
+
+	go func() {
+		defer close(doneCh)
+
+		// If the agent connection succeeds for a while, then fails, then succeeds
+		// for a while (etc.) the retry may hit the maximum. This is a normal
+		// case for long-running agents that experience coderd upgrades, so
+		// we use a short maximum retry limit.
+		for r := retry.New(time.Second, time.Minute); r.Wait(ctx); {
+			err = func() error {
+				conn, res, err := websocket.Dial(ctx, serverURL.String(), &websocket.DialOptions{
+					HTTPClient: httpClient,
+					// Need to disable compression to avoid a data-race.
+					CompressionMode: websocket.CompressionDisabled,
+				})
+				if err != nil {
+					if res == nil {
+						return err
+					}
+					return readBodyAsError(res)
+				}
+
+				for {
+					var req AgentStatsReportRequest
+					err := wsjson.Read(ctx, conn, &req)
+					if err != nil {
+						return err
+					}
+
+					s := stats()
+
+					resp := AgentStatsReportResponse{
+						NumConns: s.NumConns,
+						RxBytes:  s.RxBytes,
+						TxBytes:  s.TxBytes,
+					}
+
+					err = wsjson.Write(ctx, conn, resp)
+					if err != nil {
+						return err
+					}
+				}
+			}()
+			if err != nil && ctx.Err() == nil {
+				log.Error(ctx, "report stats", slog.Error(err))
+			}
+		}
+	}()
+
+	return closeFunc(func() error {
+		cancel()
+		<-doneCh
+		return nil
+	}), nil
+}