Skip to content

Commit

Permalink
tracing: link child into parent, even if not verbose
Browse files Browse the repository at this point in the history
Prior to this change, when a child was derived from a local parent, we
would not register the child with the parent. In effect, this meant that
any payloads in the child would not be collected.

Release note: None
  • Loading branch information
tbg committed Jan 18, 2021
1 parent a51f2e2 commit 0924c8c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
10 changes: 7 additions & 3 deletions pkg/util/tracing/crdbspan.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,16 @@ func (s *crdbSpan) recordingType() RecordingType {
// If separate recording is specified, the child is not registered with the
// parent. Thus, the parent's recording will not include this child.
func (s *crdbSpan) enableRecording(parent *crdbSpan, recType RecordingType) {
s.mu.Lock()
defer s.mu.Unlock()
s.mu.recording.recordingType.swap(recType)
if parent != nil {
parent.addChild(s)
}
if recType == RecordingOff {
return
}

s.mu.Lock()
defer s.mu.Unlock()
s.mu.recording.recordingType.swap(recType)
if recType == RecordingVerbose {
s.setBaggageItemLocked(verboseTracingBaggageKey, "1")
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/util/tracing/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,19 @@ func TestSpan_LogStructured(t *testing.T) {
require.NoError(t, types.UnmarshalAny(item, &d1))
require.IsType(t, (*types.Int32Value)(nil), d1.Message)
}

func TestNonVerboseChildSpanRegisteredWithParent(t *testing.T) {
tr := NewTracer()
tr._mode = int32(modeBackground)
sp := tr.StartSpan("root", WithForceRealSpan())
defer sp.Finish()
ch := tr.StartSpan("child", WithParentAndAutoCollection(sp), WithForceRealSpan())
defer ch.Finish()
require.Len(t, sp.crdb.mu.recording.children, 1)
require.Equal(t, ch.crdb, sp.crdb.mu.recording.children[0])
ch.LogStructured(&types.Int32Value{Value: 5})
// Check that the child span (incl its payload) is in the recording.
rec := sp.GetRecording()
require.Len(t, rec, 2)
require.Len(t, rec[1].InternalStructured, 1)
}
12 changes: 6 additions & 6 deletions pkg/util/tracing/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,16 +394,16 @@ func (t *Tracer) startSpanGeneric(

s := &helper.span

// Start recording if necessary. We inherit the recording type of the local parent, if any,
// over the remote parent, if any. If neither are specified, we're not recording.
recordingType := opts.recordingType()

if recordingType != RecordingOff {
{
// Link the newly created span to the parent, if necessary,
// and start recording, if requested.
// We inherit the recording type of the local parent, if any,
// over the remote parent, if any. If neither are specified, we're not recording.
var p *crdbSpan
if opts.Parent != nil {
p = opts.Parent.crdb
}
s.crdb.enableRecording(p, recordingType)
s.crdb.enableRecording(p, opts.recordingType())
}

// Set initial tags. These will propagate to the crdbSpan, ot, and netTr
Expand Down

0 comments on commit 0924c8c

Please sign in to comment.