Skip to content

Commit

Permalink
Merge #54737
Browse files Browse the repository at this point in the history
54737: tracing: make `span` a union type r=knz,andreimatei a=tbg

Clarify that a `span` is anywhere between zero to three subspans:

- net/trace,
- external opentracing Tracer, and
- crdb-internal trace span,

where zero subspans corresponds to what is today noopSpan (not
implemented yet, but TODO added).

Lots of threads to pull, but this is a good checkpoint since
everything compiles and the diff is mechanical and small enough
to actually review.

Release note: None


Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
  • Loading branch information
3 people committed Oct 23, 2020
2 parents da45387 + 2ace2ab commit 33e3abc
Show file tree
Hide file tree
Showing 28 changed files with 1,478 additions and 1,438 deletions.
3 changes: 2 additions & 1 deletion pkg/bench/ddl_analysis/ddl_analysis_bench.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/opentracing/opentracing-go"
)

Expand Down Expand Up @@ -113,7 +114,7 @@ func countKvBatchRequestsInRecording(r tracing.Recording) int {
return countKvBatchRequestsInSpan(r, root)
}

func countKvBatchRequestsInSpan(r tracing.Recording, sp tracing.RecordedSpan) int {
func countKvBatchRequestsInSpan(r tracing.Recording, sp tracingpb.RecordedSpan) int {
count := 0
// Count the number of OpTxnCoordSender operations while traversing the
// tree of spans.
Expand Down
1,281 changes: 641 additions & 640 deletions pkg/roachpb/api.pb.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pkg/roachpb/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import "roachpb/metadata.proto";
import "storage/enginepb/mvcc.proto";
import "storage/enginepb/mvcc3.proto";
import "util/hlc/timestamp.proto";
import "util/tracing/recorded_span.proto";
import "util/tracing/tracingpb/recorded_span.proto";
import "gogoproto/gogo.proto";

// ReadConsistencyType specifies what type of consistency is observed
Expand Down Expand Up @@ -2100,7 +2100,7 @@ message BatchResponse {
util.hlc.Timestamp now = 5 [(gogoproto.nullable) = false];
// collected_spans stores trace spans recorded during the execution of this
// request.
repeated util.tracing.RecordedSpan collected_spans = 6 [(gogoproto.nullable) = false];
repeated util.tracing.tracingpb.RecordedSpan collected_spans = 6 [(gogoproto.nullable) = false];
// Range or list of ranges used to execute the request. The server only
// populates this if return_range_info is set on the request, or if the
// server detects the client's client_range_info to be stale.
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/errors"
gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"go.etcd.io/etcd/raft"
Expand Down Expand Up @@ -562,7 +562,7 @@ func (s *statusServer) Allocator(
return output, nil
}

func recordedSpansToTraceEvents(spans []tracing.RecordedSpan) []*serverpb.TraceEvent {
func recordedSpansToTraceEvents(spans []tracingpb.RecordedSpan) []*serverpb.TraceEvent {
var output []*serverpb.TraceEvent
var buf bytes.Buffer
for _, sp := range spans {
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/distsql/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ func (ds *ServerImpl) setupFlow(
// TODO(andrei): localState.IsLocal is not quite the right thing to use.
// If that field is unset, we might still want to create a child span if
// this flow is run synchronously.
sp = tracing.StartChildSpan(opName, parentSpan, logtags.FromContext(ctx), false /* separateRecording */)
sp = ds.Tracer.(*tracing.Tracer).StartChildSpan(
opName, parentSpan.(*tracing.Span).SpanContext(), logtags.FromContext(ctx), tracing.NonRecordableSpan,
false /* separateRecording */)
} else {
// We use FollowsFrom because the flow's span outlives the SetupFlow request.
// TODO(andrei): We should use something more efficient than StartSpan; we
Expand Down
15 changes: 8 additions & 7 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/metric"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/cockroachdb/errors"
"github.com/opentracing/opentracing-go"
Expand Down Expand Up @@ -1433,8 +1434,8 @@ func (st *SessionTracing) getSessionTrace() ([]traceRow, error) {
}

// getRecording returns the recorded spans of the current trace.
func (st *SessionTracing) getRecording() []tracing.RecordedSpan {
var spans []tracing.RecordedSpan
func (st *SessionTracing) getRecording() []tracingpb.RecordedSpan {
var spans []tracingpb.RecordedSpan
if st.firstTxnSpan != nil {
spans = append(spans, tracing.GetRecording(st.firstTxnSpan)...)
}
Expand Down Expand Up @@ -1553,7 +1554,7 @@ func (st *SessionTracing) StopTracing() error {
st.showResults = false
st.recordingType = tracing.NoRecording

var spans []tracing.RecordedSpan
var spans []tracingpb.RecordedSpan

if st.firstTxnSpan != nil {
spans = append(spans, tracing.GetRecording(st.firstTxnSpan)...)
Expand Down Expand Up @@ -1726,7 +1727,7 @@ var logMessageRE = regexp.MustCompile(
//
// Note that what's described above is not the order in which SHOW TRACE FOR SESSION
// displays the information: SHOW TRACE will sort by the age column.
func generateSessionTraceVTable(spans []tracing.RecordedSpan) ([]traceRow, error) {
func generateSessionTraceVTable(spans []tracingpb.RecordedSpan) ([]traceRow, error) {
// Get all the log messages, in the right order.
var allLogs []logRecordRow

Expand Down Expand Up @@ -1831,7 +1832,7 @@ func generateSessionTraceVTable(spans []tracing.RecordedSpan) ([]traceRow, error
// getOrderedChildSpans returns all the spans in allSpans that are children of
// spanID. It assumes the input is ordered by start time, in which case the
// output is also ordered.
func getOrderedChildSpans(spanID uint64, allSpans []tracing.RecordedSpan) []spanWithIndex {
func getOrderedChildSpans(spanID uint64, allSpans []tracingpb.RecordedSpan) []spanWithIndex {
children := make([]spanWithIndex, 0)
for i := range allSpans {
if allSpans[i].ParentSpanID == spanID {
Expand All @@ -1853,7 +1854,7 @@ func getOrderedChildSpans(spanID uint64, allSpans []tracing.RecordedSpan) []span
// seenSpans is modified to record all the spans that are part of the subtrace
// rooted at span.
func getMessagesForSubtrace(
span spanWithIndex, allSpans []tracing.RecordedSpan, seenSpans map[uint64]struct{},
span spanWithIndex, allSpans []tracingpb.RecordedSpan, seenSpans map[uint64]struct{},
) ([]logRecordRow, error) {
if _, ok := seenSpans[span.SpanID]; ok {
return nil, errors.Errorf("duplicate span %d", span.SpanID)
Expand Down Expand Up @@ -1940,7 +1941,7 @@ type logRecordRow struct {
}

type spanWithIndex struct {
*tracing.RecordedSpan
*tracingpb.RecordedSpan
index int
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/execinfra/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/cockroachdb/errors"
"github.com/opentracing/opentracing-go"
Expand Down Expand Up @@ -234,7 +235,7 @@ func DrainAndForwardMetadata(ctx context.Context, src RowSource, dst RowReceiver
}

// GetTraceData returns the trace data.
func GetTraceData(ctx context.Context) []tracing.RecordedSpan {
func GetTraceData(ctx context.Context) []tracingpb.RecordedSpan {
if sp := opentracing.SpanFromContext(ctx); sp != nil {
return tracing.GetRecording(sp)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/execinfrapb/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/encoding"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/logtags"
)
Expand Down Expand Up @@ -179,7 +179,7 @@ type ProducerMetadata struct {
// TODO(vivek): change to type Error
Err error
// TraceData is sent if snowball tracing is enabled.
TraceData []tracing.RecordedSpan
TraceData []tracingpb.RecordedSpan
// LeafTxnFinalState contains the final state of the LeafTxn to be
// sent from leaf flows to the RootTxn held by the flow's ultimate
// receiver.
Expand Down
Loading

0 comments on commit 33e3abc

Please sign in to comment.