diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d1a0f604..e0306d6dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Add `ByteSlice` and `ByteSliceValue` functions for new `BYTESLICE` attribute type in `go.opentelemetry.io/otel/attribute`. (#7948) +- Apply attribute value limit to the `KindBytes` attribute type in `go.opentelemetry.io/otel/sdk/log`. (#7990) +- Apply attribute value limit to the `BYTESLICE` attribute type in `go.opentelemetry.io/otel/sdk/trace`. (#7990) - Support `BYTESLICE` attributes in `go.opentelemetry.io/otel/trace`. (#8153) - Support `BYTESLICE` attributes in `go.opentelemetry.io/otel/exporters/otlp/otlptrace`. (#8153) - Support `BYTESLICE` attributes in `go.opentelemetry.io/otel/exporters/otlp/otlplog`. (#8153) diff --git a/sdk/log/provider.go b/sdk/log/provider.go index 49e9ba290..4ac799bfd 100644 --- a/sdk/log/provider.go +++ b/sdk/log/provider.go @@ -240,8 +240,8 @@ func WithAttributeCountLimit(limit int) LoggerProviderOption { // WithAttributeValueLengthLimit sets the maximum allowed attribute value length. // -// This limit only applies to string and string slice attribute values. -// Any string longer than this value will be truncated to this length. +// This limit only applies to string, string slice, and byte slice attribute values. +// Strings and byte slices longer than this value will be truncated to this length. // // Setting this to a negative value means no limit is applied. // diff --git a/sdk/log/record.go b/sdk/log/record.go index 66ed1adb3..2b3c31e3f 100644 --- a/sdk/log/record.go +++ b/sdk/log/record.go @@ -503,7 +503,11 @@ func (r *Record) applyValueLimitsAndDedup(val log.Value) log.Value { } val = log.SliceValue(newSl...) } - + case log.KindBytes: + bs := val.AsBytes() + if r.attributeValueLengthLimit >= 0 && len(bs) > r.attributeValueLengthLimit { + val = log.BytesValue(bs[:r.attributeValueLengthLimit]) + } case log.KindMap: kvs := val.AsMap() var newKvs []log.KeyValue @@ -559,6 +563,11 @@ func (r *Record) needsValueLimitsOrDedup(val log.Value) bool { if slices.ContainsFunc(val.AsSlice(), r.needsValueLimitsOrDedup) { return true } + case log.KindBytes: + bs := val.AsBytes() + if r.attributeValueLengthLimit >= 0 && len(bs) > r.attributeValueLengthLimit { + return true + } case log.KindMap: kvs := val.AsMap() if !r.allowDupKeys && len(kvs) > 1 { diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index 4c7d49e57..472e0b2d1 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -1200,7 +1200,7 @@ func TestApplyAttrLimitsTruncation(t *testing.T) { name: "Bytes", limit: 0, input: log.BytesValue([]byte("foo")), - want: log.BytesValue([]byte("foo")), + want: log.BytesValue([]byte("")), }, { name: "String", @@ -1225,7 +1225,7 @@ func TestApplyAttrLimitsTruncation(t *testing.T) { log.BoolValue(true), log.Float64Value(1.3), log.Int64Value(43), - log.BytesValue([]byte("hello")), + log.BytesValue([]byte("")), log.StringValue(""), log.StringValue(""), log.SliceValue(log.StringValue("")), @@ -1249,7 +1249,7 @@ func TestApplyAttrLimitsTruncation(t *testing.T) { log.Bool("0", true), log.Float64("1", 1.3), log.Int64("2", 43), - log.Bytes("3", []byte("hello")), + log.Bytes("3", []byte("")), log.String("4", ""), log.String("5", ""), log.Slice("6", log.StringValue("")), @@ -1263,10 +1263,10 @@ func TestApplyAttrLimitsTruncation(t *testing.T) { want: log.StringValue("This "), }, { - name: "LongBytesNotTruncated", + name: "LongBytesTruncated", limit: 5, input: log.BytesValue([]byte("This is a very long byte array")), - want: log.BytesValue([]byte("This is a very long byte array")), + want: log.BytesValue([]byte("This ")), }, { name: "TruncationInNestedMap", @@ -1292,6 +1292,18 @@ func TestApplyAttrLimitsTruncation(t *testing.T) { log.StringValue("tool"), ), }, + { + name: "TruncationInNestedSliceOfBytes", + limit: 4, + input: log.SliceValue( + log.BytesValue([]byte("good")), + log.BytesValue([]byte("toolong")), + ), + want: log.SliceValue( + log.BytesValue([]byte("good")), + log.BytesValue([]byte("tool")), + ), + }, } for _, tc := range testcases { diff --git a/sdk/trace/benchmark_test.go b/sdk/trace/benchmark_test.go index 080d72a85..02d04609f 100644 --- a/sdk/trace/benchmark_test.go +++ b/sdk/trace/benchmark_test.go @@ -36,10 +36,12 @@ func benchmarkSpanLimits(b *testing.B, limits sdktrace.SpanLimits) { attribute.Float64("float64", 42), attribute.Float64Slice("float64Slice", []float64{42, -1}), attribute.String("string", "value"), + attribute.ByteSlice("byteSlice", []byte("value")), attribute.StringSlice("stringSlice", []string{"value", "value-1"}), attribute.Slice("slice", attribute.StringValue("value"), attribute.StringSliceValue([]string{"value", "value-1"}), + attribute.ByteSliceValue([]byte{1, 2, 3}), ), } diff --git a/sdk/trace/span.go b/sdk/trace/span.go index a6acfaee4..5242bf1ad 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -347,10 +347,11 @@ func (s *recordingSpan) addOverCapAttrs(limit int, attrs []attribute.KeyValue) { } // truncateAttr returns a truncated version of attr. Only string, string -// slice, and slice attribute values are truncated. String values are truncated +// slice, byte slice, and slice attribute values are truncated. String values are truncated // to at most a length of limit. Each string slice value is truncated in this -// fashion (the slice length itself is unaffected). For slice attribute values, -// the limit is applied to each element recursively. +// fashion (the slice length itself is unaffected), and byte slice values are truncated to at most +// limit bytes. For slice attribute values, the limit is applied to each +// element recursively. // // No truncation is performed for a negative limit. func truncateAttr(limit int, attr attribute.KeyValue) attribute.KeyValue { @@ -367,6 +368,12 @@ func truncateAttr(limit int, attr attribute.KeyValue) attribute.KeyValue { v[i] = truncate(limit, v[i]) } return attr.Key.StringSlice(v) + case attribute.BYTESLICE: + v := attr.Value.AsString() + if len(v) > limit { + return attr.Key.ByteSlice([]byte(v[:limit])) + } + return attr case attribute.SLICE: v := attr.Value.AsSlice() if !slices.ContainsFunc(v, func(e attribute.Value) bool { return needsTruncation(limit, e) }) { @@ -382,7 +389,7 @@ func truncateAttr(limit int, attr attribute.KeyValue) attribute.KeyValue { } // truncateValue returns a truncated version of v. Only string, string slice, -// and (recursively) slice values are modified. +// byte slice, and (recursively) slice values are modified. // // No truncation is performed for a negative limit. func truncateValue(limit int, v attribute.Value) attribute.Value { @@ -395,6 +402,14 @@ func truncateValue(limit int, v attribute.Value) attribute.Value { ss[i] = truncate(limit, ss[i]) } return attribute.StringSliceValue(ss) + + case attribute.BYTESLICE: + // len(v.AsString()) is identical to len(v.AsByteSlice()) but + // avoids allocating the full slice before truncation. + s := v.AsString() + if limit >= 0 && len(s) > limit { + return attribute.ByteSliceValue([]byte(s[:limit])) + } case attribute.SLICE: sl := v.AsSlice() if !slices.ContainsFunc(sl, func(e attribute.Value) bool { return needsTruncation(limit, e) }) { @@ -424,6 +439,12 @@ func needsTruncation(limit int, v attribute.Value) bool { switch v.Type() { case attribute.STRING: return stringNeedsTruncation(limit, v.AsString()) + case attribute.BYTESLICE: + // len(v.AsString()) is identical to len(v.AsByteSlice()) but + // avoids memory allocation. + if limit >= 0 && len(v.AsString()) > limit { + return true + } case attribute.STRINGSLICE: for _, s := range v.AsStringSlice() { if stringNeedsTruncation(limit, s) { diff --git a/sdk/trace/span_limits.go b/sdk/trace/span_limits.go index d35ac8ae3..348ee0e80 100644 --- a/sdk/trace/span_limits.go +++ b/sdk/trace/span_limits.go @@ -35,9 +35,9 @@ const ( type SpanLimits struct { // AttributeValueLengthLimit is the maximum allowed attribute value length. // - // This limit only applies to string, string slice, and slice attribute - // values. Any string longer than this value will be truncated to this - // length. For slice attribute values, the limit is applied to each string + // This limit only applies to string, string slice, byte slice, and slice attribute + // values. Any string and byte slice longer than this value will be truncated to this + // length. For slice attribute values, the limit is applied to each string and byte slice // element recursively. // // Setting this to a negative value means no limit is applied. diff --git a/sdk/trace/span_test.go b/sdk/trace/span_test.go index bb0c69c65..6b82bdd19 100644 --- a/sdk/trace/span_test.go +++ b/sdk/trace/span_test.go @@ -94,6 +94,7 @@ func TestTruncateAttr(t *testing.T) { const key = "key" strAttr := attribute.String(key, "value") + bytesAttr := attribute.ByteSlice(key, []byte("value")) strSliceAttr := attribute.StringSlice(key, []string{"value-0", "value-1"}) tests := []struct { @@ -110,6 +111,11 @@ func TestTruncateAttr(t *testing.T) { attr: strSliceAttr, want: strSliceAttr, }, + { + limit: -1, + attr: bytesAttr, + want: bytesAttr, + }, { limit: 0, attr: attribute.Bool(key, true), @@ -165,6 +171,11 @@ func TestTruncateAttr(t *testing.T) { attr: attribute.Stringer(key, bytes.NewBufferString("value")), want: attribute.String(key, ""), }, + { + limit: 0, + attr: bytesAttr, + want: attribute.ByteSlice(key, []byte{}), + }, { limit: 1, attr: strAttr, @@ -175,11 +186,21 @@ func TestTruncateAttr(t *testing.T) { attr: strSliceAttr, want: attribute.StringSlice(key, []string{"v", "v"}), }, + { + limit: 1, + attr: bytesAttr, + want: attribute.ByteSlice(key, []byte("v")), + }, { limit: 5, attr: strAttr, want: strAttr, }, + { + limit: 5, + attr: bytesAttr, + want: bytesAttr, + }, { limit: 7, attr: strSliceAttr, @@ -200,6 +221,11 @@ func TestTruncateAttr(t *testing.T) { attr: strSliceAttr, want: strSliceAttr, }, + { + limit: 128, + attr: bytesAttr, + want: bytesAttr, + }, { // Multi-byte string: byte length (9) exceeds limit (5) but rune count (3) does not. // Must not be truncated. @@ -301,6 +327,30 @@ func TestTruncateAttr(t *testing.T) { attr: attribute.Slice(key, attribute.StringValue("日\x80")), // 2 runes (日 + invalid byte), 4 bytes want: attribute.Slice(key, attribute.StringValue("日")), }, + { + // BYTESLICE within SLICE: each byte slice is truncated. + limit: 2, + attr: attribute.Slice(key, attribute.ByteSliceValue([]byte{1, 2, 3})), + want: attribute.Slice(key, attribute.ByteSliceValue([]byte{1, 2})), + }, + { + // BYTESLICE within SLICE: no truncation needed. + limit: 5, + attr: attribute.Slice(key, attribute.ByteSliceValue([]byte{1, 2})), + want: attribute.Slice(key, attribute.ByteSliceValue([]byte{1, 2})), + }, + { + // Mixed SLICE: BYTESLICE + STRING (both need truncation). + limit: 2, + attr: attribute.Slice(key, + attribute.ByteSliceValue([]byte{1, 2, 3}), + attribute.StringValue("abc"), + ), + want: attribute.Slice(key, + attribute.ByteSliceValue([]byte{1, 2}), + attribute.StringValue("ab"), + ), + }, } for _, test := range tests { @@ -433,6 +483,37 @@ func TestTruncate(t *testing.T) { } } +func BenchmarkTruncateAttr(b *testing.B) { + const key = "key" + + strAttr := attribute.String(key, "value") + bytesAttr := attribute.ByteSlice(key, []byte("value")) + strSliceAttr := attribute.StringSlice(key, []string{"value-0", "value-1"}) + + run := func(limit int, attr attribute.KeyValue) func(b *testing.B) { + return func(b *testing.B) { + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + var out attribute.KeyValue + for pb.Next() { + out = truncateAttr(limit, attr) + } + _ = out + }) + } + } + + b.Run("String", run(3, strAttr)) + b.Run("StringSlice", run(3, strSliceAttr)) + b.Run("ByteSlice", run(3, bytesAttr)) + b.Run("String/Limit0", run(0, strAttr)) + b.Run("StringSlice/Limit0", run(0, strSliceAttr)) + b.Run("ByteSlice/Limit0", run(0, bytesAttr)) + b.Run("String/Unlimited", run(-1, strAttr)) + b.Run("StringSlice/Unlimited", run(-1, strSliceAttr)) + b.Run("ByteSlice/Unlimited", run(-1, bytesAttr)) +} + func BenchmarkTruncate(b *testing.B) { run := func(limit int, input string) func(b *testing.B) { return func(b *testing.B) { diff --git a/trace/auto.go b/trace/auto.go index 483078584..64ac3346c 100644 --- a/trace/auto.go +++ b/trace/auto.go @@ -315,7 +315,13 @@ func convAttrValue(value attribute.Value) telemetry.Value { v := truncate(maxSpan.AttrValueLen, value.AsString()) return telemetry.StringValue(v) case attribute.BYTESLICE: - return telemetry.BytesValue(value.AsByteSlice()) + // len(v.AsString()) is identical to len(v.AsByteSlice()) but + // avoids allocating the full slice before truncation. + s := value.AsString() + if maxSpan.AttrValueLen >= 0 && len(s) > maxSpan.AttrValueLen { + return telemetry.BytesValue([]byte(s[:maxSpan.AttrValueLen])) + } + return telemetry.BytesValue([]byte(s)) case attribute.BOOLSLICE: slice := value.AsBoolSlice() out := make([]telemetry.Value, 0, len(slice)) diff --git a/trace/auto_test.go b/trace/auto_test.go index 4b24e31c6..45dec38a3 100644 --- a/trace/auto_test.go +++ b/trace/auto_test.go @@ -165,12 +165,45 @@ func TestSpanKindTransform(t *testing.T) { } func TestConvAttrValueBytes(t *testing.T) { - t.Parallel() + v := []byte("bytes") + tests := []struct { + name string + want []byte + limit int + }{ + { + name: "Unlimited", + want: []byte("bytes"), + limit: -1, + }, + { + name: "Zero", + want: []byte(""), + limit: 0, + }, + { + name: "Truncate", + want: []byte("by"), + limit: 2, + }, + { + name: "NoTruncation", + want: []byte("bytes"), + limit: 10, + }, + } + orig := maxSpan.AttrValueLen + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Cleanup(func() { maxSpan.AttrValueLen = orig }) + maxSpan.AttrValueLen = test.limit - val := convAttrValue(attribute.ByteSliceValue([]byte("bytes"))) + val := convAttrValue(attribute.ByteSliceValue(v)) - assert.Equal(t, telemetry.ValueKindBytes, val.Kind()) - assert.Equal(t, []byte("bytes"), val.AsBytes()) + assert.Equal(t, telemetry.ValueKindBytes, val.Kind()) + assert.Equal(t, test.want, val.AsBytes()) + }) + } } func TestConvAttrValueSlice(t *testing.T) {