config.PoolSpans(true) will panic

[philchia]

i create a new tracer with 128bit and pool span options, found that the PoolSpans(true) options may panic my app

cfg.NewTracer(
		config.Gen128Bit(true),
		config.PoolSpans(true),
	)

i walk through the code base, found that

func (t *Tracer) newSpan() *Span {
	if !t.options.poolSpans {
		return &Span{}
	}
	sp := t.spanPool.Get().(*Span)
	sp.context = emptyContext
	sp.tracer = nil
	sp.tags = nil
	sp.logs = nil
	return sp
}

and

func (t *Tracer) reportSpan(sp *Span) {
	t.metrics.SpansFinished.Inc(1)
	if sp.context.IsSampled() {
		t.reporter.Report(sp)
	}
	if t.options.poolSpans {
		t.spanPool.Put(sp)
	}
}

but t.reporter.Report(sp) may be an async operation by put that span into a buffered channel, and then you guys put the span back into the pool, the span is still alive in the channel, this will cause some race condition or nil pointer reference therefore panic the program

[yurishkuro]

Is this with the standard Jaeger reporter or a custom one?
Is 128bit relevant?

[philchia]

128bit not relevant, its the standard jaeger remote reporter

[philchia]

func (r *remoteReporter) Report(span *Span) {
	select {
	case r.queue <- reporterQueueItem{itemType: reporterQueueItemSpan, span: span}:
		atomic.AddInt64(&r.queueLength, 1)
	default:
		r.metrics.ReporterDropped.Inc(1)
	}
}

r.queue is a buffered channel

[guanw]

@philchia You might be interested to see this

[philchia]

@guanw the mr not exists in the latest release

[guanw]

@yurishkuro I guess we need to push a new release for jaeger-client-go?

[yurishkuro]

Fixed in 2.17