Review comments & CI appeasement

Signed-off-by: Danny Kopping <danny@coder.com>
coder · dannykopping · Mar 13, 2024 · Mar 7, 2024 · Mar 8, 2024 · Mar 8, 2024
commit 97f2bd1d72a53bec076149dea37211aed04515e9
diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -119,14 +119,14 @@ INTROSPECTION / LOGGING OPTIONS:
       --log-stackdriver string, $CODER_LOGGING_STACKDRIVER
           Output Stackdriver compatible logs to a given file.
 
-INTROSPECTION / PROMETHEUS OPTIONS:
+INTROSPECTION / PROMETHEUS OPTIONS: 
       --prometheus-address host:port, $CODER_PROMETHEUS_ADDRESS (default: 127.0.0.1:2112)
           The bind address to serve prometheus metrics.
 
-      --prometheus-aggregate-agent-stats-by string-array, $CODER_PROMETHEUS_AGGREGATE_AGENT_STATS_BY
+      --prometheus-aggregate-agent-stats-by string-array, $CODER_PROMETHEUS_AGGREGATE_AGENT_STATS_BY (default: agent_name,template_name,username,workspace_name)
           When collecting agent stats, aggregate metrics by a given set of
           comma-separated labels to reduce cardinality. Accepted values are
-          template_name, agent_name, username, workspace_name.
+          agent_name, template_name, username, workspace_name.
 
       --prometheus-collect-agent-stats bool, $CODER_PROMETHEUS_COLLECT_AGENT_STATS
           Collect agent stats (may increase charges for metrics storage).

diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden
@@ -189,10 +189,14 @@ introspection:
     # (default: <unset>, type: bool)
     collect_agent_stats: false
     # When collecting agent stats, aggregate metrics by a given set of comma-separated
-    # labels to reduce cardinality. Accepted values are template_name, agent_name,
+    # labels to reduce cardinality. Accepted values are agent_name, template_name,
     # username, workspace_name.
-    # (default: <unset>, type: string-array)
-    aggregate_agent_stats_by: []
+    # (default: agent_name,template_name,username,workspace_name, type: string-array)
+    aggregate_agent_stats_by:
+      - agent_name
+      - template_name
+      - username
+      - workspace_name
     # Collect database metrics (may increase charges for metrics storage).
     # (default: false, type: bool)
     collect_db_metrics: false

diff --git a/coderd/agentmetrics/labels.go b/coderd/agentmetrics/labels.go
@@ -1,8 +1,13 @@
 package agentmetrics
 
 const (
-	TemplateNameLabel  = "template_name"
-	AgentNameLabel     = "agent_name"
-	UsernameLabel      = "username"
-	WorkspaceNameLabel = "workspace_name"
+	LabelAgentName     = "agent_name"
+	LabelUsername      = "username"
+	LabelTemplateName  = "template_name"
+	LabelWorkspaceName = "workspace_name"
+)
+
+var (
+	LabelAll        = []string{LabelAgentName, LabelTemplateName, LabelUsername, LabelWorkspaceName}
+	LabelAgentStats = []string{LabelAgentName, LabelUsername, LabelWorkspaceName}
 )
diff --git a/coderd/prometheusmetrics/aggregator.go b/coderd/prometheusmetrics/aggregator.go
@@ -114,7 +114,7 @@ func (am *annotatedMetric) asPrometheus() (prometheus.Metric, error) {
 		extraLabels     = am.Labels
 	)
 
-	for _, label := range am.aggregateByLabels {
+	for _, label := range baseLabelNames {
 		val, err := am.getFieldByLabel(label)
 		if err != nil {
 			return nil, err
@@ -147,13 +147,13 @@ func (am *annotatedMetric) asPrometheus() (prometheus.Metric, error) {
 func (am *annotatedMetric) getFieldByLabel(label string) (string, error) {
 	var labelVal string
 	switch label {
-	case agentmetrics.WorkspaceNameLabel:
+	case agentmetrics.LabelWorkspaceName:
 		labelVal = am.workspaceName
-	case agentmetrics.TemplateNameLabel:
+	case agentmetrics.LabelTemplateName:
 		labelVal = am.templateName
-	case agentmetrics.AgentNameLabel:
+	case agentmetrics.LabelAgentName:
 		labelVal = am.agentName
-	case agentmetrics.UsernameLabel:
+	case agentmetrics.LabelUsername:
 		labelVal = am.username
 	default:
 		return "", xerrors.Errorf("unexpected label: %q", label)
@@ -162,7 +162,7 @@ func (am *annotatedMetric) getFieldByLabel(label string) (string, error) {
 	return labelVal, nil
 }
 
-func (am *annotatedMetric) clone() annotatedMetric {
+func (am *annotatedMetric) shallowCopy() annotatedMetric {
 	stats := &agentproto.Stats_Metric{
 		Name:   am.Name,
 		Type:   am.Type,
@@ -273,7 +273,7 @@ func (a *labelAggregator) aggregate(am annotatedMetric, labels []string) error {
 	metric, found := a.metrics[key]
 	if !found {
 		// Take a copy of the given annotatedMetric because it may be manipulated later and contains pointers.
-		metric = am.clone()
+		metric = am.shallowCopy()
 	}
 
 	// Store the metric.
@@ -337,8 +337,9 @@ func (ma *MetricsAggregator) Run(ctx context.Context) func() {
 
 				// If custom aggregation labels have not been chosen, generate Prometheus metrics without any pre-aggregation.
 				// This results in higher cardinality, but may be desirable in larger deployments.
+				//
 				// Default behavior.
-				if len(ma.aggregateByLabels) == 0 {
+				if len(ma.aggregateByLabels) == len(agentmetrics.LabelAll) {
 					for _, m := range ma.store {
 						// Aggregate by all available metrics.
 						m.aggregateByLabels = defaultAgentMetricsLabels
@@ -402,7 +403,7 @@ func (ma *MetricsAggregator) Run(ctx context.Context) func() {
 func (*MetricsAggregator) Describe(_ chan<- *prometheus.Desc) {
 }
 
-var defaultAgentMetricsLabels = []string{agentmetrics.UsernameLabel, agentmetrics.WorkspaceNameLabel, agentmetrics.AgentNameLabel, agentmetrics.TemplateNameLabel}
+var defaultAgentMetricsLabels = []string{agentmetrics.LabelUsername, agentmetrics.LabelWorkspaceName, agentmetrics.LabelAgentName, agentmetrics.LabelTemplateName}
 
 // AgentMetricLabels are the labels used to decorate an agent's metrics.
 // This list should match the list of labels in agentMetricsLabels.