Skip to content

Commit

Permalink
trace: Use non-generic to replace newEvictedQueue in trace.start to r…
Browse files Browse the repository at this point in the history
…educe memory usage. (open-telemetry#5497)

benchstat:
```
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/sdk/trace
              │     old     │                 new                 │
              │   sec/op    │   sec/op     vs base                │
TraceStart-10   950.6n ± 1%   641.0n ± 0%  -32.57% (p=0.000 n=10)

              │     old     │                new                 │
              │    B/op     │    B/op     vs base                │
TraceStart-10   1040.0 ± 0%   704.0 ± 0%  -32.31% (p=0.000 n=10)

              │    old     │                new                 │
              │ allocs/op  │ allocs/op   vs base                │
TraceStart-10   20.00 ± 0%   14.00 ± 0%  -30.00% (p=0.000 n=10)
```

---------

Co-authored-by: Damien Mathieu <damien.mathieu@elastic.co>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
3 people authored and OrHayat committed Jun 23, 2024
1 parent a88811d commit 9395ba6
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The exporter no longer exports the deprecated "otel.library.name" or "otel.library.version" attributes.
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/resource`. (#5490)
- Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/trace`. (#5490)
- Use non-generic functions in the `Start` method of `"go.opentelemetry.io/otel/sdk/trace".Trace` to reduce memory allocation. (#5497)

### Fixed

Expand Down
17 changes: 11 additions & 6 deletions sdk/trace/evictedqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package trace // import "go.opentelemetry.io/otel/sdk/trace"

import (
"fmt"
"slices"
"sync"

Expand All @@ -19,13 +18,19 @@ type evictedQueue[T any] struct {
logDropped func()
}

func newEvictedQueue[T any](capacity int) evictedQueue[T] {
var tVal T
msg := fmt.Sprintf("limit reached: dropping trace %T", tVal)
func newEvictedQueueEvent(capacity int) evictedQueue[Event] {
// Do not pre-allocate queue, do this lazily.
return evictedQueue[T]{
return evictedQueue[Event]{
capacity: capacity,
logDropped: sync.OnceFunc(func() { global.Warn(msg) }),
logDropped: sync.OnceFunc(func() { global.Warn("limit reached: dropping trace trace.Event") }),
}
}

func newEvictedQueueLink(capacity int) evictedQueue[Link] {
// Do not pre-allocate queue, do this lazily.
return evictedQueue[Link]{
capacity: capacity,
logDropped: sync.OnceFunc(func() { global.Warn("limit reached: dropping trace trace.Link") }),
}
}

Expand Down
32 changes: 16 additions & 16 deletions sdk/trace/evictedqueue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,47 +14,47 @@ func init() {
}

func TestAdd(t *testing.T) {
q := newEvictedQueue[string](3)
q.add("value1")
q.add("value2")
q := newEvictedQueueLink(3)
q.add(Link{})
q.add(Link{})
if wantLen, gotLen := 2, len(q.queue); wantLen != gotLen {
t.Errorf("got queue length %d want %d", gotLen, wantLen)
}
}

func TestCopy(t *testing.T) {
q := newEvictedQueue[string](3)
q.add("value1")
q := newEvictedQueueEvent(3)
q.add(Event{Name: "value1"})
cp := q.copy()

q.add("value2")
assert.Equal(t, []string{"value1"}, cp, "queue update modified copy")
q.add(Event{Name: "value2"})
assert.Equal(t, []Event{{Name: "value1"}}, cp, "queue update modified copy")

cp[0] = "value0"
assert.Equal(t, "value1", q.queue[0], "copy update modified queue")
cp[0] = Event{Name: "value0"}
assert.Equal(t, Event{Name: "value1"}, q.queue[0], "copy update modified queue")
}

func TestDropCount(t *testing.T) {
q := newEvictedQueue[string](3)
q := newEvictedQueueEvent(3)
var called bool
q.logDropped = func() { called = true }

q.add("value1")
q.add(Event{Name: "value1"})
assert.False(t, called, `"value1" logged as dropped`)
q.add("value2")
q.add(Event{Name: "value2"})
assert.False(t, called, `"value2" logged as dropped`)
q.add("value3")
q.add(Event{Name: "value3"})
assert.False(t, called, `"value3" logged as dropped`)
q.add("value1")
q.add(Event{Name: "value1"})
assert.True(t, called, `"value2" not logged as dropped`)
q.add("value4")
q.add(Event{Name: "value4"})
if wantLen, gotLen := 3, len(q.queue); wantLen != gotLen {
t.Errorf("got queue length %d want %d", gotLen, wantLen)
}
if wantDropCount, gotDropCount := 2, q.droppedCount; wantDropCount != gotDropCount {
t.Errorf("got drop count %d want %d", gotDropCount, wantDropCount)
}
wantArr := []string{"value3", "value1", "value4"}
wantArr := []Event{{Name: "value3"}, {Name: "value1"}, {Name: "value4"}}
gotArr := q.copy()

if wantLen, gotLen := len(wantArr), len(gotArr); gotLen != wantLen {
Expand Down
13 changes: 13 additions & 0 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2107,3 +2107,16 @@ func TestAddLinkToNonRecordingSpan(t *testing.T) {
t.Errorf("AddLinkToNonRecordingSpan: -got +want %s", diff)
}
}

func BenchmarkTraceStart(b *testing.B) {
tracer := NewTracerProvider().Tracer("")
ctx := trace.ContextWithSpanContext(context.Background(), trace.SpanContext{})

b.ReportAllocs()
b.ResetTimer()

for i := 0; i < b.N; i++ {
_, span := tracer.Start(ctx, "")
span.End()
}
}
4 changes: 2 additions & 2 deletions sdk/trace/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr Sa
spanKind: trace.ValidateSpanKind(config.SpanKind()),
name: name,
startTime: startTime,
events: newEvictedQueue[Event](tr.provider.spanLimits.EventCountLimit),
links: newEvictedQueue[Link](tr.provider.spanLimits.LinkCountLimit),
events: newEvictedQueueEvent(tr.provider.spanLimits.EventCountLimit),
links: newEvictedQueueLink(tr.provider.spanLimits.LinkCountLimit),
tracer: tr,
}

Expand Down

0 comments on commit 9395ba6

Please sign in to comment.