divide long functions, replace strings.builder with bytes.buffer. (#13)

Co-authored-by: Chris Clayton <chris.clayton@contino.io>

monitoring/helpers.go

@@ -1,129 +1,193 @@
 package libpack_monitoring
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"sort"
 	"strings"
+	"sync"
 	"unicode"
 
 	libpack_config "github.com/lukaszraczylo/graphql-monitoring-proxy/config"
 )
 
-func (ms *MetricsSetup) get_metrics_name(name string, labels map[string]string) (complete_name string) {
+// Cache for sorted label keys to avoid repeated sorting
+var sortedLabelKeysCache = struct {
+	sync.RWMutex
+	m map[string][]string
+}{m: make(map[string][]string)}
+
+func (ms *MetricsSetup) get_metrics_name(name string, labels map[string]string) string {
 	const unknownPodName = "unknown"
-	var sb strings.Builder
+	var buf bytes.Buffer
 
 	// Prepare default labels without initializing a new map
-	podName := unknownPodName
-	if hn, err := os.Hostname(); err == nil {
-		podName = hn
-	}
+	podName := getPodName()
 	if labels == nil {
-		labels = map[string]string{
-			"microservice": libpack_config.PKG_NAME,
-			"pod":          podName,
-		}
+		labels = defaultLabels(podName)
 	} else {
-		if _, exists := labels["microservice"]; !exists {
-			labels["microservice"] = libpack_config.PKG_NAME
-		}
-		if _, exists := labels["pod"]; !exists {
-			labels["pod"] = podName
-		}
+		ensureDefaultLabels(&labels, podName)
 	}
 
 	// Prefix handling
 	if ms.metrics_prefix != "" {
-		sb.WriteString(ms.metrics_prefix)
-		sb.WriteString("_")
+		buf.WriteString(ms.metrics_prefix)
+		buf.WriteString("_")
 	}
-	sb.WriteString(name)
+	buf.WriteString(name)
 
 	// Append labels if any
 	if len(labels) > 0 {
-		sb.WriteString("{")
+		buf.WriteString("{")
+		appendSortedLabels(&buf, labels)
+		buf.WriteString("}")
+	}
 
-		keys := make([]string, 0, len(labels))
+	return buf.String()
+}
+
+func getPodName() string {
+	const unknownPodName = "unknown"
+	if hn, err := os.Hostname(); err == nil {
+		return hn
+	}
+	return unknownPodName
+}
+
+func defaultLabels(podName string) map[string]string {
+	return map[string]string{
+		"microservice": libpack_config.PKG_NAME,
+		"pod":          podName,
+	}
+}
+
+func ensureDefaultLabels(labels *map[string]string, podName string) {
+	if *labels == nil {
+		*labels = make(map[string]string)
+	}
+	if _, exists := (*labels)["microservice"]; !exists {
+		(*labels)["microservice"] = libpack_config.PKG_NAME
+	}
+	if _, exists := (*labels)["pod"]; !exists {
+		(*labels)["pod"] = podName
+	}
+}
+
+func appendSortedLabels(buf *bytes.Buffer, labels map[string]string) {
+	keys := getSortedKeys(labels)
+	for i, k := range keys {
+		if i > 0 {
+			buf.WriteString(",")
+		}
+		buf.WriteString(k)
+		buf.WriteString("=\"")
+		buf.WriteString(labels[k])
+		buf.WriteString("\"")
+	}
+}
+
+func getSortedKeys(labels map[string]string) []string {
+	labelsKey := labelsToString(labels)
+
+	sortedLabelKeysCache.RLock()
+	keys, exists := sortedLabelKeysCache.m[labelsKey]
+	sortedLabelKeysCache.RUnlock()
+
+	if !exists {
+		keys = make([]string, 0, len(labels))
 		for k := range labels {
 			keys = append(keys, k)
 		}
 		sort.Strings(keys)
 
-		for i, k := range keys {
-			if i > 0 {
-				sb.WriteString(",")
-			}
-			sb.WriteString(k)
-			sb.WriteString("=\"")
-			sb.WriteString(labels[k])
-			sb.WriteString("\"")
-		}
-		sb.WriteString("}")
+		sortedLabelKeysCache.Lock()
+		sortedLabelKeysCache.m[labelsKey] = keys
+		sortedLabelKeysCache.Unlock()
 	}
 
+	return keys
+}
+
+func labelsToString(labels map[string]string) string {
+	var sb strings.Builder
+	for k, v := range labels {
+		sb.WriteString(k)
+		sb.WriteString("=")
+		sb.WriteString(v)
+		sb.WriteString(";")
+	}
 	return sb.String()
 }
 
 // validate_metrics_name validates the name of the metric to adhere to the Prometheus naming conventions
 // https://prometheus.io/docs/practices/naming/
 func validate_metrics_name(name string) error {
-	var sb strings.Builder // Use strings.Builder for efficient string concatenation
+	cleanedName := clean_metric_name(name)
 
-	// Track if the last character was an underscore to avoid duplicate underscores
-	lastWasUnderscore := false
-
-	for _, r := range name {
-		// Convert spaces to underscores and skip non-alphanumeric characters except underscores
-		if r == ' ' || (unicode.IsLetter(r) || unicode.IsDigit(r) || r == '_') {
-			if r == ' ' || r == '_' {
-				if lastWasUnderscore {
-					continue // Skip if the previous character was also an underscore
-				}
-				r = '_' // Convert spaces to underscores
-				lastWasUnderscore = true
-			} else {
-				lastWasUnderscore = false
-			}
-			sb.WriteRune(r) // Add valid characters to the builder
-		}
-	}
 	// Trim leading and trailing underscores
-	name_new := strings.Trim(sb.String(), "_")
+	finalName := strings.Trim(cleanedName, "_")
 
 	// Check if the processed name matches the original input
-	if name_new != name {
-		return fmt.Errorf("Invalid metric name: %s, expected %s", name, name_new)
+	if finalName != name {
+		return fmt.Errorf("Invalid metric name: %s, expected %s", name, finalName)
 	}
 	return nil
 }
 
-func compile_metrics_with_labels(name string, labels map[string]string) string {
-	var totalLength int
-	totalLength += len(name)
-	for k, v := range labels {
-		totalLength += len(k) + len(v) + 2
+// clean_metric_name processes the metric name according to Prometheus naming conventions
+func clean_metric_name(name string) string {
+	var buf bytes.Buffer
+	lastWasUnderscore := false
+
+	for _, r := range name {
+		if is_allowed_rune(r) {
+			if is_special_rune(r) {
+				if lastWasUnderscore {
+					continue // Skip if the previous character was also an underscore
+				}
+				r = '_' // Convert spaces and special characters to underscores
+				lastWasUnderscore = true
+			} else {
+				lastWasUnderscore = false
+			}
+			buf.WriteRune(r)
+		} else if !lastWasUnderscore {
+			buf.WriteRune('_')
+			lastWasUnderscore = true
+		}
 	}
 
-	var sb strings.Builder
-	sb.Grow(totalLength + 1)
+	// Remove trailing underscore
+	result := buf.String()
+	return strings.Trim(result, "_")
+}
 
-	sb.WriteString(name)
+// is_allowed_rune checks if the rune is allowed in the metric name
+func is_allowed_rune(r rune) bool {
+	return unicode.IsLetter(r) || unicode.IsDigit(r) || r == ' ' || r == '_'
+}
+
+// is_special_rune checks if the rune is a space or an underscore
+func is_special_rune(r rune) bool {
+	return r == ' ' || r == '_'
+}
+
+func compile_metrics_with_labels(name string, labels map[string]string) string {
+	var buf bytes.Buffer
+
+	buf.WriteString(name)
 
 	// Collect keys and sort them
-	keys := make([]string, 0, len(labels))
-	for k := range labels {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
+	keys := getSortedKeys(labels)
 
-	// Append sorted key-value pairs to the builder
+	// Append sorted key-value pairs to the buffer
 	for _, k := range keys {
-		sb.WriteString("_")
-		sb.WriteString(k)
-		sb.WriteString("_")
-		sb.WriteString(labels[k])
+		buf.WriteString("_")
+		buf.WriteString(k)
+		buf.WriteString("_")
+		buf.WriteString(labels[k])
 	}
 
-	return sb.String()
+	return buf.String()
 }

monitoring/helpers_test.go

@@ -1,7 +1,6 @@
 package libpack_monitoring
 
 import (
-	"os"
 	"testing"
 
 	libpack_config "github.com/lukaszraczylo/graphql-monitoring-proxy/config"
@@ -134,10 +133,96 @@ func TestValidateMetricsName(t *testing.T) {
 	}
 }
 
-func getPodName() string {
-	podName, err := os.Hostname()
-	if err != nil {
-		return "unknown"
+func TestCleanMetricName(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{"valid metric name", "valid_metric_name"},
+		{"valid@metric#name!", "valid_metric_name"},
+		{"__valid__metric__name__", "valid_metric_name"},
+		{" valid metric name ", "valid_metric_name"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			assert.Equal(t, tt.expected, clean_metric_name(tt.input))
+		})
+	}
+}
+
+func TestDefaultLabels(t *testing.T) {
+	podName := "test-pod"
+	libpack_config.PKG_NAME = "example_microservice"
+	expected := map[string]string{
+		"microservice": "example_microservice",
+		"pod":          podName,
+	}
+
+	assert.Equal(t, expected, defaultLabels(podName))
+}
+
+func TestEnsureDefaultLabels(t *testing.T) {
+	podName := "test-pod"
+	libpack_config.PKG_NAME = "example_microservice"
+
+	tests := []struct {
+		name           string
+		inputLabels    map[string]string
+		expectedLabels map[string]string
+	}{
+		{
+			name:           "Nil labels",
+			inputLabels:    nil,
+			expectedLabels: map[string]string{"microservice": "example_microservice", "pod": podName},
+		},
+		{
+			name:           "Empty labels",
+			inputLabels:    map[string]string{},
+			expectedLabels: map[string]string{"microservice": "example_microservice", "pod": podName},
+		},
+		{
+			name:           "Partial labels",
+			inputLabels:    map[string]string{"microservice": "test_service"},
+			expectedLabels: map[string]string{"microservice": "test_service", "pod": podName},
+		},
+		{
+			name:           "Complete labels",
+			inputLabels:    map[string]string{"microservice": "test_service", "pod": "custom_pod"},
+			expectedLabels: map[string]string{"microservice": "test_service", "pod": "custom_pod"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ensureDefaultLabels(&tt.inputLabels, podName)
+			assert.Equal(t, tt.expectedLabels, tt.inputLabels)
+		})
+	}
+}
+
+func TestLabelsToString(t *testing.T) {
+	tests := []struct {
+		labels   map[string]string
+		expected string
+	}{
+		{
+			labels:   map[string]string{"key1": "value1", "key2": "value2"},
+			expected: "key1=value1;key2=value2;",
+		},
+		{
+			labels:   map[string]string{"a": "1", "b": "2"},
+			expected: "a=1;b=2;",
+		},
+		{
+			labels:   map[string]string{},
+			expected: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.expected, func(t *testing.T) {
+			assert.Equal(t, tt.expected, labelsToString(tt.labels))
+		})
 	}
-	return podName
 }