Update span limits to conform with OpenTelemetry specification (#1535)

* Change the default span limit values to 128 * Rename and move MaxEventsPerSpan, MaxAttributesPerSpan, MaxLinksPerSpan into SpanLimits * Add AttributePerEventCountLimit and AttributePerLinkCountLimit * Update CHANGELOG * Apply suggestions from code review Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Discard over limited attributes of links in `span.addLink` * Change the type of droppedAttributeCount to int64 * Fix tests * Fix label -> attribute package rename from merge Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Tyler Yahn <codingalias@gmail.com>
open-telemetry · Feb 18, 2021 · 298c5a1 · 298c5a1
1 parent ecf65d7
commit 298c5a1
Show file tree

Hide file tree

Showing 6 changed files with 208 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,10 +8,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+### Added
+
+- AttributePerEventCountLimit and AttributePerLinkCountLimit for SpanLimits. (#1535)
+
 ### Changed
 
 - Replaced interface `oteltest.SpanRecorder` with its existing implementation
   `StandardSpanRecorder` (#1542).
+- Default span limit values to 128. (#1535)
+- Rename MaxEventsPerSpan, MaxAttributesPerSpan and MaxLinksPerSpan to EventCountLimit, AttributeCountLimit and LinkCountLimit, and move these fieds into SpanLimits. (#1535)
 - Renamed the `otel/label` package to `otel/attribute`. (#1541)
 
 ### Added

diff --git 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               []attribute.KeyValue
-	MessageEvents            []trace.Event
-	Links                    []trace.Link
-	StatusCode               codes.Code
-	StatusMessage            string
-	HasRemoteParent          bool
+	EndTime         time.Time
+	Attributes      []attribute.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/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 limits of a span.
+type SpanLimits struct {
+	// AttributeCountLimit is the maximum allowed span attribute count.
+	AttributeCountLimit int
+
+	// EventCountLimit is the maximum allowed span event count.
+	EventCountLimit int
+
+	// LinkCountLimit is the maximum allowed span link count.
+	LinkCountLimit int
+
+	// AttributePerEventCountLimit is the maximum allowed attribute per span event count.
+	AttributePerEventCountLimit int
+
+	// AttributePerLinkCountLimit is the maximum allowed attribute per span link count.
+	AttributePerLinkCountLimit int
+}
+
 const (
-	// DefaultMaxEventsPerSpan is default max number of message events per span
-	DefaultMaxEventsPerSpan = 1000
+	// DefaultAttributeCountLimit is the default maximum allowed span attribute count.
+	DefaultAttributeCountLimit = 128
+
+	// DefaultEventCountLimit is the default maximum allowed span event count.
+	DefaultEventCountLimit = 128
+
+	// DefaultLinkCountLimit is the default maximum allowed span link count.
+	DefaultLinkCountLimit = 128
 
-	// DefaultMaxAttributesPerSpan is default max number of attributes per span
-	DefaultMaxAttributesPerSpan = 1000
+	// DefaultAttributePerEventCountLimit is the default maximum allowed attribute per span event count.
+	DefaultAttributePerEventCountLimit = 128
 
-	// DefaultMaxLinksPerSpan is default max number of links per span
-	DefaultMaxLinksPerSpan = 1000
+	// DefaultAttributePerLinkCountLimit is the default maximum allowed attribute per span link count.
+	DefaultAttributePerLinkCountLimit = 128
 )
diff --git 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,20 @@ 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.SpanLimits.AttributePerEventCountLimit > 0 {
+		c.SpanLimits.AttributePerEventCountLimit = cfg.SpanLimits.AttributePerEventCountLimit
+	}
+	if cfg.SpanLimits.AttributePerLinkCountLimit > 0 {
+		c.SpanLimits.AttributePerLinkCountLimit = cfg.SpanLimits.AttributePerLinkCountLimit
 	}
 	c.Resource = cfg.Resource
 	if c.Resource == nil {

diff --git 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/attribute"
@@ -76,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
 
@@ -133,6 +137,9 @@ type span struct {
 
 	// tracer is the SDK tracer that created this span.
 	tracer *tracer
+
+	// spanLimits holds the limits to this span.
+	spanLimits SpanLimits
 }
 
 var _ trace.Span = &span{}
@@ -284,6 +291,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{
@@ -407,6 +420,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)
 }
 
@@ -430,9 +450,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()
@@ -480,6 +501,10 @@ func (s *span) addChild() {
 	s.mu.Unlock()
 }
 
+func (s *span) addDroppedAttributeCount(delta int) {
+	atomic.AddInt64(&s.droppedAttributeCount, int64(delta))
+}
+
 func startSpanInternal(ctx context.Context, tr *tracer, name string, parent trace.SpanContext, remoteParent bool, o *trace.SpanConfig) *span {
 	span := &span{}
 	span.spanContext = parent
@@ -494,9 +519,10 @@ 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)
+	span.spanLimits = cfg.SpanLimits
 
 	data := samplingData{
 		noParent:     hasEmptySpanContext(parent),

diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go
@@ -471,7 +471,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), WithResource(resource.Empty()))
 
 	span := startSpan(tp, "SpanAttributesOverLimit")
@@ -554,7 +554,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), WithResource(resource.Empty()))
 
 	span := startSpan(tp, "EventsOverLimit")
@@ -645,7 +645,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}}
@@ -1013,7 +1013,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
@@ -1417,3 +1417,109 @@ func TestReadWriteSpan(t *testing.T) {
 	// available via ReadWriteSpan as doing so would mean creating a lot of
 	// duplication.
 }
+
+func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) {
+	te := NewTestExporter()
+	cfg := Config{SpanLimits: SpanLimits{AttributePerEventCountLimit: 2}}
+	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+
+	span := startSpan(tp, "AddSpanEventWithOverLimitedAttributes")
+	span.AddEvent("test1", trace.WithAttributes(
+		attribute.Bool("key1", true),
+		attribute.String("key2", "value2"),
+	))
+	// Parts of the attribute should be discard
+	span.AddEvent("test2", trace.WithAttributes(
+		attribute.Bool("key1", true),
+		attribute.String("key2", "value2"),
+		attribute.String("key3", "value3"),
+		attribute.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: []attribute.KeyValue{
+					attribute.Bool("key1", true),
+					attribute.String("key2", "value2"),
+				},
+			},
+			{
+				Name: "test2",
+				Attributes: []attribute.KeyValue{
+					attribute.Bool("key1", true),
+					attribute.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 TestAddLinksWithMoreAttributesThanLimit(t *testing.T) {
+	te := NewTestExporter()
+	cfg := Config{SpanLimits: SpanLimits{AttributePerLinkCountLimit: 1}}
+	tp := NewTracerProvider(WithConfig(cfg), WithSyncer(te), WithResource(resource.Empty()))
+
+	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: []attribute.KeyValue{k1v1, k2v2}},
+		{SpanContext: sc2, Attributes: []attribute.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: []attribute.KeyValue{k1v1}},
+			{SpanContext: sc2, Attributes: []attribute.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)
+	}
+}