From 06f3e2ce8e45be52390f13be12b0091c5bb06bbf Mon Sep 17 00:00:00 2001
From: sam-at-luther <sam.wood@luthersystems.com>
Date: Fri, 9 Feb 2024 14:12:36 -0800
Subject: [PATCH] Make span names even less restrictive

---
 lisp/x/profiler/funlabeler.go                 | 30 ++++---
 lisp/x/profiler/funlabeler_test.go            | 88 ++++---------------
 .../profiler/opentelemetry_annotator_test.go  |  4 +-
 3 files changed, 34 insertions(+), 88 deletions(-)

diff --git a/lisp/x/profiler/funlabeler.go b/lisp/x/profiler/funlabeler.go
index 130ae28..6f2e025 100644
--- a/lisp/x/profiler/funlabeler.go
+++ b/lisp/x/profiler/funlabeler.go
@@ -25,10 +25,11 @@ func WithFunLabeler(funLabeler FunLabeler) Option {
 // ELPSDocLabel is a magic string used to extract function labels.
 const ELPSDocLabel = `@trace\s*{([^}]+)}`
 
-var elpsDocLabelRegExp = regexp.MustCompile(ELPSDocLabel)
-var sanitizeRegExp = regexp.MustCompile(`[\W_]+`)
-
-var validLabelRegExp = regexp.MustCompile(`[[:alpha:]][\w_]*`)
+var (
+	elpsDocLabelRegExp = regexp.MustCompile(ELPSDocLabel)
+	sanitizeRegExp     = regexp.MustCompile(`[\s_]+`)
+	validLabelRegExp   = regexp.MustCompile(`[[:graph:]]*`)
+)
 
 func sanitizeLabel(userLabel string) string {
 	if userLabel == "" {
@@ -47,7 +48,11 @@ func sanitizeLabel(userLabel string) string {
 	return ""
 }
 
-func docLabel(docStr string) string {
+func extractLabel(docStr string) string {
+	if docStr == "" {
+		return ""
+	}
+
 	matches := elpsDocLabelRegExp.FindAllStringSubmatch(docStr, -1)
 	label := ""
 	for _, match := range matches {
@@ -56,17 +61,14 @@ func docLabel(docStr string) string {
 			break
 		}
 	}
+
 	return strings.TrimSpace(label)
 }
 
+func cleanLabel(docStr string) string {
+	return sanitizeLabel(extractLabel(docStr))
+}
+
 func elpsDocFunLabeler(runtime *lisp.Runtime, fun *lisp.LVal) string {
-	docStr := fun.Docstring()
-	if docStr == "" {
-		return ""
-	}
-	label := docLabel(docStr)
-	if label == "" {
-		return ""
-	}
-	return sanitizeLabel(label)
+	return extractLabel(fun.Docstring())
 }
diff --git a/lisp/x/profiler/funlabeler_test.go b/lisp/x/profiler/funlabeler_test.go
index ddbd740..9f27fa4 100644
--- a/lisp/x/profiler/funlabeler_test.go
+++ b/lisp/x/profiler/funlabeler_test.go
@@ -6,98 +6,42 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestLabelDoc(t *testing.T) {
+func TestCleanLabel(t *testing.T) {
 	tests := []struct {
 		name     string
 		label    string
 		expected string
 	}{
 		{
-			name:     "normal case",
-			label:    "@trace{ Add-It }",
-			expected: "Add-It",
-		},
-		{
-			name:     "normal case (2)",
-			label:    "@trace{ Add-It-Again }",
-			expected: "Add-It-Again",
-		},
-		{
-			name:     "elps names",
-			label:    "@trace{ user-add! }",
-			expected: "user-add!",
-		},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			actual := docLabel(tc.label)
-			assert.Equal(t, tc.expected, actual, "docLabel(%s)", tc.label)
-		})
-	}
-}
-
-func TestLabelSanitize(t *testing.T) {
-	tests := []struct {
-		name     string
-		label    string
-		expected string
-	}{
-		{
-			name:     "normal case",
-			label:    "NormalLabel",
-			expected: "NormalLabel",
-		},
-		{
-			name:     "contains spaces",
-			label:    "Label With Spaces",
-			expected: "Label_With_Spaces",
-		},
-		{
-			name:     "contains special characters",
-			label:    "Label@#$%^&",
-			expected: "Label_",
-		},
-		{
-			name:     "starts with a digit",
-			label:    "123Label",
-			expected: "Label",
-		},
-		{
-			name:     "empty string",
+			name:     "empty",
 			label:    "",
 			expected: "",
 		},
 		{
-			name:     "starts with an underscore",
-			label:    "_Label",
-			expected: "Label",
-		},
-		{
-			name:     "label with underscores",
-			label:    "label__with__underscores",
-			expected: "label_with_underscores",
+			name:     "normal",
+			label:    "@trace{ Add-It }",
+			expected: "Add-It",
 		},
 		{
-			name:     "starts with a special character",
-			label:    "@Label",
-			expected: "Label",
+			name:     "elps set",
+			label:    "@trace{ user-add! }",
+			expected: "user-add!",
 		},
 		{
-			name:     "starts with a space",
-			label:    " Label",
-			expected: "Label",
+			name:     "elps bool",
+			label:    "@trace { user-exists? }",
+			expected: "user-exists?",
 		},
 		{
-			name:     "dashes",
-			label:    "Add-It-Again",
-			expected: "Add_It_Again",
+			name:     "spaces",
+			label:    "@trace{Add  It}",
+			expected: "Add_It",
 		},
 	}
-
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			actual := sanitizeLabel(tc.label)
-			assert.Equal(t, tc.expected, actual, "sanitizeLabel(%s)", tc.label)
+			actual := cleanLabel(tc.label)
+			assert.Equal(t, tc.expected, actual, "cleanLabel(%s)", tc.label)
 		})
 	}
 }
diff --git a/lisp/x/profiler/opentelemetry_annotator_test.go b/lisp/x/profiler/opentelemetry_annotator_test.go
index 7baa522..352ac25 100644
--- a/lisp/x/profiler/opentelemetry_annotator_test.go
+++ b/lisp/x/profiler/opentelemetry_annotator_test.go
@@ -70,7 +70,7 @@ func TestNewOpenTelemetryAnnotatorSkip(t *testing.T) {
 
 	spans := exporter.GetSpans()
 	assert.Equal(t, 7, len(spans), "Expected selective spans")
-	assert.Equal(t, "Add_It", spans[0].Name, "Expected custom label")
-	assert.Equal(t, "Add_It_Again", spans[3].Name, "Expected custom label")
+	assert.Equal(t, "Add-It", spans[0].Name, "Expected custom label")
+	assert.Equal(t, "Add-It-Again", spans[3].Name, "Expected custom label")
 	assert.Equal(t, "lambda", spans[4].Name, "Expected custom label")
 }