From 3d61fbfd5f218c3fbd403da253993096fbc6f308 Mon Sep 17 00:00:00 2001 From: Tobias Grieger Date: Thu, 25 Feb 2021 10:50:48 +0100 Subject: [PATCH] tracing: remove trace.mode cluster setting 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 --- pkg/util/tracing/alloc_test.go | 24 ++++++++++++------ pkg/util/tracing/crdbspan.go | 25 ++++++++---------- pkg/util/tracing/span.go | 2 +- pkg/util/tracing/span_test.go | 4 +-- pkg/util/tracing/tracer.go | 46 ---------------------------------- 5 files changed, 29 insertions(+), 72 deletions(-) diff --git a/pkg/util/tracing/alloc_test.go b/pkg/util/tracing/alloc_test.go index 679ceaaf628e..2556e982f0cd 100644 --- a/pkg/util/tracing/alloc_test.go +++ b/pkg/util/tracing/alloc_test.go @@ -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{} @@ -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() @@ -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) diff --git a/pkg/util/tracing/crdbspan.go b/pkg/util/tracing/crdbspan.go index fc67ae677add..ea61f462fb1a 100644 --- a/pkg/util/tracing/crdbspan.go +++ b/pkg/util/tracing/crdbspan.go @@ -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 } @@ -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 } @@ -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 @@ -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, @@ -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 { @@ -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()) }) diff --git a/pkg/util/tracing/span.go b/pkg/util/tracing/span.go index f99994ee0e7f..2206e816df17 100644 --- a/pkg/util/tracing/span.go +++ b/pkg/util/tracing/span.go @@ -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 { diff --git a/pkg/util/tracing/span_test.go b/pkg/util/tracing/span_test.go index ac98e2bef233..7a8446d7edec 100644 --- a/pkg/util/tracing/span_test.go +++ b/pkg/util/tracing/span_test.go @@ -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() @@ -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]) diff --git a/pkg/util/tracing/tracer.go b/pkg/util/tracing/tracer.go index 3f4f247963ad..7fc45b0a137e 100644 --- a/pkg/util/tracing/tracer.go +++ b/pkg/util/tracing/tracer.go @@ -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:///debug/requests", @@ -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 @@ -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 != "" { @@ -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 { @@ -299,9 +256,6 @@ func (t *Tracer) startSpanGeneric( } } - if t.mode() == modeBackground { - opts.ForceRealSpan = true - } if opts.LogTags == nil { opts.LogTags = logtags.FromContext(ctx) }