Skip to content

Commit

Permalink
tracing: remove trace.mode cluster setting
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tbg committed Feb 25, 2021
1 parent 5648691 commit 3d61fbf
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
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
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
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
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
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 3d61fbf

Please sign in to comment.