Skip to content

Commit

Permalink
Record links/events attribute drops independently (#1771)
Browse files Browse the repository at this point in the history
* Record links/events attribute drops independently

* Update PR number

* Remove unneeded span.droppedAttributeCount
  • Loading branch information
MrAlias authored Apr 6, 2021
1 parent 5bbfc22 commit e9aaa04
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 21 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE`
- `OTEL_EXPORTER_OTLP_METRICS_CERTIFICATE`
- `trace.TraceFlags` is now a defined type over `byte` and `WithSampled(bool) TraceFlags` and `IsSampled() bool` methods have been added to it. (#1770)
- The `Event` and `Link` struct types from the `go.opentelemetry.io/otel` package now include a `DroppedAttributeCount` field to record the number of attributes that were not recorded due to configured limits being reached. (#1771)
- The Jaeger exporter now reports dropped attributes for a Span event in the exported log. (#1771)

### Fixed

Expand All @@ -54,6 +56,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
This changes it to make `SamplingParameters` conform with the OpenTelemetry specification. (#1749)
- Modify `BatchSpanProcessor.ForceFlush` to abort after timeout/cancellation. (#1757)
- Improve OTLP/gRPC exporter connection errors. (#1737)
- The `DroppedAttributeCount` field of the `Span` in the `go.opentelemetry.io/otel` package now only represents the number of attributes dropped for the span itself.
It no longer is a conglomerate of itself, events, and link attributes that have been dropped. (#1771)
- Make `ExportSpans` in Jaeger Exporter honor context deadline. (#1773)

### Removed
Expand Down
2 changes: 2 additions & 0 deletions exporters/stdout/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func TestExporter_ExportSpan(t *testing.T) {
`"Value":{"Type":"STRING","Value":"value"}` +
`}` +
`],` +
`"DroppedAttributeCount":0,` +
`"Time":` + string(expectedSerializedNow) +
`},` +
`{` +
Expand All @@ -123,6 +124,7 @@ func TestExporter_ExportSpan(t *testing.T) {
`"Value":{"Type":"FLOAT64","Value":123.456}` +
`}` +
`],` +
`"DroppedAttributeCount":0,` +
`"Time":` + string(expectedSerializedNow) +
`}` +
`],` +
Expand Down
7 changes: 7 additions & 0 deletions exporters/trace/jaeger/jaeger.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
keySpanKind = "span.kind"
keyStatusCode = "otel.status_code"
keyStatusMessage = "otel.status_description"
keyDroppedAttributeCount = "otel.event.dropped_attributes_count"
keyEventName = "event"
)

Expand Down Expand Up @@ -301,6 +302,9 @@ func spanSnapshotToThrift(ss *export.SpanSnapshot) *gen.Span {
if a.Name != "" {
nTags++
}
if a.DroppedAttributeCount != 0 {
nTags++
}
fields := make([]*gen.Tag, 0, nTags)
if a.Name != "" {
// If an event contains an attribute with the same key, it needs
Expand All @@ -313,6 +317,9 @@ func spanSnapshotToThrift(ss *export.SpanSnapshot) *gen.Span {
fields = append(fields, tag)
}
}
if a.DroppedAttributeCount != 0 {
fields = append(fields, getInt64Tag(keyDroppedAttributeCount, int64(a.DroppedAttributeCount)))
}
logs = append(logs, &gen.Log{
Timestamp: a.Time.UnixNano() / 1000,
Fields: fields,
Expand Down
13 changes: 12 additions & 1 deletion exporters/trace/jaeger/jaeger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ func Test_spanSnapshotToThrift(t *testing.T) {
linkSpanID, _ := trace.SpanIDFromHex("0102030405060709")

eventNameValue := "event-test"
eventDropped := int64(10)
keyValue := "value"
statusCodeValue := int64(1)
doubleValue := 123.456
Expand Down Expand Up @@ -432,7 +433,12 @@ func Test_spanSnapshotToThrift(t *testing.T) {
attribute.Int64("int", intValue),
},
MessageEvents: []trace.Event{
{Name: eventNameValue, Attributes: []attribute.KeyValue{attribute.String("k1", keyValue)}, Time: now},
{
Name: eventNameValue,
Attributes: []attribute.KeyValue{attribute.String("k1", keyValue)},
DroppedAttributeCount: int(eventDropped),
Time: now,
},
},
StatusCode: codes.Error,
StatusMessage: statusMessage,
Expand Down Expand Up @@ -482,6 +488,11 @@ func Test_spanSnapshotToThrift(t *testing.T) {
VStr: &keyValue,
VType: gen.TagType_STRING,
},
{
Key: keyDroppedAttributeCount,
VLong: &eventDropped,
VType: gen.TagType_LONG,
},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion sdk/export/trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type SpanSnapshot struct {
StatusCode codes.Code
StatusMessage string

// DroppedAttributeCount contains dropped attributes for the span itself, events and links.
// DroppedAttributeCount contains dropped attributes for the span itself.
DroppedAttributeCount int
DroppedMessageEventCount int
DroppedLinkCount int
Expand Down
23 changes: 8 additions & 15 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"reflect"
"sync"
"sync/atomic"
"time"

"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -75,9 +74,6 @@ type ReadWriteSpan interface {
// 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 @@ -292,17 +288,19 @@ func (s *span) addEvent(name string, o ...trace.EventOption) {
c := trace.NewEventConfig(o...)

// Discard over limited attributes
var discarded int
if len(c.Attributes) > s.spanLimits.AttributePerEventCountLimit {
s.addDroppedAttributeCount(len(c.Attributes) - s.spanLimits.AttributePerEventCountLimit)
discarded = 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{
Name: name,
Attributes: c.Attributes,
Time: c.Timestamp,
Name: name,
Attributes: c.Attributes,
DroppedAttributeCount: discarded,
Time: c.Timestamp,
})
}

Expand Down Expand Up @@ -423,7 +421,7 @@ func (s *span) addLink(link trace.Link) {

// Discard over limited attributes
if len(link.Attributes) > s.spanLimits.AttributePerLinkCountLimit {
s.addDroppedAttributeCount(len(link.Attributes) - s.spanLimits.AttributePerLinkCountLimit)
link.DroppedAttributeCount = len(link.Attributes) - s.spanLimits.AttributePerLinkCountLimit
link.Attributes = link.Attributes[:s.spanLimits.AttributePerLinkCountLimit]
}

Expand All @@ -449,10 +447,9 @@ 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()
Expand Down Expand Up @@ -502,10 +499,6 @@ func (s *span) addChild() {
s.mu.Unlock()
}

func (s *span) addDroppedAttributeCount(delta int) {
atomic.AddInt64(&s.droppedAttributeCount, int64(delta))
}

func (*span) private() {}

func startSpanInternal(ctx context.Context, tr *tracer, name string, o *trace.SpanConfig) *span {
Expand Down
15 changes: 11 additions & 4 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1491,10 +1491,10 @@ func TestAddEventsWithMoreAttributesThanLimit(t *testing.T) {
attribute.Bool("key1", true),
attribute.String("key2", "value2"),
},
DroppedAttributeCount: 2,
},
},
SpanKind: trace.SpanKindInternal,
DroppedAttributeCount: 2,
InstrumentationLibrary: instrumentation.Library{Name: "AddSpanEventWithOverLimitedAttributes"},
}
if diff := cmpDiff(got, want); diff != "" {
Expand Down Expand Up @@ -1536,10 +1536,17 @@ func TestAddLinksWithMoreAttributesThanLimit(t *testing.T) {
Parent: sc.WithRemote(true),
Name: "span0",
Links: []trace.Link{
{SpanContext: sc1, Attributes: []attribute.KeyValue{k1v1}},
{SpanContext: sc2, Attributes: []attribute.KeyValue{k2v2}},
{
SpanContext: sc1,
Attributes: []attribute.KeyValue{k1v1},
DroppedAttributeCount: 1,
},
{
SpanContext: sc2,
Attributes: []attribute.KeyValue{k2v2},
DroppedAttributeCount: 2,
},
},
DroppedAttributeCount: 3,
SpanKind: trace.SpanKindInternal,
InstrumentationLibrary: instrumentation.Library{Name: "Links"},
}
Expand Down
11 changes: 11 additions & 0 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,10 @@ type Event struct {
// Attributes describe the aspects of the event.
Attributes []attribute.KeyValue

// DroppedAttributeCount is the number of attributes that were not
// recorded due to configured limits being reached.
DroppedAttributeCount int

// Time at which this event was recorded.
Time time.Time
}
Expand All @@ -567,8 +571,15 @@ type Event struct {
// form. A Link is used to keep reference to the original SpanContext and
// track the relationship.
type Link struct {
// SpanContext of the linked Span.
SpanContext

// Attributes describe the aspects of the link.
Attributes []attribute.KeyValue

// DroppedAttributeCount is the number of attributes that were not
// recorded due to configured limits being reached.
DroppedAttributeCount int
}

// SpanKind is the role a Span plays in a Trace.
Expand Down

0 comments on commit e9aaa04

Please sign in to comment.