Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update span limits to conform with OpenTelemetry specification #1535

Merged
merged 12 commits into from
Feb 18, 2021
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 9 additions & 7 deletions sdk/export/trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 32 additions & 14 deletions sdk/trace/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
32 changes: 21 additions & 11 deletions sdk/trace/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
34 changes: 30 additions & 4 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"reflect"
"sync"
"sync/atomic"
"time"

"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}

Expand All @@ -430,9 +450,10 @@ func (s *span) Snapshot() *export.SpanSnapshot {
sd.StatusCode = s.statusCode
sd.StatusMessage = s.statusMessage

sd.DroppedAttributeCount = int(s.droppedAttributeCount)
XSAM marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down Expand Up @@ -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
Expand All @@ -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),
Expand Down
114 changes: 110 additions & 4 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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}}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}