From 640659ff1070bbb4bfa0db7aa6bf3bd0e4a244bb Mon Sep 17 00:00:00 2001
From: sam-at-luther <sam.wood@luthersystems.com>
Date: Thu, 8 Feb 2024 14:23:38 -0800
Subject: [PATCH] Cleanup function name labeler

---
 lisp/x/profiler/callgrind.go               |  5 +++--
 lisp/x/profiler/go_annotator.go            |  3 ++-
 lisp/x/profiler/opencensus_annotator.go    |  3 ++-
 lisp/x/profiler/opentelemetry_annotator.go |  3 ++-
 lisp/x/profiler/profiler.go                | 14 +++++++-------
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lisp/x/profiler/callgrind.go b/lisp/x/profiler/callgrind.go
index e772fdb..846efed 100644
--- a/lisp/x/profiler/callgrind.go
+++ b/lisp/x/profiler/callgrind.go
@@ -128,9 +128,10 @@ func (p *callgrindProfiler) Start(fun *lisp.LVal) func() {
 	if p.skipTrace(fun) {
 		return func() {}
 	}
+	prettyLabel, _ := p.prettyFunName(fun)
 	// Mark the time and point of entry. It feels like we're building the call stack in Runtime
 	// again, but we're not - it's called, not callers.
-	p.incrementCallRef(p.prettyFunName(fun), fun.Source)
+	p.incrementCallRef(prettyLabel, fun.Source)
 
 	return func() {
 		p.end(fun)
@@ -178,7 +179,7 @@ func (p *callgrindProfiler) end(fun *lisp.LVal) {
 	}
 	p.Lock()
 	defer p.Unlock()
-	fName := p.prettyFunName(fun)
+	fName, _ := p.prettyFunName(fun)
 	source, line := getSource(fun)
 	// Write what function we've been observing and where to find it
 	fmt.Fprintf(p.writer, "fl=%s\n", p.getRef(source))
diff --git a/lisp/x/profiler/go_annotator.go b/lisp/x/profiler/go_annotator.go
index 1a4d2eb..ebfdc9e 100644
--- a/lisp/x/profiler/go_annotator.go
+++ b/lisp/x/profiler/go_annotator.go
@@ -53,7 +53,8 @@ func (p *pprofAnnotator) Start(fun *lisp.LVal) func() {
 	// if we always ran inside a context, or a whole conditional code path and the added complication that brings
 	// if we did it that way.
 	oldContext := p.currentContext
-	p.currentContext = pprof.WithLabels(p.currentContext, pprof.Labels("function", p.prettyFunName(fun)))
+	prettyLabel, _ := p.prettyFunName(fun)
+	p.currentContext = pprof.WithLabels(p.currentContext, pprof.Labels("function", prettyLabel))
 	// apply the selected labels to the current goroutine (NB this will propagate if the code branches further down...
 	pprof.SetGoroutineLabels(p.currentContext)
 
diff --git a/lisp/x/profiler/opencensus_annotator.go b/lisp/x/profiler/opencensus_annotator.go
index 201fd46..5843c19 100644
--- a/lisp/x/profiler/opencensus_annotator.go
+++ b/lisp/x/profiler/opencensus_annotator.go
@@ -47,7 +47,8 @@ func (p *ocAnnotator) Start(fun *lisp.LVal) func() {
 		return func() {}
 	}
 	oldContext := p.currentContext
-	p.currentContext, p.currentSpan = trace.StartSpan(p.currentContext, p.prettyFunName(fun))
+	prettyLabel, _ := p.prettyFunName(fun)
+	p.currentContext, p.currentSpan = trace.StartSpan(p.currentContext, prettyLabel)
 	return func() {
 		file, line := getSource(fun)
 		p.currentSpan.Annotate([]trace.Attribute{
diff --git a/lisp/x/profiler/opentelemetry_annotator.go b/lisp/x/profiler/opentelemetry_annotator.go
index 6d91227..88e2744 100644
--- a/lisp/x/profiler/opentelemetry_annotator.go
+++ b/lisp/x/profiler/opentelemetry_annotator.go
@@ -62,7 +62,8 @@ func (p *otelAnnotator) Start(fun *lisp.LVal) func() {
 		return func() {}
 	}
 	oldContext := p.currentContext
-	p.currentContext, p.currentSpan = contextTracer(p.currentContext).Start(p.currentContext, p.prettyFunName(fun))
+	prettyLabel, _ := p.prettyFunName(fun)
+	p.currentContext, p.currentSpan = contextTracer(p.currentContext).Start(p.currentContext, prettyLabel)
 	return func() {
 		file, line := getSource(fun)
 		p.currentSpan.AddEvent("source", trace.WithAttributes(attribute.Key("file").String(file), attribute.Key("line").Int64(int64(line))))
diff --git a/lisp/x/profiler/profiler.go b/lisp/x/profiler/profiler.go
index df37d63..9dccc11 100644
--- a/lisp/x/profiler/profiler.go
+++ b/lisp/x/profiler/profiler.go
@@ -41,15 +41,15 @@ func (p *profiler) Start(fun *lisp.LVal) func() {
 	return func() {}
 }
 
-func (p *profiler) prettyFunName(fun *lisp.LVal) string {
-	label := ""
+// prettyFunName returns a pretty name and original name for a fun. If there is
+// no pretty name, then the pretty name is the original name.
+func (p *profiler) prettyFunName(fun *lisp.LVal) (string, string) {
+	origLabel := defaultFunName(p.runtime, fun)
+	prettyLabel := origLabel
 	if p.funLabeler != nil {
-		label = p.funLabeler(p.runtime, fun)
+		prettyLabel = p.funLabeler(p.runtime, fun)
 	}
-	if label == "" {
-		label = defaultFunName(p.runtime, fun)
-	}
-	return label
+	return prettyLabel, origLabel
 }
 
 // skipTrace is a helper function to decide whether to skip tracing.