From bd740f4a971c90e955222a6da3c4dca942dcf0a1 Mon Sep 17 00:00:00 2001
From: Robb Kidd <robbkidd@honeycomb.io>
Date: Tue, 28 Nov 2023 18:10:07 -0500
Subject: [PATCH] fix: reduce memalloc if field names pre-prefixed

There is a remarkable amount of memory allocation occurring under load
to perform the field name "app." prefixing by the Beeline with "app.".
This change skips the memory allocations needed for string concat and
usage if the "app." prefix is already present on the field name
provided.
---
 beeline.go      | 30 ++++++++++++++++++++++--------
 beeline_test.go | 43 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/beeline.go b/beeline.go
index 9cda6c0f..9ce3dbb2 100644
--- a/beeline.go
+++ b/beeline.go
@@ -272,7 +272,10 @@ func Close() {
 // a Handler, feel free to call AddField freely within your code. Pass it the
 // context from the request (`r.Context()`) and the key and value you wish to
 // add.This function is good for span-level data, eg timers or the arguments to
-// a specific function call, etc. Fields added here are prefixed with `app.`
+// a specific function call, etc.
+//
+// Field keys added will be prefixed with `app.` if the key does not already have
+// that prefix.
 //
 // Errors are treated as a special case for convenience: if `val` is of type
 // `error` then the key is set to the error's message in the span.
@@ -280,13 +283,12 @@ func AddField(ctx context.Context, key string, val interface{}) {
 	span := trace.GetSpanFromContext(ctx)
 	if span != nil {
 		if val != nil {
-			namespacedKey := "app." + key // Avoid excess parsing/allocation work
 			if valErr, ok := val.(error); ok {
 				// treat errors specially because it's a pain to have to
 				// remember to stringify them
-				span.AddField(namespacedKey, valErr.Error())
+				span.AddField(getNamespacedKey(key), valErr.Error())
 			} else {
-				span.AddField(namespacedKey, val)
+				span.AddField(getNamespacedKey(key), val)
 			}
 		}
 	}
@@ -297,14 +299,26 @@ func AddField(ctx context.Context, key string, val interface{}) {
 // Additionally, these fields are packaged up and passed along to downstream
 // processes if they are also using a beeline. This function is good for adding
 // context that is better scoped to the request than this specific unit of work,
-// eg user IDs, globally relevant feature flags, errors, etc. Fields added here
-// are prefixed with `app.`
+// eg user IDs, globally relevant feature flags, errors, etc.
+//
+// Field keys added will be prefixed with `app.` if the key does not already have
+// that prefix.
 func AddFieldToTrace(ctx context.Context, key string, val interface{}) {
-	namespacedKey := "app." + key // Avoid excess parsing/allocation work
 	tr := trace.GetTraceFromContext(ctx)
 	if tr != nil {
-		tr.AddField(namespacedKey, val)
+		tr.AddField(getNamespacedKey(key), val)
+	}
+}
+
+// getNamespacedKey ensures a key name is prefixed with "app." if the prefix
+// isn't already present. If the key is already namespaced, this reduces the
+// number of memory allocations needed to add a field to a span.
+func getNamespacedKey(key string) string {
+	const prefix = "app."
+	if strings.HasPrefix(key, prefix) {
+		return key
 	}
+	return prefix + key
 }
 
 // StartSpan lets you start a new span as a child of an already instrumented
diff --git a/beeline_test.go b/beeline_test.go
index 262585d1..c6568ff2 100644
--- a/beeline_test.go
+++ b/beeline_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/honeycombio/beeline-go/trace"
 	"github.com/honeycombio/libhoney-go/transmission"
 
 	libhoney "github.com/honeycombio/libhoney-go"
@@ -105,8 +106,46 @@ func BenchmarkBeelineAddField(b *testing.B) {
 	setupLibhoney(b)
 
 	ctx, _ := StartSpan(context.Background(), "parent")
-	for n := 0; n < b.N; n++ {
-		AddField(ctx, "foo", 1)
+
+	b.Run("oldAddField/whatever", func(b *testing.B) {
+		for n := 0; n < b.N; n++ {
+			oldAddField(ctx, "foo", 1)
+		}
+	})
+	b.Run("AddField/no-prefix", func(b *testing.B) {
+		for n := 0; n < b.N; n++ {
+			AddField(ctx, "foo", 1)
+		}
+	})
+	b.Run("AddField/half-prefixed", func(b *testing.B) {
+		for n := 0; n < b.N; n++ {
+			if n%2 == 0 {
+				AddField(ctx, "app.foo", 1)
+			} else {
+				AddField(ctx, "foo", 1)
+			}
+		}
+	})
+	b.Run("AddField/all-prefixed", func(b *testing.B) {
+		for n := 0; n < b.N; n++ {
+			AddField(ctx, "app.foo", 1)
+		}
+	})
+}
+
+func oldAddField(ctx context.Context, key string, val interface{}) {
+	span := trace.GetSpanFromContext(ctx)
+	if span != nil {
+		if val != nil {
+			namespacedKey := "app." + key
+			if valErr, ok := val.(error); ok {
+				// treat errors specially because it's a pain to have to
+				// remember to stringify them
+				span.AddField(namespacedKey, valErr.Error())
+			} else {
+				span.AddField(namespacedKey, val)
+			}
+		}
 	}
 }