Skip to content

Commit

Permalink
Merge cockroachdb#61115
Browse files Browse the repository at this point in the history 
61115: tracing: remove trace.mode cluster setting r=knz a=tbg

This was added experimentally in this release cycle and was determined
to not be suitable for inclusion in the release.

Release justification: remove functionality unsuitable for release
Release note: None


Co-authored-by: Tobias Grieger <[email protected]>
  • Loading branch information
@tbg
craig[bot] and tbg committed Feb 25, 2021
2 parents 5648691 + 3d61fbf commit 65ce294
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 72 deletions.
24 changes: 16 additions & 8 deletions pkg/util/tracing/alloc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ func BenchmarkTracer_StartSpanCtx(b *testing.B) {

tr := NewTracer()
sv := settings.Values{}
tracingMode.Override(&sv, int64(modeBackground))
tr.Configure(&sv)

staticLogTags := logtags.Buffer{}
Expand All @@ -50,10 +49,21 @@ func BenchmarkTracer_StartSpanCtx(b *testing.B) {
opts []SpanOption
}{
{"none", nil},
{"logtag", []SpanOption{WithLogTags(&staticLogTags)}},
{"tag", []SpanOption{WithTags(staticTag)}},
{"autoparent", []SpanOption{WithParentAndAutoCollection(parSp)}},
{"manualparent", []SpanOption{WithParentAndManualCollection(parSp.Meta())}},
{"real", []SpanOption{
WithForceRealSpan(),
}},
{"real,logtag", []SpanOption{
WithForceRealSpan(), WithLogTags(&staticLogTags),
}},
{"real,tag", []SpanOption{
WithForceRealSpan(), WithTags(staticTag),
}},
{"real,autoparent", []SpanOption{
WithForceRealSpan(), WithParentAndAutoCollection(parSp),
}},
{"real,manualparent", []SpanOption{
WithForceRealSpan(), WithParentAndManualCollection(parSp.Meta()),
}},
} {
b.Run(fmt.Sprintf("opts=%s", tc.name), func(b *testing.B) {
b.ReportAllocs()
Expand All @@ -67,11 +77,9 @@ func BenchmarkTracer_StartSpanCtx(b *testing.B) {

}

// BenchmarkSpan_GetRecording microbenchmarks GetRecording
// when background tracing is enabled.
// BenchmarkSpan_GetRecording microbenchmarks GetRecording.
func BenchmarkSpan_GetRecording(b *testing.B) {
var sv settings.Values
tracingMode.Override(&sv, int64(modeBackground))
tr := NewTracer()
tr.Configure(&sv)

Expand Down
25 changes: 11 additions & 14 deletions pkg/util/tracing/crdbspan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (s *crdbSpan) disableRecording() {
}
}

func (s *crdbSpan) getRecording(m mode, everyoneIsV211 bool) Recording {
func (s *crdbSpan) getRecording(everyoneIsV211 bool) Recording {
if s == nil {
return nil // noop span
}
Expand All @@ -157,7 +157,7 @@ func (s *crdbSpan) getRecording(m mode, everyoneIsV211 bool) Recording {
// that v20.2 is not around any longer.
//
// TODO(tbg): remove this in the v21.2 cycle.
if m == modeLegacy && s.recordingType() == RecordingOff {
if s.recordingType() == RecordingOff {
s.mu.Unlock()
return nil
}
Expand All @@ -168,12 +168,12 @@ func (s *crdbSpan) getRecording(m mode, everyoneIsV211 bool) Recording {
result := make(Recording, 0, 1+len(s.mu.recording.children)+len(s.mu.recording.remoteSpans))
// Shallow-copy the children so we can process them without the lock.
children := s.mu.recording.children
result = append(result, s.getRecordingLocked(m))
result = append(result, s.getRecordingLocked())
result = append(result, s.mu.recording.remoteSpans...)
s.mu.Unlock()

for _, child := range children {
result = append(result, child.getRecording(m, everyoneIsV211)...)
result = append(result, child.getRecording(everyoneIsV211)...)
}

// Sort the spans by StartTime, except the first Span (the root of this
Expand Down Expand Up @@ -257,7 +257,7 @@ func (s *crdbSpan) setBaggageItemLocked(restrictedKey, value string) {

// getRecordingLocked returns the Span's recording. This does not include
// children.
func (s *crdbSpan) getRecordingLocked(m mode) tracingpb.RecordedSpan {
func (s *crdbSpan) getRecordingLocked() tracingpb.RecordedSpan {
rs := tracingpb.RecordedSpan{
TraceID: s.traceID,
SpanID: s.spanID,
Expand Down Expand Up @@ -287,14 +287,11 @@ func (s *crdbSpan) getRecordingLocked(m mode) tracingpb.RecordedSpan {

// When nobody is configured to see our spans, skip some allocations
// related to Span UX improvements.
onlyBackgroundTracing := m == modeBackground && s.recordingType() == RecordingOff
if !onlyBackgroundTracing {
if s.mu.duration == -1 {
addTag("_unfinished", "1")
}
if s.mu.recording.recordingType.load() == RecordingVerbose {
addTag("_verbose", "1")
}
if s.mu.duration == -1 {
addTag("_unfinished", "1")
}
if s.mu.recording.recordingType.load() == RecordingVerbose {
addTag("_verbose", "1")
}

if s.mu.stats != nil {
Expand Down Expand Up @@ -325,7 +322,7 @@ func (s *crdbSpan) getRecordingLocked(m mode) tracingpb.RecordedSpan {
rs.Baggage[k] = v
}
}
if !onlyBackgroundTracing && s.logTags != nil {
if s.logTags != nil {
setLogTags(s.logTags.Get(), func(remappedKey string, tag *logtags.Tag) {
addTag(remappedKey, tag.ValueStr())
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/tracing/span.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func (s *spanInner) ResetRecording() {
}

func (s *spanInner) GetRecording() Recording {
return s.crdb.getRecording(s.tracer.mode(), s.tracer.TracingVerbosityIndependentSemanticsIsActive())
return s.crdb.getRecording(s.tracer.TracingVerbosityIndependentSemanticsIsActive())
}

func (s *spanInner) ImportRemoteSpans(remoteSpans []tracingpb.RecordedSpan) error {
Expand Down
4 changes: 1 addition & 3 deletions pkg/util/tracing/span_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ func TestSpanRecordStructured(t *testing.T) {

func TestSpanRecordStructuredLimit(t *testing.T) {
tr := NewTracer()
tr._mode = int32(modeBackground)
sp := tr.StartSpan("root", WithForceRealSpan())
defer sp.Finish()

Expand Down Expand Up @@ -243,10 +242,9 @@ func TestSpanRecordStructuredLimit(t *testing.T) {

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())
ch := tr.StartSpan("child", WithParentAndAutoCollection(sp))
defer ch.Finish()
require.Len(t, sp.i.crdb.mu.recording.children, 1)
require.Equal(t, ch.i.crdb, sp.i.crdb.mu.recording.children[0])
Expand Down
46 changes: 0 additions & 46 deletions pkg/util/tracing/tracer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,42 +66,6 @@ const (
fieldNameShadowType = prefixTracerState + "shadowtype"
)

type mode int32

const (
modeLegacy mode = iota
modeBackground
)

// tracingMode informs the creation of noop spans and the default recording mode
// of created spans.
//
// If set to 'background', trace spans will be created for all operations, but
// these will record sparse structured information, unless an operation
// explicitly requests the verbose from. It's optimized for low overhead, and
// powers fine-grained statistics and alerts.
//
// If set to 'legacy', trace spans will not be created by default. This is
// unless an internal code path explicitly requests for it, or if an auxiliary
// tracer (such as lightstep or zipkin) is configured. This tracing mode always
// records in the verbose form. Using this mode has two effects: the
// observability of the cluster may be degraded (as most trace spans are elided)
// and where trace spans are created, they may consume large amounts of memory.
//
// Note that regardless of this setting, configuring an auxiliary trace sink
// will cause verbose traces to be created for all operations, which may lead to
// high memory consumption. It is not currently possible to send non-verbose
// traces to auxiliary sinks.
var tracingMode = settings.RegisterEnumSetting(
"trace.mode",
"if set to 'background', traces will be created for all operations (in"+
"'legacy' mode it's created when explicitly requested or when auxiliary tracers are configured)",
"legacy",
map[int64]string{
int64(modeLegacy): "legacy",
int64(modeBackground): "background",
})

var enableNetTrace = settings.RegisterBoolSetting(
"trace.debug.enable",
"if set, traces for recent requests can be seen at https://<ui>/debug/requests",
Expand Down Expand Up @@ -143,8 +107,6 @@ type Tracer struct {
// x/net/trace or lightstep and we are not recording.
noopSpan *Span

_mode int32 // modeLegacy or modeBackground, accessed atomically

// True if tracing to the debug/requests endpoint. Accessed via t.useNetTrace().
_useNetTrace int32 // updated atomically

Expand Down Expand Up @@ -198,7 +160,6 @@ func NewTracer() *Tracer {
// it updated if they change).
func (t *Tracer) Configure(sv *settings.Values) {
reconfigure := func() {
atomic.StoreInt32(&t._mode, int32(tracingMode.Get(sv)))
if lsToken := lightstepToken.Get(sv); lsToken != "" {
t.setShadowTracer(createLightStepTracer(lsToken))
} else if zipkinAddr := zipkinCollector.Get(sv); zipkinAddr != "" {
Expand Down Expand Up @@ -272,10 +233,6 @@ func (t *Tracer) StartSpanCtx(
return t.startSpanGeneric(ctx, operationName, opts)
}

func (t *Tracer) mode() mode {
return mode(atomic.LoadInt32(&t._mode))
}

// AlwaysTrace returns true if operations should be traced regardless of the
// context.
func (t *Tracer) AlwaysTrace() bool {
Expand All @@ -299,9 +256,6 @@ func (t *Tracer) startSpanGeneric(
}
}

if t.mode() == modeBackground {
opts.ForceRealSpan = true
}
if opts.LogTags == nil {
opts.LogTags = logtags.FromContext(ctx)
}
Expand Down

0 comments on commit 65ce294

Please sign in to comment.