Skip to content

Commit

Permalink
tracing: better basictracer options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
RaduBerinde committed Sep 30, 2016
1 parent 0796fae commit 651edf2
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
2 changes: 1 addition & 1 deletion GLOCKFILE
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 27 additions & 7 deletions util/tracing/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
// basciTracerOptions 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
}

Expand All @@ -138,16 +157,17 @@ 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 != "" {
// TODO(radu): set maxLogsPerSpan in lightstep when it is available.
lsTr := lightstep.NewTracer(lightstep.Options{AccessToken: lightstepToken})
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
Expand Down

0 comments on commit 651edf2

Please sign in to comment.