From da4cc1f647f111c0b15be2e5147cf0c0c4d368c8 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sat, 13 Feb 2021 13:16:56 +0800 Subject: [PATCH 1/9] Change the default span limit values to 128 --- sdk/trace/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/trace/config.go b/sdk/trace/config.go index 42b1851f833..f3e429e85fd 100644 --- a/sdk/trace/config.go +++ b/sdk/trace/config.go @@ -41,11 +41,11 @@ type Config struct { const ( // DefaultMaxEventsPerSpan is default max number of message events per span - DefaultMaxEventsPerSpan = 1000 + DefaultMaxEventsPerSpan = 128 // DefaultMaxAttributesPerSpan is default max number of attributes per span - DefaultMaxAttributesPerSpan = 1000 + DefaultMaxAttributesPerSpan = 128 // DefaultMaxLinksPerSpan is default max number of links per span - DefaultMaxLinksPerSpan = 1000 + DefaultMaxLinksPerSpan = 128 ) From a0c7fa89d2b0a4dea9c16c827b316ed752bc2944 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 14 Feb 2021 11:21:41 +0800 Subject: [PATCH 2/9] Rename and move MaxEventsPerSpan, MaxAttributesPerSpan, MaxLinksPerSpan into SpanLimits --- sdk/trace/config.go | 46 ++++++++++++++++++++++++++++------------- sdk/trace/provider.go | 26 +++++++++++++---------- sdk/trace/span.go | 6 +++--- sdk/trace/trace_test.go | 6 +++--- 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/sdk/trace/config.go b/sdk/trace/config.go index f3e429e85fd..44f4dfacbd7 100644 --- a/sdk/trace/config.go +++ b/sdk/trace/config.go @@ -26,26 +26,44 @@ type Config struct { // IDGenerator is for internal use only. IDGenerator IDGenerator - // MaxEventsPerSpan is max number of message events per span - MaxEventsPerSpan int - - // MaxAnnotationEventsPerSpan is max number of attributes per span - MaxAttributesPerSpan int - - // MaxLinksPerSpan is max number of links per span - MaxLinksPerSpan int + // SpanLimits used to limit the number of attributes, events and links to a span. + SpanLimits SpanLimits // Resource contains attributes representing an entity that produces telemetry. Resource *resource.Resource } +// SpanLimits represents the limit to a span. +type SpanLimits struct { + // AttributeCountLimit is maximum allowed span attribute count. + AttributeCountLimit int + + // EventCountLimit is maximum allowed span event count. + EventCountLimit int + + // LinkCountLimit is maximum allowed span link count. + LinkCountLimit int + + // AttributePerEventCountLimit is maximum allowed attribute per span event count. + AttributePerEventCountLimit int + + // AttributePerLinkCountLimit is maximum allowed attribute per span link count. + AttributePerLinkCountLimit int +} + const ( - // DefaultMaxEventsPerSpan is default max number of message events per span - DefaultMaxEventsPerSpan = 128 + // DefaultAttributeCountLimit is default maximum allowed span attribute count. + DefaultAttributeCountLimit = 128 + + // DefaultEventCountLimit is default maximum allowed span event count. + DefaultEventCountLimit = 128 + + // DefaultLinkCountLimit is default maximum allowed span link count. + DefaultLinkCountLimit = 128 - // DefaultMaxAttributesPerSpan is default max number of attributes per span - DefaultMaxAttributesPerSpan = 128 + // DefaultAttributePerEventCountLimit is default maximum allowed attribute per span event count. + DefaultAttributePerEventCountLimit = 128 - // DefaultMaxLinksPerSpan is default max number of links per span - DefaultMaxLinksPerSpan = 128 + // DefaultAttributePerLinkCountLimit is default maximum allowed attribute per span link count. + DefaultAttributePerLinkCountLimit = 128 ) diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 901e8d4a76d..e0af5b2248b 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -65,11 +65,15 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider { namedTracer: make(map[instrumentation.Library]*tracer), } tp.config.Store(&Config{ - DefaultSampler: ParentBased(AlwaysSample()), - IDGenerator: defaultIDGenerator(), - MaxAttributesPerSpan: DefaultMaxAttributesPerSpan, - MaxEventsPerSpan: DefaultMaxEventsPerSpan, - MaxLinksPerSpan: DefaultMaxLinksPerSpan, + DefaultSampler: ParentBased(AlwaysSample()), + IDGenerator: defaultIDGenerator(), + SpanLimits: SpanLimits{ + AttributeCountLimit: DefaultAttributeCountLimit, + EventCountLimit: DefaultEventCountLimit, + LinkCountLimit: DefaultLinkCountLimit, + AttributePerEventCountLimit: DefaultAttributePerEventCountLimit, + AttributePerLinkCountLimit: DefaultAttributePerLinkCountLimit, + }, }) for _, sp := range o.processors { @@ -170,14 +174,14 @@ func (p *TracerProvider) ApplyConfig(cfg Config) { if cfg.IDGenerator != nil { c.IDGenerator = cfg.IDGenerator } - if cfg.MaxEventsPerSpan > 0 { - c.MaxEventsPerSpan = cfg.MaxEventsPerSpan + if cfg.SpanLimits.EventCountLimit > 0 { + c.SpanLimits.EventCountLimit = cfg.SpanLimits.EventCountLimit } - if cfg.MaxAttributesPerSpan > 0 { - c.MaxAttributesPerSpan = cfg.MaxAttributesPerSpan + if cfg.SpanLimits.AttributeCountLimit > 0 { + c.SpanLimits.AttributeCountLimit = cfg.SpanLimits.AttributeCountLimit } - if cfg.MaxLinksPerSpan > 0 { - c.MaxLinksPerSpan = cfg.MaxLinksPerSpan + if cfg.SpanLimits.LinkCountLimit > 0 { + c.SpanLimits.LinkCountLimit = cfg.SpanLimits.LinkCountLimit } if cfg.Resource != nil { c.Resource = cfg.Resource diff --git a/sdk/trace/span.go b/sdk/trace/span.go index b09b4fa2b1b..52cf9523aec 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -481,9 +481,9 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac span.spanContext.SpanID = cfg.IDGenerator.NewSpanID(ctx, parent.TraceID) } - span.attributes = newAttributesMap(cfg.MaxAttributesPerSpan) - span.messageEvents = newEvictedQueue(cfg.MaxEventsPerSpan) - span.links = newEvictedQueue(cfg.MaxLinksPerSpan) + span.attributes = newAttributesMap(cfg.SpanLimits.AttributeCountLimit) + span.messageEvents = newEvictedQueue(cfg.SpanLimits.EventCountLimit) + span.links = newEvictedQueue(cfg.SpanLimits.LinkCountLimit) data := samplingData{ noParent: hasEmptySpanContext(parent), diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index fb73b2ef0f1..16ba5bd1e3e 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -482,7 +482,7 @@ func TestSamplerAttributesLocalChildSpan(t *testing.T) { func TestSetSpanAttributesOverLimit(t *testing.T) { te := NewTestExporter() - cfg := Config{MaxAttributesPerSpan: 2} + cfg := Config{SpanLimits: SpanLimits{AttributeCountLimit: 2}} tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) span := startSpan(tp, "SpanAttributesOverLimit") @@ -565,7 +565,7 @@ func TestEvents(t *testing.T) { func TestEventsOverLimit(t *testing.T) { te := NewTestExporter() - cfg := Config{MaxEventsPerSpan: 2} + cfg := Config{SpanLimits: SpanLimits{EventCountLimit: 2}} tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) span := startSpan(tp, "EventsOverLimit") @@ -656,7 +656,7 @@ func TestLinks(t *testing.T) { func TestLinksOverLimit(t *testing.T) { te := NewTestExporter() - cfg := Config{MaxLinksPerSpan: 2} + cfg := Config{SpanLimits: SpanLimits{LinkCountLimit: 2}} sc1 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}} sc2 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}} From 82aec7dd454ac91436fc0c64c452f815250de40a Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 14 Feb 2021 17:15:12 +0800 Subject: [PATCH 3/9] Add AttributePerEventCountLimit and AttributePerLinkCountLimit --- sdk/trace/provider.go | 6 +++ sdk/trace/span.go | 20 +++++++- sdk/trace/trace_test.go | 108 +++++++++++++++++++++++++++++++++++++++- sdk/trace/tracer.go | 17 +++++-- 4 files changed, 144 insertions(+), 7 deletions(-) diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index e0af5b2248b..d1230e7910f 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -183,6 +183,12 @@ func (p *TracerProvider) ApplyConfig(cfg Config) { if cfg.SpanLimits.LinkCountLimit > 0 { c.SpanLimits.LinkCountLimit = cfg.SpanLimits.LinkCountLimit } + if cfg.SpanLimits.AttributePerEventCountLimit > 0 { + c.SpanLimits.AttributePerEventCountLimit = cfg.SpanLimits.AttributePerEventCountLimit + } + if cfg.SpanLimits.AttributePerLinkCountLimit > 0 { + c.SpanLimits.AttributePerLinkCountLimit = cfg.SpanLimits.AttributePerLinkCountLimit + } if cfg.Resource != nil { c.Resource = cfg.Resource } diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 52cf9523aec..ff8c7351f04 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -19,6 +19,7 @@ import ( "fmt" "reflect" "sync" + "sync/atomic" "time" "go.opentelemetry.io/otel/codes" @@ -133,6 +134,11 @@ type span struct { // tracer is the SDK tracer that created this span. tracer *tracer + + // spanLimits holds the limit to this span. + spanLimits SpanLimits + + droppedAttributeCount int32 } var _ trace.Span = &span{} @@ -264,6 +270,12 @@ func (s *span) AddEvent(name string, o ...trace.EventOption) { func (s *span) addEvent(name string, o ...trace.EventOption) { c := trace.NewEventConfig(o...) + // Discard over limited attributes + if len(c.Attributes) > s.spanLimits.AttributePerEventCountLimit { + s.addDroppedAttributeCount(len(c.Attributes) - s.spanLimits.AttributePerEventCountLimit) + c.Attributes = c.Attributes[:s.spanLimits.AttributePerEventCountLimit] + } + s.mu.Lock() defer s.mu.Unlock() s.messageEvents.add(trace.Event{ @@ -417,9 +429,10 @@ func (s *span) Snapshot() *export.SpanSnapshot { sd.StatusCode = s.statusCode sd.StatusMessage = s.statusMessage + sd.DroppedAttributeCount = int(s.droppedAttributeCount) if s.attributes.evictList.Len() > 0 { sd.Attributes = s.attributes.toKeyValue() - sd.DroppedAttributeCount = s.attributes.droppedCount + sd.DroppedAttributeCount += s.attributes.droppedCount } if len(s.messageEvents.queue) > 0 { sd.MessageEvents = s.interfaceArrayToMessageEventArray() @@ -467,6 +480,10 @@ func (s *span) addChild() { s.mu.Unlock() } +func (s *span) addDroppedAttributeCount(delta int) { + atomic.AddInt32(&s.droppedAttributeCount, int32(delta)) +} + func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trace.SpanContext, remoteParent bool, o *trace.SpanConfig) *span { span := &span{} span.spanContext = parent @@ -484,6 +501,7 @@ func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trac span.attributes = newAttributesMap(cfg.SpanLimits.AttributeCountLimit) span.messageEvents = newEvictedQueue(cfg.SpanLimits.EventCountLimit) span.links = newEvictedQueue(cfg.SpanLimits.LinkCountLimit) + span.spanLimits = cfg.SpanLimits data := samplingData{ noParent: hasEmptySpanContext(parent), diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 16ba5bd1e3e..b14c1f7f5bc 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1024,7 +1024,7 @@ func TestExecutionTracerTaskEnd(t *testing.T) { s.executionTracerTaskEnd = executionTracerTaskEnd spans = append(spans, s) // parent not sampled - //tp.ApplyConfig(Config{DefaultSampler: AlwaysSample()}) + // tp.ApplyConfig(Config{DefaultSampler: AlwaysSample()}) _, apiSpan = tr.Start(context.Background(), "foo") s = apiSpan.(*span) s.executionTracerTaskEnd = executionTracerTaskEnd @@ -1396,3 +1396,109 @@ func TestReadWriteSpan(t *testing.T) { // available via ReadWriteSpan as doing so would mean creating a lot of // duplication. } + +func TestAddSpanEventWithOverLimitedAttributes(t *testing.T) { + te := NewTestExporter() + cfg := Config{SpanLimits: SpanLimits{AttributePerEventCountLimit: 2}} + tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) + + span := startSpan(tp, "AddSpanEventWithOverLimitedAttributes") + span.AddEvent("test1", trace.WithAttributes( + label.Bool("key1", true), + label.String("key2", "value2"), + )) + // Parts of the attribute should be discard + span.AddEvent("test2", trace.WithAttributes( + label.Bool("key1", true), + label.String("key2", "value2"), + label.String("key3", "value3"), + label.String("key4", "value4"), + )) + got, err := endSpan(te, span) + if err != nil { + t.Fatal(err) + } + + for i := range got.MessageEvents { + if !checkTime(&got.MessageEvents[i].Time) { + t.Error("exporting span: expected nonzero Event Time") + } + } + + want := &export.SpanSnapshot{ + SpanContext: trace.SpanContext{ + TraceID: tid, + TraceFlags: 0x1, + }, + ParentSpanID: sid, + Name: "span0", + Attributes: nil, + MessageEvents: []trace.Event{ + { + Name: "test1", + Attributes: []label.KeyValue{ + label.Bool("key1", true), + label.String("key2", "value2"), + }, + }, + { + Name: "test2", + Attributes: []label.KeyValue{ + label.Bool("key1", true), + label.String("key2", "value2"), + }, + }, + }, + SpanKind: trace.SpanKindInternal, + HasRemoteParent: true, + DroppedAttributeCount: 2, + InstrumentationLibrary: instrumentation.Library{Name: "AddSpanEventWithOverLimitedAttributes"}, + } + if diff := cmpDiff(got, want); diff != "" { + t.Errorf("SetSpanAttributesOverLimit: -got +want %s", diff) + } +} + +func TestWithLinksWithOverLimitedAttributes(t *testing.T) { + te := NewTestExporter() + cfg := Config{SpanLimits: SpanLimits{AttributePerLinkCountLimit: 1}} + tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) + + k1v1 := label.String("key1", "value1") + k2v2 := label.String("key2", "value2") + k3v3 := label.String("key3", "value3") + k4v4 := label.String("key4", "value4") + + sc1 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}} + sc2 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}} + + span := startSpan(tp, "Links", trace.WithLinks([]trace.Link{ + {SpanContext: sc1, Attributes: []label.KeyValue{k1v1, k2v2}}, + {SpanContext: sc2, Attributes: []label.KeyValue{k2v2, k3v3, k4v4}}, + }...)) + + got, err := endSpan(te, span) + if err != nil { + t.Fatal(err) + } + + want := &export.SpanSnapshot{ + SpanContext: trace.SpanContext{ + TraceID: tid, + TraceFlags: 0x1, + }, + ParentSpanID: sid, + Name: "span0", + HasRemoteParent: true, + Links: []trace.Link{ + {SpanContext: sc1, Attributes: []label.KeyValue{k1v1}}, + {SpanContext: sc2, Attributes: []label.KeyValue{k2v2}}, + }, + DroppedAttributeCount: 3, + SpanKind: trace.SpanKindInternal, + InstrumentationLibrary: instrumentation.Library{Name: "Links"}, + } + if diff := cmpDiff(got, want); diff != "" { + t.Errorf("Link: -got +want %s", diff) + } +} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 8bc5109d977..3f6bd66d373 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -38,9 +38,10 @@ var _ trace.Tracer = &tracer{} // configured appropriately by any SpanOption passed. Any Timestamp option // passed will be used as the start time of the Span's life-cycle. func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanOption) (context.Context, trace.Span) { - config := trace.NewSpanConfig(options...) + spanConfig := trace.NewSpanConfig(options...) + cfg := tr.provider.config.Load().(*Config) - parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, config.NewRoot) + parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, spanConfig.NewRoot) if p := trace.SpanFromContext(ctx); p != nil { if sdkSpan, ok := p.(*span); ok { @@ -48,14 +49,20 @@ func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanO } } - span := startSpanInternal(ctx, tr, name, parentSpanContext, remoteParent, config) + span := startSpanInternal(ctx, tr, name, parentSpanContext, remoteParent, spanConfig) for _, l := range links { span.addLink(l) } - for _, l := range config.Links { + for _, l := range spanConfig.Links { + // Discard over limited attributes + if len(l.Attributes) > cfg.SpanLimits.AttributePerLinkCountLimit { + span.addDroppedAttributeCount(len(l.Attributes) - cfg.SpanLimits.AttributePerLinkCountLimit) + l.Attributes = l.Attributes[:cfg.SpanLimits.AttributePerLinkCountLimit] + } + span.addLink(l) } - span.SetAttributes(config.Attributes...) + span.SetAttributes(spanConfig.Attributes...) span.tracer = tr From 6e3c8df536761ec4774c4c65d9204e50b415ee55 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 14 Feb 2021 20:01:58 +0800 Subject: [PATCH 4/9] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b830a131be..70cdce08290 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- AttributePerEventCountLimit and AttributePerLinkCountLimit for SpanLimits. (#1535) + ### Changed - Rename project default branch from `master` to `main`. @@ -15,6 +19,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add tooling to maintain "replace" directives in go.mod files automatically. (#1528) - Create new modules: otel/metric, otel/trace, otel/oteltest, otel/sdk/export/metric, otel/sdk/metric (#1528) - Move metric-related public APIs from otel to otel/metric/global. (#1528) +- Default span limit values to 128. (#1535) +- Rename MaxEventsPerSpan, MaxAttributesPerSpan and MaxLinksPerSpan to EventCountLimit, AttributeCountLimit and LinkCountLimit, and move these fieds into SpanLimits. (#1535) ## [0.16.0] - 2020-01-13 From 404f16b175f3c30090c847a6a0ea0c1fc6250751 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 18 Feb 2021 10:32:59 +0800 Subject: [PATCH 5/9] Apply suggestions from code review Co-authored-by: Tyler Yahn --- sdk/trace/config.go | 22 +++++++++++----------- sdk/trace/span.go | 2 +- sdk/trace/trace_test.go | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/sdk/trace/config.go b/sdk/trace/config.go index 44f4dfacbd7..87ca50baed8 100644 --- a/sdk/trace/config.go +++ b/sdk/trace/config.go @@ -33,37 +33,37 @@ type Config struct { Resource *resource.Resource } -// SpanLimits represents the limit to a span. +// SpanLimits represents the limits of a span. type SpanLimits struct { - // AttributeCountLimit is maximum allowed span attribute count. + // AttributeCountLimit is the maximum allowed span attribute count. AttributeCountLimit int - // EventCountLimit is maximum allowed span event count. + // EventCountLimit is the maximum allowed span event count. EventCountLimit int - // LinkCountLimit is maximum allowed span link count. + // LinkCountLimit is the maximum allowed span link count. LinkCountLimit int - // AttributePerEventCountLimit is maximum allowed attribute per span event count. + // AttributePerEventCountLimit is the maximum allowed attribute per span event count. AttributePerEventCountLimit int - // AttributePerLinkCountLimit is maximum allowed attribute per span link count. + // AttributePerLinkCountLimit is the maximum allowed attribute per span link count. AttributePerLinkCountLimit int } const ( - // DefaultAttributeCountLimit is default maximum allowed span attribute count. + // DefaultAttributeCountLimit is the default maximum allowed span attribute count. DefaultAttributeCountLimit = 128 - // DefaultEventCountLimit is default maximum allowed span event count. + // DefaultEventCountLimit is the default maximum allowed span event count. DefaultEventCountLimit = 128 - // DefaultLinkCountLimit is default maximum allowed span link count. + // DefaultLinkCountLimit is the default maximum allowed span link count. DefaultLinkCountLimit = 128 - // DefaultAttributePerEventCountLimit is default maximum allowed attribute per span event count. + // DefaultAttributePerEventCountLimit is the default maximum allowed attribute per span event count. DefaultAttributePerEventCountLimit = 128 - // DefaultAttributePerLinkCountLimit is default maximum allowed attribute per span link count. + // DefaultAttributePerLinkCountLimit is the default maximum allowed attribute per span link count. DefaultAttributePerLinkCountLimit = 128 ) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index ff8c7351f04..32e1ab61879 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -135,7 +135,7 @@ type span struct { // tracer is the SDK tracer that created this span. tracer *tracer - // spanLimits holds the limit to this span. + // spanLimits holds the limits to this span. spanLimits SpanLimits droppedAttributeCount int32 diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index b14c1f7f5bc..1eb620ff773 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1397,7 +1397,7 @@ func TestReadWriteSpan(t *testing.T) { // duplication. } -func TestAddSpanEventWithOverLimitedAttributes(t *testing.T) { +func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { te := NewTestExporter() cfg := Config{SpanLimits: SpanLimits{AttributePerEventCountLimit: 2}} tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) @@ -1459,7 +1459,7 @@ func TestAddSpanEventWithOverLimitedAttributes(t *testing.T) { } } -func TestWithLinksWithOverLimitedAttributes(t *testing.T) { +func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { te := NewTestExporter() cfg := Config{SpanLimits: SpanLimits{AttributePerLinkCountLimit: 1}} tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) From 6caf6eadeec21cae869f54399196fb7ab2d1f1a6 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 18 Feb 2021 11:51:54 +0800 Subject: [PATCH 6/9] Discard over limited attributes of links in `span.addLink` --- sdk/export/trace/trace.go | 16 +++++++++------- sdk/trace/span.go | 7 +++++++ sdk/trace/tracer.go | 17 +++++------------ 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/sdk/export/trace/trace.go b/sdk/export/trace/trace.go index 2851a0e06cb..b37a34da951 100644 --- a/sdk/export/trace/trace.go +++ b/sdk/export/trace/trace.go @@ -61,13 +61,15 @@ type SpanSnapshot struct { StartTime time.Time // The wall clock time of EndTime will be adjusted to always be offset // from StartTime by the duration of the span. - EndTime time.Time - Attributes []label.KeyValue - MessageEvents []trace.Event - Links []trace.Link - StatusCode codes.Code - StatusMessage string - HasRemoteParent bool + EndTime time.Time + Attributes []label.KeyValue + MessageEvents []trace.Event + Links []trace.Link + StatusCode codes.Code + StatusMessage string + HasRemoteParent bool + + // DroppedAttributeCount contains dropped attributes for the span itself, events and links. DroppedAttributeCount int DroppedMessageEventCount int DroppedLinkCount int diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 32e1ab61879..786a481413c 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -406,6 +406,13 @@ func (s *span) addLink(link trace.Link) { } s.mu.Lock() defer s.mu.Unlock() + + // Discard over limited attributes + if len(link.Attributes) > s.spanLimits.AttributePerLinkCountLimit { + s.addDroppedAttributeCount(len(link.Attributes) - s.spanLimits.AttributePerLinkCountLimit) + link.Attributes = link.Attributes[:s.spanLimits.AttributePerLinkCountLimit] + } + s.links.add(link) } diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 3f6bd66d373..8bc5109d977 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -38,10 +38,9 @@ var _ trace.Tracer = &tracer{} // configured appropriately by any SpanOption passed. Any Timestamp option // passed will be used as the start time of the Span's life-cycle. func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanOption) (context.Context, trace.Span) { - spanConfig := trace.NewSpanConfig(options...) - cfg := tr.provider.config.Load().(*Config) + config := trace.NewSpanConfig(options...) - parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, spanConfig.NewRoot) + parentSpanContext, remoteParent, links := parent.GetSpanContextAndLinks(ctx, config.NewRoot) if p := trace.SpanFromContext(ctx); p != nil { if sdkSpan, ok := p.(*span); ok { @@ -49,20 +48,14 @@ func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanO } } - span := startSpanInternal(ctx, tr, name, parentSpanContext, remoteParent, spanConfig) + span := startSpanInternal(ctx, tr, name, parentSpanContext, remoteParent, config) for _, l := range links { span.addLink(l) } - for _, l := range spanConfig.Links { - // Discard over limited attributes - if len(l.Attributes) > cfg.SpanLimits.AttributePerLinkCountLimit { - span.addDroppedAttributeCount(len(l.Attributes) - cfg.SpanLimits.AttributePerLinkCountLimit) - l.Attributes = l.Attributes[:cfg.SpanLimits.AttributePerLinkCountLimit] - } - + for _, l := range config.Links { span.addLink(l) } - span.SetAttributes(spanConfig.Attributes...) + span.SetAttributes(config.Attributes...) span.tracer = tr From 5ab674b7a958719bdfea522aab9327de5cd91efb Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 18 Feb 2021 13:28:32 +0800 Subject: [PATCH 7/9] Change the type of droppedAttributeCount to int64 --- sdk/trace/span.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 831fd92130b..a39516aecef 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -77,6 +77,9 @@ var emptySpanContext = trace.SpanContext{} // span is an implementation of the OpenTelemetry Span API representing the // individual component of a trace. type span struct { + // droppedAttributeCount contains dropped attributes for the events and links. + droppedAttributeCount int64 + // mu protects the contents of this span. mu sync.Mutex @@ -137,8 +140,6 @@ type span struct { // spanLimits holds the limits to this span. spanLimits SpanLimits - - droppedAttributeCount int32 } var _ trace.Span = &span{} @@ -529,7 +530,7 @@ func (s *span) addChild() { } func (s *span) addDroppedAttributeCount(delta int) { - atomic.AddInt32(&s.droppedAttributeCount, int32(delta)) + atomic.AddInt64(&s.droppedAttributeCount, int64(delta)) } func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trace.SpanContext, remoteParent bool, o *trace.SpanConfig) *span { From 4692352f20f810aa5a6afc7d3a045cc0e4fa34fc Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 18 Feb 2021 14:29:49 +0800 Subject: [PATCH 8/9] Fix tests --- sdk/trace/trace_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index a2302869676..fe533a16e94 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1432,7 +1432,7 @@ func TestReadWriteSpan(t *testing.T) { func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { te := NewTestExporter() cfg := Config{SpanLimits: SpanLimits{AttributePerEventCountLimit: 2}} - tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) + tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty())) span := startSpan(tp, "AddSpanEventWithOverLimitedAttributes") span.AddEvent("test1", trace.WithAttributes( @@ -1494,7 +1494,7 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { te := NewTestExporter() cfg := Config{SpanLimits: SpanLimits{AttributePerLinkCountLimit: 1}} - tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te)) + tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty())) k1v1 := label.String("key1", "value1") k2v2 := label.String("key2", "value2") From 19e53c5b1413efc6f230baed187baf40386b4776 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 18 Feb 2021 11:21:06 -0800 Subject: [PATCH 9/9] Fix label -> attribute package rename from merge --- sdk/trace/trace_test.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 41b08f0c17a..bf04c2f2c84 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -1425,15 +1425,15 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { span := startSpan(tp, "AddSpanEventWithOverLimitedAttributes") span.AddEvent("test1", trace.WithAttributes( - label.Bool("key1", true), - label.String("key2", "value2"), + attribute.Bool("key1", true), + attribute.String("key2", "value2"), )) // Parts of the attribute should be discard span.AddEvent("test2", trace.WithAttributes( - label.Bool("key1", true), - label.String("key2", "value2"), - label.String("key3", "value3"), - label.String("key4", "value4"), + attribute.Bool("key1", true), + attribute.String("key2", "value2"), + attribute.String("key3", "value3"), + attribute.String("key4", "value4"), )) got, err := endSpan(te, span) if err != nil { @@ -1457,16 +1457,16 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) { MessageEvents: []trace.Event{ { Name: "test1", - Attributes: []label.KeyValue{ - label.Bool("key1", true), - label.String("key2", "value2"), + Attributes: []attribute.KeyValue{ + attribute.Bool("key1", true), + attribute.String("key2", "value2"), }, }, { Name: "test2", - Attributes: []label.KeyValue{ - label.Bool("key1", true), - label.String("key2", "value2"), + Attributes: []attribute.KeyValue{ + attribute.Bool("key1", true), + attribute.String("key2", "value2"), }, }, }, @@ -1485,17 +1485,17 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { cfg := Config{SpanLimits: SpanLimits{AttributePerLinkCountLimit: 1}} tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty())) - k1v1 := label.String("key1", "value1") - k2v2 := label.String("key2", "value2") - k3v3 := label.String("key3", "value3") - k4v4 := label.String("key4", "value4") + k1v1 := attribute.String("key1", "value1") + k2v2 := attribute.String("key2", "value2") + k3v3 := attribute.String("key3", "value3") + k4v4 := attribute.String("key4", "value4") sc1 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}} sc2 := trace.SpanContext{TraceID: trace.TraceID([16]byte{1, 1}), SpanID: trace.SpanID{3}} span := startSpan(tp, "Links", trace.WithLinks([]trace.Link{ - {SpanContext: sc1, Attributes: []label.KeyValue{k1v1, k2v2}}, - {SpanContext: sc2, Attributes: []label.KeyValue{k2v2, k3v3, k4v4}}, + {SpanContext: sc1, Attributes: []attribute.KeyValue{k1v1, k2v2}}, + {SpanContext: sc2, Attributes: []attribute.KeyValue{k2v2, k3v3, k4v4}}, }...)) got, err := endSpan(te, span) @@ -1512,8 +1512,8 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) { Name: "span0", HasRemoteParent: true, Links: []trace.Link{ - {SpanContext: sc1, Attributes: []label.KeyValue{k1v1}}, - {SpanContext: sc2, Attributes: []label.KeyValue{k2v2}}, + {SpanContext: sc1, Attributes: []attribute.KeyValue{k1v1}}, + {SpanContext: sc2, Attributes: []attribute.KeyValue{k2v2}}, }, DroppedAttributeCount: 3, SpanKind: trace.SpanKindInternal,