diff --git a/pkg/cli/debug_send_kv_batch.go b/pkg/cli/debug_send_kv_batch.go index 81d8befead08..4d6f118b0f28 100644 --- a/pkg/cli/debug_send_kv_batch.go +++ b/pkg/cli/debug_send_kv_batch.go @@ -249,7 +249,7 @@ func sendKVBatchRequestWithTracingOption( if sp != nil { // Import the remotely collected spans, if any. - sp.ImportRemoteSpans(br.CollectedSpans) + sp.ImportRemoteRecording(br.CollectedSpans) // Extract the recording. rec = sp.GetRecording(tracing.RecordingVerbose) diff --git a/pkg/kv/kvclient/kvcoord/transport.go b/pkg/kv/kvclient/kvcoord/transport.go index dd9d8c526318..7bc422d64320 100644 --- a/pkg/kv/kvclient/kvcoord/transport.go +++ b/pkg/kv/kvclient/kvcoord/transport.go @@ -222,7 +222,7 @@ func (gt *grpcTransport) sendBatch( return nil, errors.Errorf( "trying to ingest remote spans but there is no recording span set up") } - span.ImportRemoteSpans(reply.CollectedSpans) + span.ImportRemoteRecording(reply.CollectedSpans) } } return reply, err @@ -352,7 +352,7 @@ func (s *senderTransport) SendNext( if span == nil { panic("trying to ingest remote spans but there is no recording span set up") } - span.ImportRemoteSpans(br.CollectedSpans) + span.ImportRemoteRecording(br.CollectedSpans) } return br, nil diff --git a/pkg/kv/kvserver/store_snapshot.go b/pkg/kv/kvserver/store_snapshot.go index c383e9a98e23..ce855ee11643 100644 --- a/pkg/kv/kvserver/store_snapshot.go +++ b/pkg/kv/kvserver/store_snapshot.go @@ -1343,7 +1343,7 @@ func delegateSnapshot( "trying to ingest remote spans but there is no recording span set up", ) } else { - span.ImportRemoteSpans(resp.CollectedSpans) + span.ImportRemoteRecording(resp.CollectedSpans) } } switch resp.SnapResponse.Status { diff --git a/pkg/sql/crdb_internal_test.go b/pkg/sql/crdb_internal_test.go index 536b2c9cabdc..4df6ba473d93 100644 --- a/pkg/sql/crdb_internal_test.go +++ b/pkg/sql/crdb_internal_test.go @@ -766,7 +766,7 @@ func setupTraces(t1, t2 *tracing.Tracer) (tracingpb.TraceID, func()) { // Start another remote child span on "node 2" that we finish. childRemoteChildFinished := t2.StartSpan("root.child.remotechilddone", tracing.WithRemoteParentFromSpanMeta(child.Meta())) - child.ImportRemoteSpans(childRemoteChildFinished.FinishAndGetRecording(tracing.RecordingVerbose)) + child.ImportRemoteRecording(childRemoteChildFinished.FinishAndGetRecording(tracing.RecordingVerbose)) // Start another remote child span on "node 2" that we finish. This will have // a different trace_id from the spans created above. diff --git a/pkg/sql/distsql_running.go b/pkg/sql/distsql_running.go index c061f294f0e3..eb6360edd756 100644 --- a/pkg/sql/distsql_running.go +++ b/pkg/sql/distsql_running.go @@ -981,7 +981,7 @@ func (r *DistSQLReceiver) pushMeta(meta *execinfrapb.ProducerMetadata) execinfra } if len(meta.TraceData) > 0 { if span := tracing.SpanFromContext(r.ctx); span != nil { - span.ImportRemoteSpans(meta.TraceData) + span.ImportRemoteRecording(meta.TraceData) } var ev roachpb.ContentionEvent for i := range meta.TraceData { diff --git a/pkg/sql/rowexec/tablereader_test.go b/pkg/sql/rowexec/tablereader_test.go index fc66ae1dda14..8ec1a2eba68e 100644 --- a/pkg/sql/rowexec/tablereader_test.go +++ b/pkg/sql/rowexec/tablereader_test.go @@ -414,7 +414,7 @@ func TestLimitScans(t *testing.T) { // Simulate what the DistSQLReceiver does and ingest the trace. if meta != nil && len(meta.TraceData) > 0 { - sp.ImportRemoteSpans(meta.TraceData) + sp.ImportRemoteRecording(meta.TraceData) } if row == nil && meta == nil { diff --git a/pkg/util/tracing/collector/collector_test.go b/pkg/util/tracing/collector/collector_test.go index cf4217d76a2e..8bf916fad4b0 100644 --- a/pkg/util/tracing/collector/collector_test.go +++ b/pkg/util/tracing/collector/collector_test.go @@ -86,7 +86,7 @@ func setupTraces(t1, t2 *tracing.Tracer) (tracingpb.TraceID, tracingpb.TraceID, // Start another remote child span on "node 2" that we finish. childRemoteChildFinished := t2.StartSpan("root.child.remotechilddone", tracing.WithRemoteParentFromSpanMeta(child.Meta())) - child.ImportRemoteSpans(childRemoteChildFinished.FinishAndGetRecording(tracing.RecordingVerbose)) + child.ImportRemoteRecording(childRemoteChildFinished.FinishAndGetRecording(tracing.RecordingVerbose)) // Start a root span on "node 2". root2 := t2.StartSpan("root2", tracing.WithRecording(tracing.RecordingVerbose)) diff --git a/pkg/util/tracing/crdbspan.go b/pkg/util/tracing/crdbspan.go index 39fa9ee616f2..e8f97768ee89 100644 --- a/pkg/util/tracing/crdbspan.go +++ b/pkg/util/tracing/crdbspan.go @@ -457,22 +457,23 @@ func (s *crdbSpan) getStructuredRecording(includeDetachedChildren bool) Recordin return Recording{res} } -// recordFinishedChildren adds children to s' recording. +// recordFinishedChildren adds the spans in childRecording to s' recording. // -// s takes ownership of children; the caller is not allowed to use them anymore. -func (s *crdbSpan) recordFinishedChildren(children []tracingpb.RecordedSpan) { - if len(children) == 0 { +// s takes ownership of childRecording; the caller is not allowed to use them anymore. +func (s *crdbSpan) recordFinishedChildren(childRecording Recording) { + if len(childRecording) == 0 { return } s.mu.Lock() defer s.mu.Unlock() - s.recordFinishedChildrenLocked(children) + s.recordFinishedChildrenLocked(childRecording) } -// s takes ownership of children; the caller is not allowed to use them anymore. -func (s *crdbSpan) recordFinishedChildrenLocked(children []tracingpb.RecordedSpan) { - if len(children) == 0 { +// s takes ownership of childRecording; the caller is not allowed to use them +// anymore. +func (s *crdbSpan) recordFinishedChildrenLocked(childRecording Recording) { + if len(childRecording) == 0 { return } @@ -480,13 +481,14 @@ func (s *crdbSpan) recordFinishedChildrenLocked(children []tracingpb.RecordedSpa // received, or only the structured events. switch s.recordingType() { case RecordingVerbose: - // Change the root of the remote recording to be a child of this Span. This is + // Change the root of the recording to be a child of this Span. This is // usually already the case, except with DistSQL traces where remote - // processors run in spans that FollowFrom an RPC Span that we don't collect. - children[0].ParentSpanID = s.spanID + // processors run in spans that FollowFrom an RPC Span that we don't + // collect. + childRecording[0].ParentSpanID = s.spanID - if len(s.mu.recording.finishedChildren)+len(children) <= maxRecordedSpansPerTrace { - s.mu.recording.finishedChildren = append(s.mu.recording.finishedChildren, children...) + if len(s.mu.recording.finishedChildren)+len(childRecording) <= maxRecordedSpansPerTrace { + s.mu.recording.finishedChildren = append(s.mu.recording.finishedChildren, childRecording...) break } @@ -494,8 +496,8 @@ func (s *crdbSpan) recordFinishedChildrenLocked(children []tracingpb.RecordedSpa // records by falling through. fallthrough case RecordingStructured: - for ci := range children { - child := &children[ci] + for ci := range childRecording { + child := &childRecording[ci] for i := range child.StructuredRecords { s.recordInternalLocked(&child.StructuredRecords[i], &s.mu.recording.structured) } diff --git a/pkg/util/tracing/grpc_interceptor_test.go b/pkg/util/tracing/grpc_interceptor_test.go index 496175334b92..b2b3b472db7e 100644 --- a/pkg/util/tracing/grpc_interceptor_test.go +++ b/pkg/util/tracing/grpc_interceptor_test.go @@ -252,7 +252,7 @@ func TestGRPCInterceptors(t *testing.T) { var rec tracingpb.RecordedSpan require.NoError(t, types.UnmarshalAny(recAny, &rec)) require.Len(t, rec.StructuredRecords, 1) - sp.ImportRemoteSpans([]tracingpb.RecordedSpan{rec}) + sp.ImportRemoteRecording([]tracingpb.RecordedSpan{rec}) var n int finalRecs := sp.FinishAndGetRecording(tracing.RecordingVerbose) for _, rec := range finalRecs { diff --git a/pkg/util/tracing/span.go b/pkg/util/tracing/span.go index f4924242b994..dbd7f3a19345 100644 --- a/pkg/util/tracing/span.go +++ b/pkg/util/tracing/span.go @@ -352,12 +352,14 @@ func (sp *Span) GetConfiguredRecording() Recording { return sp.i.GetRecording(recType, false /* finishing */) } -// ImportRemoteSpans adds RecordedSpan data to the recording of the given Span; -// these spans will be part of the result of GetRecording. Used to import -// recorded traces from other nodes. -func (sp *Span) ImportRemoteSpans(remoteSpans []tracingpb.RecordedSpan) { +// ImportRemoteRecording adds the spans in remoteRecording as children of the +// receiver. As a result of this, the imported recording will be a part of the +// GetRecording() output for the receiver. +// +// This function is used to import a recording from another node. +func (sp *Span) ImportRemoteRecording(remoteRecording Recording) { if !sp.detectUseAfterFinish() { - sp.i.ImportRemoteSpans(remoteSpans) + sp.i.ImportRemoteRecording(remoteRecording) } } diff --git a/pkg/util/tracing/span_inner.go b/pkg/util/tracing/span_inner.go index 3e5b3c1bc7f8..d1f66b656ac2 100644 --- a/pkg/util/tracing/span_inner.go +++ b/pkg/util/tracing/span_inner.go @@ -87,8 +87,8 @@ func (s *spanInner) GetRecording(recType RecordingType, finishing bool) Recordin return s.crdb.GetRecording(recType, finishing) } -func (s *spanInner) ImportRemoteSpans(remoteSpans []tracingpb.RecordedSpan) { - s.crdb.recordFinishedChildren(remoteSpans) +func (s *spanInner) ImportRemoteRecording(remoteRecording []tracingpb.RecordedSpan) { + s.crdb.recordFinishedChildren(remoteRecording) } func (s *spanInner) Finish() { diff --git a/pkg/util/tracing/span_options.go b/pkg/util/tracing/span_options.go index 6c58a467c04e..23d069bce29a 100644 --- a/pkg/util/tracing/span_options.go +++ b/pkg/util/tracing/span_options.go @@ -231,18 +231,17 @@ type remoteParent SpanMeta // For the purposes of trace recordings, there's no mechanism ensuring that the // child's recording will be passed to the parent span. When that's desired, it // has to be done manually by calling Span.GetRecording() and propagating the -// result to the parent by calling Span.ImportRemoteSpans(). +// result to the parent by calling Span.ImportRemoteRecording(). // // The canonical use case for this is around RPC boundaries, where a server // handling a request wants to create a child span descending from a parent on a // remote machine. // -// node 1 (network) node 2 +// node 1 (network) node 2 // -------------------------------------------------------------------------- -// Span.Meta() ----------> sp2 := Tracer.StartSpan( -// WithRemoteParentFromSpanMeta(.)) -// doSomething(sp2) -// Span.ImportRemoteSpans(.) <---------- sp2.FinishAndGetRecording() +// Span.Meta() ----------> sp2 := Tracer.StartSpan(WithRemoteParentFromSpanMeta(.)) +// doSomething(sp2) +// Span.ImportRemoteRecording(.) <---------- sp2.FinishAndGetRecording() // // By default, the child span is derived using a ChildOf relationship, which // corresponds to the expectation that the parent span will usually wait for the diff --git a/pkg/util/tracing/span_test.go b/pkg/util/tracing/span_test.go index f9c18c737385..aa85234ae7a0 100644 --- a/pkg/util/tracing/span_test.go +++ b/pkg/util/tracing/span_test.go @@ -73,7 +73,7 @@ func TestRecordingString(t *testing.T) { remoteChild.Record("remote child 1") remoteRec := remoteChild.FinishAndGetRecording(RecordingVerbose) - root.ImportRemoteSpans(remoteRec) + root.ImportRemoteRecording(remoteRec) root.Record("root 3") @@ -162,7 +162,7 @@ func TestRecordingInRecording(t *testing.T) { // have to be imported into the parent manually (this would usually happen via // code at the RPC boundaries). grandChild := tr.StartSpan("grandchild", WithParent(child), WithDetachedRecording()) - child.ImportRemoteSpans(grandChild.FinishAndGetRecording(RecordingVerbose)) + child.ImportRemoteRecording(grandChild.FinishAndGetRecording(RecordingVerbose)) childRec := child.FinishAndGetRecording(RecordingVerbose) require.NoError(t, CheckRecordedSpans(childRec, ` span: child @@ -189,7 +189,7 @@ func TestRecordingInRecording(t *testing.T) { // Verify that GetRecording propagates the structured events even when the // receiving Span isn't verbose during import. -func TestImportRemoteSpans(t *testing.T) { +func TestImportRemoteRecording(t *testing.T) { for _, verbose := range []bool{false, true} { t.Run(fmt.Sprintf("%s=%t", "verbose-child=", verbose), func(t *testing.T) { tr := NewTracerWithOpt(context.Background()) @@ -203,7 +203,7 @@ func TestImportRemoteSpans(t *testing.T) { ch := tr.StartSpan("child", WithParent(sp), WithDetachedRecording()) ch.RecordStructured(&types.Int32Value{Value: 4}) ch.Record("foo") - sp.ImportRemoteSpans(ch.FinishAndGetRecording(RecordingVerbose)) + sp.ImportRemoteRecording(ch.FinishAndGetRecording(RecordingVerbose)) if verbose { require.NoError(t, CheckRecording(sp.FinishAndGetRecording(RecordingVerbose), ` @@ -223,7 +223,7 @@ func TestImportRemoteSpans(t *testing.T) { } } -func TestImportRemoteSpansMaintainsRightByteSize(t *testing.T) { +func TestImportRemoteRecordingMaintainsRightByteSize(t *testing.T) { tr1 := NewTracer() child := tr1.StartSpan("child", WithRecording(RecordingStructured)) @@ -231,7 +231,7 @@ func TestImportRemoteSpansMaintainsRightByteSize(t *testing.T) { child.RecordStructured(&types.StringValue{Value: "test"}) root := tr1.StartSpan("root", WithRecording(RecordingStructured)) - root.ImportRemoteSpans(child.GetRecording(RecordingStructured)) + root.ImportRemoteRecording(child.GetRecording(RecordingStructured)) c := root.i.crdb c.mu.Lock() buf := c.mu.recording.structured diff --git a/pkg/util/tracing/tracer_test.go b/pkg/util/tracing/tracer_test.go index 70cc64023f1e..e1e9c04fb6a7 100644 --- a/pkg/util/tracing/tracer_test.go +++ b/pkg/util/tracing/tracer_test.go @@ -330,7 +330,7 @@ func TestTracerInjectExtract(t *testing.T) { t.Fatal(err) } - s1.ImportRemoteSpans(rec) + s1.ImportRemoteRecording(rec) if err := CheckRecordedSpans(s1.FinishAndGetRecording(RecordingVerbose), ` span: a tags: _verbose=1 @@ -492,7 +492,7 @@ func TestTracer_VisitSpans(t *testing.T) { childChild := tr2.StartSpan("root.child.remotechild", WithRemoteParentFromSpanMeta(child.Meta())) childChildFinished := tr2.StartSpan("root.child.remotechilddone", WithRemoteParentFromSpanMeta(child.Meta())) require.Len(t, tr2.activeSpansRegistry.mu.m, 2) - child.ImportRemoteSpans(childChildFinished.FinishAndGetRecording(RecordingVerbose)) + child.ImportRemoteRecording(childChildFinished.FinishAndGetRecording(RecordingVerbose)) require.Len(t, tr2.activeSpansRegistry.mu.m, 1) // All spans are part of the recording (root.child.remotechilddone was @@ -524,7 +524,7 @@ func TestSpanRecordingFinished(t *testing.T) { tr2 := NewTracer() childTraceInfo := child.Meta().ToProto() remoteChildChild := tr2.StartSpan("root.child.remotechild", WithRemoteParentFromTraceInfo(&childTraceInfo)) - child.ImportRemoteSpans(remoteChildChild.GetRecording(RecordingVerbose)) + child.ImportRemoteRecording(remoteChildChild.GetRecording(RecordingVerbose)) remoteChildChild.Finish() // All spans are un-finished.