Safely truncate over-length string attributes

sdk/trace/span.go

@@ -20,8 +20,10 @@ import (
 	"reflect"
 	"runtime"
 	rt "runtime/trace"
+	"strings"
 	"sync"
 	"time"
+	"unicode/utf8"
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
@@ -294,7 +296,7 @@ func (s *recordingSpan) addOverCapAttrs(limit int, attrs []attribute.KeyValue) {
 
 // truncateAttr returns a truncated version of attr. Only string and string
 // slice attribute values are truncated. String values are truncated to at
-// most a length of limit. Each string slice value is truncated in this fasion
+// most a length of limit. Each string slice value is truncated in this fashion
 // (the slice length itself is unaffected).
 //
 // No truncation is perfromed for a negative limit.
@@ -305,7 +307,7 @@ func truncateAttr(limit int, attr attribute.KeyValue) attribute.KeyValue {
 	switch attr.Value.Type() {
 	case attribute.STRING:
 		if v := attr.Value.AsString(); len(v) > limit {
-			return attr.Key.String(v[:limit])
+			return attr.Key.String(safeTruncate(v, limit))
 		}
 	case attribute.STRINGSLICE:
 		// Do no mutate the original, make a copy.
@@ -324,14 +326,41 @@ func truncateAttr(limit int, attr attribute.KeyValue) attribute.KeyValue {
 		v := trucated.Value.AsStringSlice()
 		for i := range v {
 			if len(v[i]) > limit {
-				v[i] = v[i][:limit]
+				v[i] = safeTruncate(v[i], limit)
 			}
 		}
 		return trucated
 	}
 	return attr
 }
 
+// safeTruncate truncates the string and guarantees valid UTF-8 is returned.
+func safeTruncate(input string, limit int) string {
+	cnt := 0
+	for cnt <= limit {
+		r, size := utf8.DecodeRuneInString(input[cnt:])
+		if r == utf8.RuneError {
+			break
+		}
+
+		if cnt+size > limit {
+			return input[:cnt]
+		}
+		cnt += size
+	}
+	return invalidTruncate(input, limit)
+}
+
+// invalidTruncate truncates the string and then removes invalid UTF-8 points.
+// this is a fallback from safeTruncate() in case the input is invalid.
+func invalidTruncate(input string, limit int) string {
+	valid := strings.ToValidUTF8(input[:limit], "")
+	if len(valid) <= limit {
+		return valid
+	}
+	return valid[:limit]
+}
+
 // End ends the span. This method does nothing if the span is already ended or
 // is not being recorded.
 //

sdk/trace/span_limits_test.go

@@ -168,6 +168,7 @@ func testSpanLimits(t *testing.T, limits SpanLimits) ReadOnlySpan {
 	span.SetAttributes(
 		attribute.String("string", "abc"),
 		attribute.StringSlice("stringSlice", []string{"abc", "def"}),
+		attribute.String("euro", "€"), // this is a 3-byte rune
 	)
 	span.AddEvent("event 1", trace.WithAttributes(a...))
 	span.AddEvent("event 2", trace.WithAttributes(a...))
@@ -186,24 +187,27 @@ func TestSpanLimits(t *testing.T) {
 		attrs := testSpanLimits(t, limits).Attributes()
 		assert.Contains(t, attrs, attribute.String("string", "abc"))
 		assert.Contains(t, attrs, attribute.StringSlice("stringSlice", []string{"abc", "def"}))
+		assert.Contains(t, attrs, attribute.String("euro", "€"))
 
 		limits.AttributeValueLengthLimit = 2
 		attrs = testSpanLimits(t, limits).Attributes()
 		// Ensure string and string slice attributes are truncated.
 		assert.Contains(t, attrs, attribute.String("string", "ab"))
 		assert.Contains(t, attrs, attribute.StringSlice("stringSlice", []string{"ab", "de"}))
+		assert.Contains(t, attrs, attribute.String("euro", ""))
 
 		limits.AttributeValueLengthLimit = 0
 		attrs = testSpanLimits(t, limits).Attributes()
 		assert.Contains(t, attrs, attribute.String("string", ""))
 		assert.Contains(t, attrs, attribute.StringSlice("stringSlice", []string{"", ""}))
+		assert.Contains(t, attrs, attribute.String("euro", ""))
 	})
 
 	t.Run("AttributeCountLimit", func(t *testing.T) {
 		limits := NewSpanLimits()
 		// Unlimited.
 		limits.AttributeCountLimit = -1
-		assert.Len(t, testSpanLimits(t, limits).Attributes(), 2)
+		assert.Len(t, testSpanLimits(t, limits).Attributes(), 3)
 
 		limits.AttributeCountLimit = 1
 		assert.Len(t, testSpanLimits(t, limits).Attributes(), 1)

sdk/trace/span_test.go

@@ -134,6 +134,22 @@ func TestTruncateAttr(t *testing.T) {
 			attr:  strSliceAttr,
 			want:  strSliceAttr,
 		},
+		{
+			// This tests the ordinary safeTruncate().
+			limit: 10,
+			attr:  attribute.String(key, "€€€€"), // 3 bytes each
+			want:  attribute.String(key, "€€€"),
+		},
+		{
+			// This tests the fallback to invalidTruncate().
+			//
+			// Note that after removing the invalid rune,
+			// the string is over length and still has to
+			// be truncated on a code point boundary.
+			limit: 10,
+			attr:  attribute.String(key, "€"[0:2]+"hello€€"), // corrupted first rune, over limit
+			want:  attribute.String(key, "hello€"),
+		},
 	}
 
 	for _, test := range tests {