From dde2011577872862559d512e1e1e64ca04ef4472 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 18 Nov 2024 16:39:44 -0800 Subject: [PATCH 1/5] Configure span limits Add functionality to `auto/sdk` so it can interpret configuration from OTel defined environment variables for tracing limits. --- sdk/limit.go | 87 +++++++++++++++++++++++++++++++++++++++++++++++ sdk/limit_test.go | 81 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 sdk/limit.go create mode 100644 sdk/limit_test.go diff --git a/sdk/limit.go b/sdk/limit.go new file mode 100644 index 000000000..caae4a2d9 --- /dev/null +++ b/sdk/limit.go @@ -0,0 +1,87 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package sdk + +import ( + "os" + "strconv" +) + +// maxSpan are the span limits resolved during startup. +var maxSpan = newSpanLimits() + +type spanLimits struct { + // Attrs is the number of allowed attributes for a span. + // + // This is resolved from the environment variable value for the + // OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT key if it exists. Otherwise, the + // environment variable value for OTEL_ATTRIBUTE_COUNT_LIMIT, or 128 if + // that is not set, is used. + Attrs int + // AttrValueLen is the maximum attribute value length allowed for a span. + // + // This is resolved from the environment variable value for the + // OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT key if it exists. Otherwise, the + // environment variable value for OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT, or -1 + // if that is not set, is used. + AttrValueLen int + // Events is the number of allowed events for a span. + // + // This is resolved from the environment variable value for the + // OTEL_SPAN_EVENT_COUNT_LIMIT key, or 128 is used if that is not set. + Events int + // EventAttrs is the number of allowed attributes for a span event. + // + // The is resolved from the environment variable value for the + // OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT key, or 128 is used if that is not set. + EventAttrs int + // Links is the number of allowed Links for a span. + // + // This is resolved from the environment variable value for the + // OTEL_SPAN_LINK_COUNT_LIMIT, or 128 is used if that is not set. + Links int + // LinkAttrs is the number of allowed attributes for a span link. + // + // This is resolved from the environment variable value for the + // OTEL_LINK_ATTRIBUTE_COUNT_LIMIT, or 128 is used if that is not set. + LinkAttrs int +} + +func newSpanLimits() spanLimits { + return spanLimits{ + Attrs: firstEnv( + 128, + "OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", + "OTEL_ATTRIBUTE_COUNT_LIMIT", + ), + AttrValueLen: firstEnv( + -1, // Unlimited. + "OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", + "OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", + ), + Events: firstEnv(128, "OTEL_SPAN_EVENT_COUNT_LIMIT"), + EventAttrs: firstEnv(128, "OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT"), + Links: firstEnv(128, "OTEL_SPAN_LINK_COUNT_LIMIT"), + LinkAttrs: firstEnv(128, "OTEL_LINK_ATTRIBUTE_COUNT_LIMIT"), + } +} + +// firstEnv returns the parsed integer value of the first matching environment +// variable from keys. The defaultVal is returned if the value is not an +// integer or no match is found. +func firstEnv(defaultVal int, keys ...string) int { + for _, key := range keys { + strV := os.Getenv(key) + if strV == "" { + continue + } + + v, err := strconv.Atoi(strV) + if err == nil { + return v + } + } + + return defaultVal +} diff --git a/sdk/limit_test.go b/sdk/limit_test.go new file mode 100644 index 000000000..ec63dd405 --- /dev/null +++ b/sdk/limit_test.go @@ -0,0 +1,81 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package sdk + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSpanAttrValLenLimit(t *testing.T) { + testLimit( + t, + func(sl spanLimits) int { return sl.AttrValueLen }, + -1, + "OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", + "OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", + ) +} + +func TestSpanAttrsLimit(t *testing.T) { + testLimit( + t, + func(sl spanLimits) int { return sl.Attrs }, + 128, + "OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", + "OTEL_ATTRIBUTE_COUNT_LIMIT", + ) +} + +func TestSpanEventsLimit(t *testing.T) { + testLimit( + t, + func(sl spanLimits) int { return sl.Events }, + 128, + "OTEL_SPAN_EVENT_COUNT_LIMIT", + ) +} + +func TestSpanLinksLimit(t *testing.T) { + testLimit( + t, + func(sl spanLimits) int { return sl.Links }, + 128, + "OTEL_SPAN_LINK_COUNT_LIMIT", + ) +} + +func TestSpanEventAttrsLimit(t *testing.T) { + testLimit( + t, + func(sl spanLimits) int { return sl.EventAttrs }, + 128, + "OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT", + ) +} + +func TestSpanLinkAttrsLimit(t *testing.T) { + testLimit( + t, + func(sl spanLimits) int { return sl.LinkAttrs }, + 128, + "OTEL_LINK_ATTRIBUTE_COUNT_LIMIT", + ) +} + +func testLimit(t *testing.T, f func(spanLimits) int, zero int, keys ...string) { + t.Helper() + + t.Run("Default", func(t *testing.T) { + assert.Equal(t, zero, f(newSpanLimits())) + }) + + for _, key := range keys { + t.Run(key, func(t *testing.T) { + t.Setenv(key, "43") + assert.Equal(t, 43, f(newSpanLimits())) + }) + } +} From 661f5a3f0e9ba6b217a1e42a5d0dfaec7a487e16 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 24 Nov 2024 08:58:27 -0800 Subject: [PATCH 2/5] Add attribute value limits Include tests for span attribute limits. --- sdk/span.go | 82 +++++++++++++++++- sdk/span_test.go | 219 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+), 1 deletion(-) diff --git a/sdk/span.go b/sdk/span.go index d8f5786e0..e2683f8c4 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -8,9 +8,11 @@ import ( "fmt" "reflect" "runtime" + "strings" "sync" "sync/atomic" "time" + "unicode/utf8" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -130,7 +132,8 @@ func convAttrValue(value attribute.Value) telemetry.Value { case attribute.FLOAT64: return telemetry.Float64Value(value.AsFloat64()) case attribute.STRING: - return telemetry.StringValue(value.AsString()) + v := truncate(maxSpan.AttrValueLen, value.AsString()) + return telemetry.StringValue(v) case attribute.BOOLSLICE: slice := value.AsBoolSlice() out := make([]telemetry.Value, 0, len(slice)) @@ -156,6 +159,7 @@ func convAttrValue(value attribute.Value) telemetry.Value { slice := value.AsStringSlice() out := make([]telemetry.Value, 0, len(slice)) for _, v := range slice { + v = truncate(maxSpan.AttrValueLen, v) out = append(out, telemetry.StringValue(v)) } return telemetry.SliceValue(out...) @@ -163,6 +167,82 @@ func convAttrValue(value attribute.Value) telemetry.Value { return telemetry.Value{} } +// truncate returns a truncated version of s such that it contains less than +// the limit number of characters. Truncation is applied by returning the limit +// number of valid characters contained in s. +// +// If limit is negative, it returns the original string. +// +// UTF-8 is supported. When truncating, all invalid characters are dropped +// before applying truncation. +// +// If s already contains less than the limit number of bytes, it is returned +// unchanged. No invalid characters are removed. +func truncate(limit int, s string) string { + // This prioritize performance in the following order based on the most + // common expected use-cases. + // + // - Short values less than the default limit (128). + // - Strings with valid encodings that exceed the limit. + // - No limit. + // - Strings with invalid encodings that exceed the limit. + if limit < 0 || len(s) <= limit { + return s + } + + // Optimistically, assume all valid UTF-8. + var b strings.Builder + count := 0 + for i, c := range s { + if c != utf8.RuneError { + count++ + if count > limit { + return s[:i] + } + continue + } + + _, size := utf8.DecodeRuneInString(s[i:]) + if size == 1 { + // Invalid encoding. + b.Grow(len(s) - 1) + _, _ = b.WriteString(s[:i]) + s = s[i:] + break + } + } + + // Fast-path, no invalid input. + if b.Cap() == 0 { + return s + } + + // Truncate while validating UTF-8. + for i := 0; i < len(s) && count < limit; { + c := s[i] + if c < utf8.RuneSelf { + // Optimization for single byte runes (common case). + _ = b.WriteByte(c) + i++ + count++ + continue + } + + _, size := utf8.DecodeRuneInString(s[i:]) + if size == 1 { + // We checked for all 1-byte runes above, this is a RuneError. + i++ + continue + } + + _, _ = b.WriteString(s[i : i+size]) + i += size + count++ + } + + return b.String() +} + func (s *span) End(opts ...trace.SpanEndOption) { if s == nil || !s.sampled.Swap(false) { return diff --git a/sdk/span_test.go b/sdk/span_test.go index 00d3a272d..b963ca076 100644 --- a/sdk/span_test.go +++ b/sdk/span_test.go @@ -456,6 +456,75 @@ func TestSpanSetAttributes(t *testing.T) { assert.Equal(t, tAttrs, s.span.Attrs, "SpanAttributes did not override") } +func TestSpanAttributeValueLimits(t *testing.T) { + value := "hello world" + + aStr := attribute.String("string", value) + aStrSlice := attribute.StringSlice("slice", []string{value, value}) + + eq := func(a, b []telemetry.Attr) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if !a[i].Equal(b[i]) { + return false + } + } + return true + } + + tests := []struct { + limit int + want string + }{ + {0, ""}, + {2, value[:2]}, + {11, value}, + {-1, value}, + } + for _, test := range tests { + t.Run("Limit/"+strconv.Itoa(test.limit), func(t *testing.T) { + orig := maxSpan.AttrValueLen + maxSpan.AttrValueLen = test.limit + t.Cleanup(func() { maxSpan.AttrValueLen = orig }) + + builder := spanBuilder{} + + want := []telemetry.Attr{ + telemetry.String("string", test.want), + telemetry.Slice( + "slice", + telemetry.StringValue(test.want), + telemetry.StringValue(test.want), + ), + } + + s := builder.Build() + s.SetAttributes(aStr, aStrSlice) + assert.Truef(t, eq(want, s.span.Attrs), "set span attributes: got %#v, want %#v", s.span.Attrs, want) + + s.AddEvent("test", trace.WithAttributes(aStr, aStrSlice)) + assert.Truef(t, eq(want, s.span.Events[0].Attrs), "span event attributes: got %#v, want %#v", s.span.Events[0].Attrs, want) + + s.AddLink(trace.Link{ + Attributes: []attribute.KeyValue{aStr, aStrSlice}, + }) + assert.Truef(t, eq(want, s.span.Links[0].Attrs), "span link attributes: got %#v, want %#v", s.span.Links[0].Attrs, want) + + builder.Options = []trace.SpanStartOption{ + trace.WithAttributes(aStr, aStrSlice), + trace.WithLinks(trace.Link{ + Attributes: []attribute.KeyValue{aStr, aStrSlice}, + }), + } + s = builder.Build() + assert.Truef(t, eq(want, s.span.Attrs), "new span attributes: got %#v, want %#v", s.span.Attrs, want) + assert.Truef(t, eq(want, s.span.Links[0].Attrs), "new span link attributes: got %#v, want %#v", s.span.Attrs, want) + }) + } +} + func TestSpanTracerProvider(t *testing.T) { var s span @@ -484,6 +553,156 @@ func (b spanBuilder) Build() *span { return s } +func resetMaxSpan() (cleanup func()) { + orig := maxSpan + maxSpan = newSpanLimits() + return func() { maxSpan = orig } +} + +func TestTruncate(t *testing.T) { + type group struct { + limit int + input string + expected string + } + + tests := []struct { + name string + groups []group + }{ + // Edge case: limit is negative, no truncation should occur + { + name: "NoTruncation", + groups: []group{ + {-1, "No truncation!", "No truncation!"}, + }, + }, + + // Edge case: string is already shorter than the limit, no truncation + // should occur + { + name: "ShortText", + groups: []group{ + {10, "Short text", "Short text"}, + {15, "Short text", "Short text"}, + {100, "Short text", "Short text"}, + }, + }, + + // Edge case: truncation happens with ASCII characters only + { + name: "ASCIIOnly", + groups: []group{ + {1, "Hello World!", "H"}, + {5, "Hello World!", "Hello"}, + {12, "Hello World!", "Hello World!"}, + }, + }, + + // Truncation including multi-byte characters (UTF-8) + { + name: "ValidUTF-8", + groups: []group{ + {7, "Hello, 世界", "Hello, "}, + {8, "Hello, 世界", "Hello, 世"}, + {2, "こんにちは", "こん"}, + {3, "こんにちは", "こんに"}, + {5, "こんにちは", "こんにちは"}, + {12, "こんにちは", "こんにちは"}, + }, + }, + + // Truncation with invalid UTF-8 characters + { + name: "InvalidUTF-8", + groups: []group{ + {11, "Invalid\x80text", "Invalidtext"}, + // Do not modify invalid text if equal to limit. + {11, "Valid text\x80", "Valid text\x80"}, + // Do not modify invalid text if under limit. + {15, "Valid text\x80", "Valid text\x80"}, + {5, "Hello\x80World", "Hello"}, + {11, "Hello\x80World\x80!", "HelloWorld!"}, + {15, "Hello\x80World\x80Test", "HelloWorldTest"}, + {15, "Hello\x80\x80\x80World\x80Test", "HelloWorldTest"}, + {15, "\x80\x80\x80Hello\x80\x80\x80World\x80Test\x80\x80", "HelloWorldTest"}, + }, + }, + + // Truncation with mixed validn and invalid UTF-8 characters + { + name: "MixedUTF-8", + groups: []group{ + {6, "€"[0:2] + "hello€€", "hello€"}, + {6, "€" + "€"[0:2] + "hello", "€hello"}, + {11, "Valid text\x80📜", "Valid text📜"}, + {11, "Valid text📜\x80", "Valid text📜"}, + {14, "😊 Hello\x80World🌍🚀", "😊 HelloWorld🌍🚀"}, + {14, "😊\x80 Hello\x80World🌍🚀", "😊 HelloWorld🌍🚀"}, + {14, "😊\x80 Hello\x80World🌍\x80🚀", "😊 HelloWorld🌍🚀"}, + {14, "😊\x80 Hello\x80World🌍\x80🚀\x80", "😊 HelloWorld🌍🚀"}, + {14, "\x80😊\x80 Hello\x80World🌍\x80🚀\x80", "😊 HelloWorld🌍🚀"}, + }, + }, + + // Edge case: empty string, should return empty string + { + name: "Empty", + groups: []group{ + {5, "", ""}, + }, + }, + + // Edge case: limit is 0, should return an empty string + { + name: "Zero", + groups: []group{ + {0, "Some text", ""}, + {0, "", ""}, + }, + }, + } + + for _, tt := range tests { + for _, g := range tt.groups { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := truncate(g.limit, g.input) + assert.Equalf( + t, g.expected, got, + "input: %q([]rune%v))\ngot: %q([]rune%v)\nwant %q([]rune%v)", + g.input, []rune(g.input), + got, []rune(got), + g.expected, []rune(g.expected), + ) + }) + } + } +} + +func BenchmarkTruncate(b *testing.B) { + run := func(limit int, input string) func(b *testing.B) { + return func(b *testing.B) { + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + var out string + for pb.Next() { + out = truncate(limit, input) + } + _ = out + }) + } + } + b.Run("Unlimited", run(-1, "hello 😊 world 🌍🚀")) + b.Run("Zero", run(0, "Some text")) + b.Run("Short", run(10, "Short Text")) + b.Run("ASCII", run(5, "Hello, World!")) + b.Run("ValidUTF-8", run(10, "hello 😊 world 🌍🚀")) + b.Run("InvalidUTF-8", run(6, "€"[0:2]+"hello€€")) + b.Run("MixedUTF-8", run(14, "\x80😊\x80 Hello\x80World🌍\x80🚀\x80")) +} + func TestSpanConcurrentSafe(t *testing.T) { t.Parallel() From 326ce8c68756b07216440759697d7cdfa5d67cfd Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 24 Nov 2024 08:59:29 -0800 Subject: [PATCH 3/5] Add attribute limits --- sdk/span.go | 33 +++++++++++++++++++++++++++++++-- sdk/span_test.go | 38 ++++++++++++++++++++++++++++++++++++++ sdk/tracer.go | 2 +- 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/sdk/span.go b/sdk/span.go index e2683f8c4..90d6e6373 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -82,7 +82,12 @@ func (s *span) SetAttributes(attrs ...attribute.KeyValue) { s.mu.Lock() defer s.mu.Unlock() - // TODO: handle attribute limits. + limit := maxSpan.Attrs + if limit == 0 { + // No attributes allowed. + s.span.DroppedAttrs += uint32(len(attrs)) + return + } m := make(map[string]int) for i, a := range s.span.Attrs { @@ -92,6 +97,7 @@ func (s *span) SetAttributes(attrs ...attribute.KeyValue) { for _, a := range attrs { val := convAttrValue(a.Value) if val.Empty() { + s.span.DroppedAttrs++ continue } @@ -100,17 +106,40 @@ func (s *span) SetAttributes(attrs ...attribute.KeyValue) { Key: string(a.Key), Value: val, } - } else { + } else if limit < 0 || len(s.span.Attrs) < limit { s.span.Attrs = append(s.span.Attrs, telemetry.Attr{ Key: string(a.Key), Value: val, }) m[string(a.Key)] = len(s.span.Attrs) - 1 + } else { + s.span.DroppedAttrs++ } } } +// convCappedAttrs converts up to limit attrs into a []telemetry.Attr. The +// number of dropped attributes is also returned. +func convCappedAttrs(limit int, attrs []attribute.KeyValue) ([]telemetry.Attr, uint32) { + if limit == 0 { + return nil, uint32(len(attrs)) + } + + if limit < 0 { + // Unlimited. + return convAttrs(attrs), 0 + } + + limit = min(len(attrs), limit) + return convAttrs(attrs[:limit]), uint32(len(attrs) - limit) +} + func convAttrs(attrs []attribute.KeyValue) []telemetry.Attr { + if len(attrs) == 0 { + // Avoid allocations if not necessary. + return nil + } + out := make([]telemetry.Attr, 0, len(attrs)) for _, attr := range attrs { key := string(attr.Key) diff --git a/sdk/span_test.go b/sdk/span_test.go index b963ca076..515097560 100644 --- a/sdk/span_test.go +++ b/sdk/span_test.go @@ -525,6 +525,44 @@ func TestSpanAttributeValueLimits(t *testing.T) { } } +func TestSpanAttributeLimits(t *testing.T) { + tests := []struct { + limit int + want []telemetry.Attr + dropped uint32 + }{ + {0, nil, uint32(len(tAttrs))}, + {2, tAttrs[:2], uint32(len(tAttrs) - 2)}, + {len(tAttrs), tAttrs, 0}, + {-1, tAttrs, 0}, + } + + for _, test := range tests { + t.Run("Limit/"+strconv.Itoa(test.limit), func(t *testing.T) { + orig := maxSpan.Attrs + maxSpan.Attrs = test.limit + t.Cleanup(func() { maxSpan.Attrs = orig }) + + builder := spanBuilder{} + + s := builder.Build() + s.SetAttributes(attrs...) + assert.Equal(t, test.want, s.span.Attrs, "set span attributes") + assert.Equal(t, test.dropped, s.span.DroppedAttrs, "dropped attrs") + + s.SetAttributes(attrs...) + assert.Equal(t, test.want, s.span.Attrs, "set span attributes twice") + assert.Equal(t, 2*test.dropped, s.span.DroppedAttrs, "2x dropped attrs") + + builder.Options = []trace.SpanStartOption{trace.WithAttributes(attrs...)} + + s = builder.Build() + assert.Equal(t, test.want, s.span.Attrs, "new span attributes") + assert.Equal(t, test.dropped, s.span.DroppedAttrs, "dropped attrs") + }) + } +} + func TestSpanTracerProvider(t *testing.T) { var s span diff --git a/sdk/tracer.go b/sdk/tracer.go index 478141fff..b2c6ea62e 100644 --- a/sdk/tracer.go +++ b/sdk/tracer.go @@ -67,9 +67,9 @@ func (t tracer) traces(name string, cfg trace.SpanConfig, sc, psc trace.SpanCont ParentSpanID: telemetry.SpanID(psc.SpanID()), Name: name, Kind: spanKind(cfg.SpanKind()), - Attrs: convAttrs(cfg.Attributes()), Links: convLinks(cfg.Links()), } + span.Attrs, span.DroppedAttrs = convCappedAttrs(maxSpan.Attrs, cfg.Attributes()) if t := cfg.Timestamp(); !t.IsZero() { span.StartTime = cfg.Timestamp() From 1fcc86ecf8c7c6d173832b146db2f964f0ec1382 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 24 Nov 2024 08:59:55 -0800 Subject: [PATCH 4/5] Add link limits --- sdk/span.go | 20 +++++++++++-- sdk/span_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++---- sdk/tracer.go | 14 ++++++++- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/sdk/span.go b/sdk/span.go index 90d6e6373..50b4b4c0b 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -367,10 +367,22 @@ func (s *span) AddLink(link trace.Link) { return } + l := maxSpan.Links + s.mu.Lock() defer s.mu.Unlock() - // TODO: handle link limits. + if l == 0 { + s.span.DroppedLinks++ + return + } + + if l > 0 && len(s.span.Links) == l { + // Drop head while avoiding allocation of more capacity. + copy(s.span.Links[:l-1], s.span.Links[1:]) + s.span.Links = s.span.Links[:l-1] + s.span.DroppedLinks++ + } s.span.Links = append(s.span.Links, convLink(link)) } @@ -384,13 +396,15 @@ func convLinks(links []trace.Link) []*telemetry.SpanLink { } func convLink(link trace.Link) *telemetry.SpanLink { - return &telemetry.SpanLink{ + l := &telemetry.SpanLink{ TraceID: telemetry.TraceID(link.SpanContext.TraceID()), SpanID: telemetry.SpanID(link.SpanContext.SpanID()), TraceState: link.SpanContext.TraceState().String(), - Attrs: convAttrs(link.Attributes), Flags: uint32(link.SpanContext.TraceFlags()), } + l.Attrs, l.DroppedAttrs = convCappedAttrs(maxSpan.LinkAttrs, link.Attributes) + + return l } func (s *span) SetName(name string) { diff --git a/sdk/span_test.go b/sdk/span_test.go index 515097560..61e450da1 100644 --- a/sdk/span_test.go +++ b/sdk/span_test.go @@ -354,6 +354,78 @@ func TestSpanAddLink(t *testing.T) { assert.Equal(t, want, s.span.Links) } +func TestSpanAddLinkLimit(t *testing.T) { + tests := []struct { + limit int + want []*telemetry.SpanLink + dropped uint32 + }{ + {0, nil, 2}, + {1, []*telemetry.SpanLink{tLink1}, 1}, + {2, []*telemetry.SpanLink{tLink0, tLink1}, 0}, + {-1, []*telemetry.SpanLink{tLink0, tLink1}, 0}, + } + + for _, test := range tests { + t.Run("Limit/"+strconv.Itoa(test.limit), func(t *testing.T) { + orig := maxSpan.Links + maxSpan.Links = test.limit + t.Cleanup(func() { maxSpan.Links = orig }) + + builder := spanBuilder{} + s := builder.Build() + s.AddLink(link0) + s.AddLink(link1) + assert.Equal(t, test.want, s.span.Links, "AddLink") + assert.Equal(t, test.dropped, s.span.DroppedLinks, "AddLink DroppedLinks") + + builder.Options = []trace.SpanStartOption{ + trace.WithLinks(link0, link1), + } + s = builder.Build() + assert.Equal(t, test.want, s.span.Links, "NewSpan") + assert.Equal(t, test.dropped, s.span.DroppedLinks, "NewSpan DroppedLinks") + }) + } +} + +func TestSpanLinkAttrLimit(t *testing.T) { + tests := []struct { + limit int + want []telemetry.Attr + dropped uint32 + }{ + {0, nil, uint32(len(tAttrs))}, + {2, tAttrs[:2], uint32(len(tAttrs) - 2)}, + {len(tAttrs), tAttrs, 0}, + {-1, tAttrs, 0}, + } + + link := trace.Link{Attributes: attrs} + for _, test := range tests { + t.Run("Limit/"+strconv.Itoa(test.limit), func(t *testing.T) { + orig := maxSpan.LinkAttrs + maxSpan.LinkAttrs = test.limit + t.Cleanup(func() { maxSpan.LinkAttrs = orig }) + + builder := spanBuilder{} + + s := builder.Build() + s.AddLink(link) + + require.Len(t, s.span.Links, 1) + got := s.span.Links[0] + assert.Equal(t, test.want, got.Attrs, "AddLink attrs") + assert.Equal(t, test.dropped, got.DroppedAttrs, "dropped AddLink attrs") + + builder.Options = []trace.SpanStartOption{trace.WithLinks(link)} + s = builder.Build() + assert.Equal(t, test.want, got.Attrs, "NewSpan link attrs") + assert.Equal(t, test.dropped, got.DroppedAttrs, "dropped NewSpan link attrs") + }) + } +} + func TestSpanIsRecording(t *testing.T) { builder := spanBuilder{} s := builder.Build() @@ -591,12 +663,6 @@ func (b spanBuilder) Build() *span { return s } -func resetMaxSpan() (cleanup func()) { - orig := maxSpan - maxSpan = newSpanLimits() - return func() { maxSpan = orig } -} - func TestTruncate(t *testing.T) { type group struct { limit int diff --git a/sdk/tracer.go b/sdk/tracer.go index b2c6ea62e..cbcfabde3 100644 --- a/sdk/tracer.go +++ b/sdk/tracer.go @@ -67,10 +67,22 @@ func (t tracer) traces(name string, cfg trace.SpanConfig, sc, psc trace.SpanCont ParentSpanID: telemetry.SpanID(psc.SpanID()), Name: name, Kind: spanKind(cfg.SpanKind()), - Links: convLinks(cfg.Links()), } + span.Attrs, span.DroppedAttrs = convCappedAttrs(maxSpan.Attrs, cfg.Attributes()) + links := cfg.Links() + if limit := maxSpan.Links; limit == 0 { + span.DroppedLinks = uint32(len(links)) + } else { + if limit > 0 { + n := max(len(links)-limit, 0) + span.DroppedLinks = uint32(n) + links = links[n:] + } + span.Links = convLinks(links) + } + if t := cfg.Timestamp(); !t.IsZero() { span.StartTime = cfg.Timestamp() } else { From 49f5bbadd6769d90865031e04fff55ca805cc626 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 25 Nov 2024 09:07:22 -0800 Subject: [PATCH 5/5] Add span event limits --- sdk/span.go | 23 ++++++++++++---- sdk/span_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/sdk/span.go b/sdk/span.go index 50b4b4c0b..6ebea12a9 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -353,13 +353,24 @@ func (s *span) AddEvent(name string, opts ...trace.EventOption) { // addEvent adds an event with name and attrs at tStamp to the span. The span // lock (s.mu) needs to be held by the caller. func (s *span) addEvent(name string, tStamp time.Time, attrs []attribute.KeyValue) { - // TODO: handle event limits. + limit := maxSpan.Events - s.span.Events = append(s.span.Events, &telemetry.SpanEvent{ - Time: tStamp, - Name: name, - Attrs: convAttrs(attrs), - }) + if limit == 0 { + s.span.DroppedEvents++ + return + } + + if limit > 0 && len(s.span.Events) == limit { + // Drop head while avoiding allocation of more capacity. + copy(s.span.Events[:limit-1], s.span.Events[1:]) + s.span.Events = s.span.Events[:limit-1] + s.span.DroppedEvents++ + } + + e := &telemetry.SpanEvent{Time: tStamp, Name: name} + e.Attrs, e.DroppedAttrs = convCappedAttrs(maxSpan.EventAttrs, attrs) + + s.span.Events = append(s.span.Events, e) } func (s *span) AddLink(link trace.Link) { diff --git a/sdk/span_test.go b/sdk/span_test.go index 61e450da1..6e9f8a452 100644 --- a/sdk/span_test.go +++ b/sdk/span_test.go @@ -474,6 +474,77 @@ func TestSpanRecordError(t *testing.T) { assert.True(t, hasST, "missing stacktrace attribute") } +func TestAddEventLimit(t *testing.T) { + const a, b, c = "a", "b", "c" + + ts := time.Now() + + evtA := &telemetry.SpanEvent{Name: "a", Time: ts} + evtB := &telemetry.SpanEvent{Name: "b", Time: ts} + evtC := &telemetry.SpanEvent{Name: "c", Time: ts} + + tests := []struct { + limit int + want []*telemetry.SpanEvent + dropped uint32 + }{ + {0, nil, 3}, + {1, []*telemetry.SpanEvent{evtC}, 2}, + {2, []*telemetry.SpanEvent{evtB, evtC}, 1}, + {3, []*telemetry.SpanEvent{evtA, evtB, evtC}, 0}, + {-1, []*telemetry.SpanEvent{evtA, evtB, evtC}, 0}, + } + + for _, test := range tests { + t.Run("Limit/"+strconv.Itoa(test.limit), func(t *testing.T) { + orig := maxSpan.Events + maxSpan.Events = test.limit + t.Cleanup(func() { maxSpan.Events = orig }) + + builder := spanBuilder{} + + s := builder.Build() + s.addEvent(a, ts, nil) + s.addEvent(b, ts, nil) + s.addEvent(c, ts, nil) + + assert.Equal(t, test.want, s.span.Events, "add event") + assert.Equal(t, test.dropped, s.span.DroppedEvents, "dropped events") + }) + } +} + +func TestAddEventAttrLimit(t *testing.T) { + tests := []struct { + limit int + want []telemetry.Attr + dropped uint32 + }{ + {0, nil, uint32(len(tAttrs))}, + {2, tAttrs[:2], uint32(len(tAttrs) - 2)}, + {len(tAttrs), tAttrs, 0}, + {-1, tAttrs, 0}, + } + + for _, test := range tests { + t.Run("Limit/"+strconv.Itoa(test.limit), func(t *testing.T) { + orig := maxSpan.EventAttrs + maxSpan.EventAttrs = test.limit + t.Cleanup(func() { maxSpan.EventAttrs = orig }) + + builder := spanBuilder{} + + s := builder.Build() + s.addEvent("name", time.Now(), attrs) + + require.Len(t, s.span.Events, 1) + got := s.span.Events[0] + assert.Equal(t, test.want, got.Attrs, "event attrs") + assert.Equal(t, test.dropped, got.DroppedAttrs, "dropped event attrs") + }) + } +} + func TestSpanSpanContext(t *testing.T) { s := spanBuilder{SpanContext: spanContext0}.Build() assert.Equal(t, spanContext0, s.SpanContext())