Skip to content

Commit

Permalink
Performance improvements for the trace SDK in Span. (#5874)
Browse files Browse the repository at this point in the history
Good day,
Thanks for review this PR!
This PR follows from
#5864 and avoid
multiple locks in `End`, `RecordError`, `AddEvent` and `AddLink`.

Benchstats result are:
```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: 11th Gen Intel(R) Core(TM) i5-11400H @ 2.70GHz
           │ event-3cbd9671.txt │        event-pr-e9744b48.txt        │
           │       sec/op       │                 │
SpanEnd-12          63.07n ± 1%   53.63n ± 1%  -14.97% (p=0.000 n=10)

           │ event-3cbd9671.txt │     event-pr-e9744b48.txt      │
           │        B/op        │    B/op     vs base            │
SpanEnd-12           0.000 ± 0%   0.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

           │ event-3cbd9671.txt │     event-pr-e9744b48.txt      │
           │     allocs/op      │ allocs/op   vs base            │
SpanEnd-12           0.000 ± 0%   0.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

```

---------

Co-authored-by: David Ashpole <dashpole@google.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
  • Loading branch information
3 people authored Oct 9, 2024
1 parent 8e9baf2 commit 6441653
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The `Record` type in `go.opentelemetry.io/otel/log` is no longer comparable. (#5847)
- Performance improvements for the trace SDK `SetAttributes` method in `Span`. (#5864)
- Reduce memory allocations for the `Event` and `Link` lists in `Span`. (#5858)
- Performance improvements for the trace SDK `AddEvent`, `AddLink`, `RecordError` and `End` methods in `Span`. (#5874)

### Deprecated

Expand Down
46 changes: 33 additions & 13 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ func (s *recordingSpan) IsRecording() bool {

// isRecording returns if this span is being recorded. If this span has ended
// this will return false.
// This is done without acquiring a lock.
//
// This method assumes s.mu.Lock is held by the caller.
func (s *recordingSpan) isRecording() bool {
if s == nil {
return false
Expand Down Expand Up @@ -408,9 +409,10 @@ func (s *recordingSpan) End(options ...trace.SpanEndOption) {
// the span's duration in case some operation below takes a while.
et := monotonicEndTime(s.startTime)

// Do relative expensive check now that we have an end time and see if we
// need to do any more processing.
if !s.IsRecording() {
// Lock the span now that we have an end time and see if we need to do any more processing.
s.mu.Lock()
if !s.isRecording() {
s.mu.Unlock()
return
}

Expand All @@ -435,10 +437,11 @@ func (s *recordingSpan) End(options ...trace.SpanEndOption) {
}

if s.executionTracerTaskEnd != nil {
s.mu.Unlock()
s.executionTracerTaskEnd()
s.mu.Lock()
}

s.mu.Lock()
// Setting endTime to non-zero marks the span as ended and not recording.
if config.Timestamp().IsZero() {
s.endTime = et
Expand Down Expand Up @@ -472,7 +475,13 @@ func monotonicEndTime(start time.Time) time.Time {
// does not change the Span status. If this span is not being recorded or err is nil
// than this method does nothing.
func (s *recordingSpan) RecordError(err error, opts ...trace.EventOption) {
if s == nil || err == nil || !s.IsRecording() {
if s == nil || err == nil {
return
}

s.mu.Lock()
defer s.mu.Unlock()
if !s.isRecording() {
return
}

Expand Down Expand Up @@ -508,14 +517,23 @@ func recordStackTrace() string {
}

// AddEvent adds an event with the provided name and options. If this span is
// not being recorded than this method does nothing.
// not being recorded then this method does nothing.
func (s *recordingSpan) AddEvent(name string, o ...trace.EventOption) {
if !s.IsRecording() {
if s == nil {
return
}

s.mu.Lock()
defer s.mu.Unlock()
if !s.isRecording() {
return
}
s.addEvent(name, o...)
}

// addEvent adds an event with the provided name and options.
//
// This method assumes s.mu.Lock is held by the caller.
func (s *recordingSpan) addEvent(name string, o ...trace.EventOption) {
c := trace.NewEventConfig(o...)
e := Event{Name: name, Attributes: c.Attributes(), Time: c.Timestamp()}
Expand All @@ -532,9 +550,7 @@ func (s *recordingSpan) addEvent(name string, o ...trace.EventOption) {
e.Attributes = e.Attributes[:limit]
}

s.mu.Lock()
s.events.add(e)
s.mu.Unlock()
}

// SetName sets the name of this span. If this span is not being recorded than
Expand Down Expand Up @@ -682,14 +698,20 @@ func (s *recordingSpan) Resource() *resource.Resource {
}

func (s *recordingSpan) AddLink(link trace.Link) {
if !s.IsRecording() {
if s == nil {
return
}
if !link.SpanContext.IsValid() && len(link.Attributes) == 0 &&
link.SpanContext.TraceState().Len() == 0 {
return
}

s.mu.Lock()
defer s.mu.Unlock()
if !s.isRecording() {
return
}

l := Link{SpanContext: link.SpanContext, Attributes: link.Attributes}

// Discard attributes over limit.
Expand All @@ -703,9 +725,7 @@ func (s *recordingSpan) AddLink(link trace.Link) {
l.Attributes = l.Attributes[:limit]
}

s.mu.Lock()
s.links.add(l)
s.mu.Unlock()
}

// DroppedAttributes returns the number of attributes dropped by the span
Expand Down

0 comments on commit 6441653

Please sign in to comment.