From e6d056559b37eb5bcb0ace6754379f54f7d131dc Mon Sep 17 00:00:00 2001
From: Chris Grindstaff <chris@gstaff.org>
Date: Tue, 24 Sep 2024 14:28:37 -0400
Subject: [PATCH] perf: reduce allocations when serving Prometheus metrics

---
 cmd/exporters/prometheus/httpd.go           | 62 +++++++++++----------
 cmd/exporters/prometheus/prometheus_test.go | 18 ++----
 2 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/cmd/exporters/prometheus/httpd.go b/cmd/exporters/prometheus/httpd.go
index 9aa0773e2..c0fc61e44 100644
--- a/cmd/exporters/prometheus/httpd.go
+++ b/cmd/exporters/prometheus/httpd.go
@@ -129,21 +129,25 @@ func (p *Prometheus) ServeMetrics(w http.ResponseWriter, r *http.Request) {
 	}
 
 	p.cache.Lock()
+	// Count the number of metrics so we can pre-allocate the slice to avoid reallocations
 	for _, metrics := range p.cache.Get() {
-		data = append(data, metrics...)
 		count += len(metrics)
 	}
+
+	data = make([][]byte, 0, count)
+	tagsSeen := make(map[string]bool)
+
+	for _, metrics := range p.cache.Get() {
+		data = addMetricsToSlice(data, metrics, tagsSeen, p.addMetaTags)
+	}
 	p.cache.Unlock()
 
 	// serve our own metadata
 	// notice that some values are always taken from previous session
 	md, _ := p.render(p.Metadata)
-	data = append(data, md...)
-	count += len(md)
+	data = addMetricsToSlice(data, md, tagsSeen, p.addMetaTags)
 
-	if p.addMetaTags {
-		data = filterMetaTags(data)
-	}
+	count += len(md)
 
 	w.WriteHeader(http.StatusOK)
 	w.Header().Set("Content-Type", "text/plain")
@@ -169,35 +173,37 @@ func (p *Prometheus) ServeMetrics(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// filterMetaTags removes duplicate TYPE/HELP tags in the metrics
-// Note: this is a workaround, normally Render() will only add
-// one TYPE/HELP for each metric type, however since some metric
-// types (e.g. metadata_collector_metrics) are submitted from multiple
-// collectors, we end up with duplicates in the final batch delivered
-// over HTTP.
-func filterMetaTags(metrics [][]byte) [][]byte {
-
-	filtered := make([][]byte, 0)
-
-	metricsWithTags := make(map[string]bool)
-
-	for i, m := range metrics {
-		if bytes.HasPrefix(m, []byte("# ")) {
-			if fields := strings.Fields(string(m)); len(fields) > 3 {
-				name := fields[2]
-				if !metricsWithTags[name] {
-					metricsWithTags[name] = true
-					filtered = append(filtered, m)
+// addMetricsToSlice adds metrics to a slice, skipping duplicates. Normally
+// Render() only adds one TYPE/HELP for each metric type. Some metric types
+// (e.g., metadata_collector_metrics) are submitted from multiple collectors.
+// That causes duplicates that are removed in this function. The seen map is
+// used to keep track of which metrics have been added. The metrics slice is
+// expected to be in the format: # HELP metric_name help text # TYPE metric_name
+// type metric_name{tag="value"} value
+func addMetricsToSlice(data [][]byte, metrics [][]byte, seen map[string]bool, addMetaTags bool) [][]byte {
+
+	if !addMetaTags {
+		return append(data, metrics...)
+	}
+
+	for i, metric := range metrics {
+		if bytes.HasPrefix(metric, []byte("# ")) {
+			if fields := bytes.Fields(metric); len(fields) > 3 {
+				name := string(fields[2])
+				if !seen[name] {
+					seen[name] = true
+					data = append(data, metric)
 					if i+1 < len(metrics) {
-						filtered = append(filtered, metrics[i+1])
+						data = append(data, metrics[i+1])
 					}
 				}
 			}
 		} else {
-			filtered = append(filtered, m)
+			data = append(data, metric)
 		}
 	}
-	return filtered
+
+	return data
 }
 
 // ServeInfo provides a human-friendly overview of metric types and source collectors
diff --git a/cmd/exporters/prometheus/prometheus_test.go b/cmd/exporters/prometheus/prometheus_test.go
index 34e67b9ed..0456f930d 100644
--- a/cmd/exporters/prometheus/prometheus_test.go
+++ b/cmd/exporters/prometheus/prometheus_test.go
@@ -5,7 +5,6 @@
 package prometheus
 
 import (
-	"bytes"
 	"github.com/google/go-cmp/cmp"
 	"github.com/netapp/harvest/v2/cmd/poller/exporter"
 	"github.com/netapp/harvest/v2/cmd/poller/options"
@@ -40,20 +39,13 @@ func TestFilterMetaTags(t *testing.T) {
 		[]byte(`some_other_metric{node="node_3"} 0.0`),
 	}
 
-	output := filterMetaTags(example)
+	seen := make(map[string]bool)
+	data := addMetricsToSlice(nil, example, seen, true)
 
-	if len(output) != len(expected) {
-		t.Fatalf("filtered data should have %d, but got %d lines", len(expected), len(output))
+	diff := cmp.Diff(data, expected)
+	if diff != "" {
+		t.Errorf("Mismatch (-got +want):\n%s", diff)
 	}
-
-	// output should have exact same lines in the same order
-	for i := range output {
-		if !bytes.Equal(output[i], expected[i]) {
-			t.Fatalf("line:%d - output = [%s], expected = [%s]", i, string(output[i]), string(expected[i]))
-		}
-	}
-
-	t.Log("OK - output is exactly what is expected")
 }
 
 func TestEscape(t *testing.T) {