tracing: better basictracer options

We added log limits to basictracer in opentracing/basictracer-go#39
This change sets better options for our tracers:
 - drop all logs for spans which we are not recording (those that go net/trace)
 - increase the default value of the new maxLogsPerSpan setting to 1000

Fixes cockroachdb#9529.

GLOCKFILE

@@ -74,7 +74,7 @@ github.com/kkaneda/returncheck bf081fa7155e3a27df1f056a49d50685edfa5b1b
 github.com/kr/pretty 737b74a46c4bf788349f72cb256fed10aea4d0ac
 github.com/kr/text 7cafcd837844e784b526369c9bce262804aebc60
 github.com/lib/pq 80f8150043c80fb52dee6bc863a709cdac7ec8f8
-github.com/lightstep/lightstep-tracer-go 8d59645adbb3c85f28ae340a055a9349cdd99fe7
+github.com/lightstep/lightstep-tracer-go 7ec5005048fddb1fc15627e1bf58796ce01d919e
 github.com/mattn/go-isatty 66b8e73f3f5cda9f96b69efd03dd3d7fc4a5cdb8
 github.com/mattn/go-runewidth d6bea18f789704b5f83375793155289da36a3c7f
 github.com/mattn/goveralls f4d273b02ce1b4e48acf3662b717aa987bfc4118
@@ -85,7 +85,7 @@ github.com/montanaflynn/stats 60dcacf48f43d6dd654d0ed94120ff5806c5ca5c
 github.com/olekukonko/tablewriter daf2955e742cf123959884fdff4685aa79b63135
 github.com/opencontainers/runc ae7a92e352116f38bce3f7cb3aa2bc590a7e3f65
 github.com/opennota/check 5b00aacd5639507d2b039245a278ec9f5505509f
-github.com/opentracing/basictracer-go 6dea169035b81f39f8f157256225ba02ca7104ab
+github.com/opentracing/basictracer-go 1b32af207119a14b1b231d451df3ed04a72efebf
 github.com/opentracing/opentracing-go 30dda9350627161ff15581c0bdc504e32ec9a536
 github.com/pborman/uuid c55201b036063326c5b1b89ccfe45a184973d073
 github.com/peterbourgon/g2s 5767a0b2078638d14800683fd0fe425604883f63

util/tracing/tracer.go

@@ -36,6 +36,9 @@ import (
 // Snowball is set as Baggage on traces which are used for snowball tracing.
 const Snowball = "sb"
 
+// maxLogsPerSpan limits the number of logs in a Span; use a comfortable limit.
+const maxLogsPerSpan = 1000
+
 // A CallbackRecorder immediately invokes itself on received trace spans.
 type CallbackRecorder func(sp basictracer.RawSpan)
 
@@ -69,10 +72,15 @@ func JoinOrNew(tr opentracing.Tracer, carrier *Span, opName string) (opentracing
 // JoinOrNewSnowball returns a Span which records directly via the specified
 // callback. If the given DelegatingCarrier is nil, a new Span is created.
 // otherwise, the created Span is a child.
+//
+// The recorder should be nil if we don't need to record spans.
+//
 // TODO(andrei): JoinOrNewSnowball creates a new tracer, which is not kosher.
 // Also this can't use the lightstep tracer.
-func JoinOrNewSnowball(opName string, carrier *Span, callback func(sp basictracer.RawSpan)) (opentracing.Span, error) {
-	tr := basictracer.NewWithOptions(defaultOptions(callback))
+func JoinOrNewSnowball(
+	opName string, carrier *Span, recorder func(sp basictracer.RawSpan),
+) (opentracing.Span, error) {
+	tr := basictracer.NewWithOptions(basictracerOptions(recorder))
 	sp, err := JoinOrNew(tr, carrier, opName)
 	if err == nil {
 		// We definitely want to sample a Snowball trace.
@@ -89,7 +97,7 @@ func JoinOrNewSnowball(opName string, carrier *Span, callback func(sp basictrace
 func NewTracerAndSpanFor7881(
 	callback func(sp basictracer.RawSpan),
 ) (opentracing.Span, opentracing.Tracer, error) {
-	opts := defaultOptions(callback)
+	opts := basictracerOptions(callback)
 	// Don't trim the logs in "unsampled" spans". Note that this tracer does not
 	// use sampling; instead it uses an ad-hoc mechanism for marking spans of
 	// interest.
@@ -119,13 +127,24 @@ func ForkCtxSpan(ctx context.Context, opName string) (context.Context, func()) {
 	return ctx, func() {}
 }
 
-func defaultOptions(recorder func(basictracer.RawSpan)) basictracer.Options {
+// basicTracerOptions initializes options for basictracer.
+// The recorder should be nil if we don't need to record spans.
+func basictracerOptions(recorder func(basictracer.RawSpan)) basictracer.Options {
 	opts := basictracer.DefaultOptions()
 	opts.ShouldSample = func(traceID uint64) bool { return false }
 	opts.TrimUnsampledSpans = true
-	opts.Recorder = CallbackRecorder(recorder)
 	opts.NewSpanEventListener = events.NetTraceIntegrator
 	opts.DebugAssertUseAfterFinish = true // provoke crash on use-after-Finish
+	if recorder == nil {
+		opts.Recorder = CallbackRecorder(func(_ basictracer.RawSpan) {})
+		// If we are not recording the spans, there is no need to keep them in
+		// memory. Events still get passed to the NetTraceIntegrator.
+		opts.DropAllLogs = true
+	} else {
+		opts.Recorder = CallbackRecorder(recorder)
+		// Set a comfortable limit of log events per span.
+		opts.MaxLogsPerSpan = maxLogsPerSpan
+	}
 	return opts
 }
 
@@ -138,16 +157,19 @@ var lightstepOnly = envutil.EnvOrDefaultBool("COCKROACH_LIGHTSTEP_ONLY", false)
 // newTracer implements NewTracer and allows that function to be mocked out via Disable().
 var newTracer = func() opentracing.Tracer {
 	if lightstepToken != "" {
-		lsTr := lightstep.NewTracer(lightstep.Options{AccessToken: lightstepToken})
+		lsTr := lightstep.NewTracer(lightstep.Options{
+			AccessToken:    lightstepToken,
+			MaxLogsPerSpan: maxLogsPerSpan,
+		})
 		if lightstepOnly {
 			return lsTr
 		}
-		basicTr := basictracer.NewWithOptions(defaultOptions(func(_ basictracer.RawSpan) {}))
+		basicTr := basictracer.NewWithOptions(basictracerOptions(nil))
 		// The TeeTracer uses the first tracer for serialization of span contexts;
 		// lightspan needs to be first because it correlates spans between nodes.
 		return NewTeeTracer(lsTr, basicTr)
 	}
-	return basictracer.NewWithOptions(defaultOptions(func(_ basictracer.RawSpan) {}))
+	return basictracer.NewWithOptions(basictracerOptions(nil))
 }
 
 // NewTracer creates a Tracer which records to the net/trace