Skip to content

Commit

Permalink
tracing: use byte-limits for structured events per span
Browse files Browse the repository at this point in the history
Touches cockroachdb#59188. Follow-on work from cockroachdb#60678.

Release justification: low risk, high benefit changes to existing
functionality

Release note: None
  • Loading branch information
irfansharif committed Mar 5, 2021
1 parent 05a7bec commit 7e1316f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 11 deletions.
9 changes: 7 additions & 2 deletions pkg/util/tracing/crdbspan.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ type crdbSpanMu struct {
// those that were set before recording started)?
tags opentracing.Tags

structured ring.Buffer // of Structured events
bytesStructured int64
structured ring.Buffer // of Structured events

// The Span's associated baggage.
baggage map[string]string
Expand Down Expand Up @@ -225,9 +226,13 @@ func (s *crdbSpan) recordStructured(item Structured) {
s.mu.Lock()
defer s.mu.Unlock()

if s.mu.structured.Len() == maxStructuredEventsPerSpan {
s.mu.bytesStructured += int64(item.Size())
for s.mu.bytesStructured > maxStructuredBytesPerSpan {
last := s.mu.structured.GetLast().(Structured)
s.mu.structured.RemoveLast()
s.mu.bytesStructured -= int64(last.Size())
}

s.mu.structured.AddFirst(item)
}

Expand Down
15 changes: 10 additions & 5 deletions pkg/util/tracing/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,18 @@ func TestSpanRecordStructuredLimit(t *testing.T) {
sp := tr.StartSpan("root", WithForceRealSpan())
defer sp.Finish()

offset := 1000 // we start at a high enough integer to not have to worry about variable payload sizes
payload := func(i int) Structured { return &types.Int32Value{Value: int32(i)} }

numPayloads := maxStructuredBytesPerSpan / payload(offset).Size()
const extra = 10
for i := int32(1); i <= maxStructuredEventsPerSpan+extra; i++ {
sp.RecordStructured(&types.Int32Value{Value: i})
for i := offset + 1; i <= offset+numPayloads+extra; i++ {
sp.RecordStructured(payload(i))
}

rec := sp.GetRecording()
require.Len(t, rec, 1)
require.Len(t, rec[0].InternalStructured, maxStructuredEventsPerSpan)
require.Len(t, rec[0].InternalStructured, numPayloads)

first := rec[0].InternalStructured[0]
last := rec[0].InternalStructured[len(rec[0].InternalStructured)-1]
Expand All @@ -235,10 +240,10 @@ func TestSpanRecordStructuredLimit(t *testing.T) {

var res int32
require.NoError(t, types.StdInt32Unmarshal(&res, first.Value))
require.Equal(t, res, int32(maxStructuredEventsPerSpan+extra))
require.Equal(t, res, int32(offset+numPayloads+extra))

require.NoError(t, types.StdInt32Unmarshal(&res, last.Value))
require.Equal(t, res, int32(extra+1))
require.Equal(t, res, int32(offset+extra+1))
}

func TestNonVerboseChildSpanRegisteredWithParent(t *testing.T) {
Expand Down
7 changes: 3 additions & 4 deletions pkg/util/tracing/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ const (
// maxLogsPerSpan limits the number of logs in a Span; use a comfortable
// limit.
maxLogsPerSpan = 1000
// maxStructuredEventsPerSpan limits the number of structured events in a
// span; use a comfortable limit.
maxStructuredEventsPerSpan = 50
// maxStructuredBytesPerSpan limits the size of structured events in a span;
// use a comfortable limit.
maxStructuredBytesPerSpan = 1 << 10 // 1 KiB
// maxChildrenPerSpan limits the number of (direct) child spans in a Span.
maxChildrenPerSpan = 1000
// maxSpanRegistrySize limits the number of local root spans tracked in
Expand Down Expand Up @@ -368,7 +368,6 @@ func (t *Tracer) startSpanGeneric(
duration: -1, // unfinished
},
}
helper.crdbSpan.mu.structured.Reserve(maxStructuredEventsPerSpan)
helper.span.i = spanInner{
tracer: t,
crdb: &helper.crdbSpan,
Expand Down

0 comments on commit 7e1316f

Please sign in to comment.