From cd337895d74efc903fc4fc9898a33dd9af08d9c8 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 18 Nov 2024 16:39:44 -0800 Subject: [PATCH 1/6] 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 59ff11bba62a4c2db3a4e3a21432e7fa11e3b043 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Sun, 24 Nov 2024 08:59:29 -0800 Subject: [PATCH 2/6] Add attribute limits --- sdk/span.go | 33 +++++++++++++++++++++++++++++++-- sdk/span_test.go | 38 ++++++++++++++++++++++++++++++++++++++ sdk/tracer.go | 5 ++++- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/sdk/span.go b/sdk/span.go index d8f5786e0..94cad91e8 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -80,7 +80,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 { @@ -90,6 +95,7 @@ func (s *span) SetAttributes(attrs ...attribute.KeyValue) { for _, a := range attrs { val := convAttrValue(a.Value) if val.Empty() { + s.span.DroppedAttrs++ continue } @@ -98,17 +104,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, int) { + if limit == 0 { + return nil, len(attrs) + } + + if limit < 0 { + // Unlimited. + return convAttrs(attrs), 0 + } + + limit = min(len(attrs), limit) + return convAttrs(attrs[:limit]), 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 00d3a272d..e855aa4b4 100644 --- a/sdk/span_test.go +++ b/sdk/span_test.go @@ -456,6 +456,44 @@ func TestSpanSetAttributes(t *testing.T) { assert.Equal(t, tAttrs, s.span.Attrs, "SpanAttributes did not override") } +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..cb6121bf5 100644 --- a/sdk/tracer.go +++ b/sdk/tracer.go @@ -67,10 +67,13 @@ 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()), } + var dropped int + span.Attrs, dropped = convCappedAttrs(maxSpan.Attrs, cfg.Attributes()) + span.DroppedAttrs = uint32(dropped) + if t := cfg.Timestamp(); !t.IsZero() { span.StartTime = cfg.Timestamp() } else { From a90224d9b4f96d771c19419da47265bc8d6f35d6 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 25 Nov 2024 08:08:09 -0800 Subject: [PATCH 3/6] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c93f3179e..4b216387d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ## [Unreleased] +### Added + +- Add support for span attribute limits to `go.opentelemtry.io/auto/sdk`. ([#1315](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/1315)) + ## [v0.18.0-alpha] - 2024-11-20 ### Changed From 590176fd82dd37d063bb30c8424a6b688c7b3d75 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 25 Nov 2024 09:11:04 -0800 Subject: [PATCH 4/6] Change convCappedAttrs return drop count to uint32 --- sdk/span.go | 6 +++--- sdk/tracer.go | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/sdk/span.go b/sdk/span.go index 94cad91e8..e81a13014 100644 --- a/sdk/span.go +++ b/sdk/span.go @@ -118,9 +118,9 @@ func (s *span) SetAttributes(attrs ...attribute.KeyValue) { // 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, int) { +func convCappedAttrs(limit int, attrs []attribute.KeyValue) ([]telemetry.Attr, uint32) { if limit == 0 { - return nil, len(attrs) + return nil, uint32(len(attrs)) } if limit < 0 { @@ -129,7 +129,7 @@ func convCappedAttrs(limit int, attrs []attribute.KeyValue) ([]telemetry.Attr, i } limit = min(len(attrs), limit) - return convAttrs(attrs[:limit]), len(attrs) - limit + return convAttrs(attrs[:limit]), uint32(len(attrs) - limit) } func convAttrs(attrs []attribute.KeyValue) []telemetry.Attr { diff --git a/sdk/tracer.go b/sdk/tracer.go index cb6121bf5..b2c6ea62e 100644 --- a/sdk/tracer.go +++ b/sdk/tracer.go @@ -69,10 +69,7 @@ func (t tracer) traces(name string, cfg trace.SpanConfig, sc, psc trace.SpanCont Kind: spanKind(cfg.SpanKind()), Links: convLinks(cfg.Links()), } - - var dropped int - span.Attrs, dropped = convCappedAttrs(maxSpan.Attrs, cfg.Attributes()) - span.DroppedAttrs = uint32(dropped) + span.Attrs, span.DroppedAttrs = convCappedAttrs(maxSpan.Attrs, cfg.Attributes()) if t := cfg.Timestamp(); !t.IsZero() { span.StartTime = cfg.Timestamp() From 4bf211efd42982636f791ee2cdb941c3e3e5ecd1 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 26 Nov 2024 08:27:26 -0800 Subject: [PATCH 5/6] Replace tests with table test --- sdk/limit_test.go | 135 ++++++++++++++++++++++++---------------------- 1 file changed, 71 insertions(+), 64 deletions(-) diff --git a/sdk/limit_test.go b/sdk/limit_test.go index ec63dd405..8057f4da6 100644 --- a/sdk/limit_test.go +++ b/sdk/limit_test.go @@ -9,73 +9,80 @@ import ( "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 TestSpanLimit(t *testing.T) { + tests := []struct { + name string + get func(spanLimits) int + zero int + keys []string + }{ + { + name: "AttributeValueLengthLimit", + get: func(sl spanLimits) int { return sl.AttrValueLen }, + zero: -1, + keys: []string{ + "OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", + "OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", + }, + }, + { + name: "AttributeCountLimit", + get: func(sl spanLimits) int { return sl.Attrs }, + zero: 128, + keys: []string{ + "OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", + "OTEL_ATTRIBUTE_COUNT_LIMIT", + }, + }, + { + name: "EventCountLimit", + get: func(sl spanLimits) int { return sl.Events }, + zero: 128, + keys: []string{"OTEL_SPAN_EVENT_COUNT_LIMIT"}, + }, + { + name: "EventAttributeCountLimit", + get: func(sl spanLimits) int { return sl.EventAttrs }, + zero: 128, + keys: []string{"OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT"}, + }, + { + name: "LinkCountLimit", + get: func(sl spanLimits) int { return sl.Links }, + zero: 128, + keys: []string{"OTEL_SPAN_LINK_COUNT_LIMIT"}, + }, + { + name: "LinkAttributeCountLimit", + get: func(sl spanLimits) int { return sl.LinkAttrs }, + zero: 128, + keys: []string{"OTEL_LINK_ATTRIBUTE_COUNT_LIMIT"}, + }, + } -func testLimit(t *testing.T, f func(spanLimits) int, zero int, keys ...string) { - t.Helper() + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Run("Default", func(t *testing.T) { + assert.Equal(t, test.zero, test.get(newSpanLimits())) + }) - t.Run("Default", func(t *testing.T) { - assert.Equal(t, zero, f(newSpanLimits())) - }) + t.Run("ValidValue", func(t *testing.T) { + for _, key := range test.keys { + t.Run(key, func(t *testing.T) { + t.Setenv(key, "43") + assert.Equal(t, 43, test.get(newSpanLimits())) + }) + } + }) - for _, key := range keys { - t.Run(key, func(t *testing.T) { - t.Setenv(key, "43") - assert.Equal(t, 43, f(newSpanLimits())) + t.Run("InvalidValue", func(t *testing.T) { + for _, key := range test.keys { + t.Run(key, func(t *testing.T) { + t.Setenv(key, "invalid int value.") + assert.Equal(t, test.zero, test.get(newSpanLimits())) + }) + } + }) }) } } From d0c218221ad2cac14a00660c87986c15ee881cda Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 26 Nov 2024 13:18:33 -0800 Subject: [PATCH 6/6] Log failure to parse env var --- sdk/limit.go | 7 +++++++ sdk/limit_test.go | 22 +++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sdk/limit.go b/sdk/limit.go index caae4a2d9..86babf1a8 100644 --- a/sdk/limit.go +++ b/sdk/limit.go @@ -4,6 +4,7 @@ package sdk import ( + "log/slog" "os" "strconv" ) @@ -81,6 +82,12 @@ func firstEnv(defaultVal int, keys ...string) int { if err == nil { return v } + slog.Warn( + "invalid limit environment variable", + "error", err, + "key", key, + "value", strV, + ) } return defaultVal diff --git a/sdk/limit_test.go b/sdk/limit_test.go index 8057f4da6..448651f2a 100644 --- a/sdk/limit_test.go +++ b/sdk/limit_test.go @@ -4,9 +4,13 @@ package sdk import ( + "bytes" + "encoding/json" + "log/slog" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSpanLimit(t *testing.T) { @@ -78,8 +82,24 @@ func TestSpanLimit(t *testing.T) { t.Run("InvalidValue", func(t *testing.T) { for _, key := range test.keys { t.Run(key, func(t *testing.T) { - t.Setenv(key, "invalid int value.") + orig := slog.Default() + t.Cleanup(func() { slog.SetDefault(orig) }) + + var buf bytes.Buffer + h := slog.NewJSONHandler(&buf, &slog.HandlerOptions{}) + slog.SetDefault(slog.New(h)) + + const value = "invalid int value." + t.Setenv(key, value) assert.Equal(t, test.zero, test.get(newSpanLimits())) + + var data map[string]any + require.NoError(t, json.NewDecoder(&buf).Decode(&data)) + assert.Equal(t, "invalid limit environment variable", data["msg"], "log message") + assert.Equal(t, "WARN", data["level"], "logged level") + assert.Equal(t, key, data["key"], "logged key") + assert.Equal(t, value, data["value"], "logged value") + assert.NotEmpty(t, data["error"], "logged error") }) } })