Data race in simple span processor

[brian-gavin]

Description

The Go race detector finds a data race in the following code:

package main

import (
	"context"
	"sync"

	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/exporters/jaeger"
	"go.opentelemetry.io/otel/sdk/trace"
)

func init() {
	jaegerExporter, err := jaeger.New(jaeger.WithAgentEndpoint())
	if err != nil {
		panic(err)
	}
	tp := trace.NewTracerProvider(trace.WithSyncer(jaegerExporter))
	otel.SetTracerProvider(tp)
}

func main() {
	var wg sync.WaitGroup
	wg.Add(2)
	go func() {
		_, span := otel.Tracer("test").Start(context.Background(), "A")
		defer span.End()
		wg.Done()
	}()
	go func() {
		_, span := otel.Tracer("test").Start(context.Background(), "B")
		defer span.End()
		wg.Done()
	}()
	wg.Wait()
}

Running with -race will print (at least):

==================
WARNING: DATA RACE
Read at 0x00c00007f2c0 by goroutine 9:
  bytes.(*Buffer).Reset()
      /usr/local/go/src/bytes/buffer.go:98 +0x87
  go.opentelemetry.io/otel/exporters/jaeger.(*agentClientUDP).calcSizeOfSerializedThrift()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/agent.go:205 +0x47
  go.opentelemetry.io/otel/exporters/jaeger.(*agentClientUDP).EmitBatch()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/agent.go:125 +0x93
  go.opentelemetry.io/otel/exporters/jaeger.(*agentUploader).upload()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/uploader.go:283 +0x59
  go.opentelemetry.io/otel/exporters/jaeger.(*Exporter).ExportSpans()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/jaeger.go:105 +0x491
  go.opentelemetry.io/otel/sdk/trace.(*simpleSpanProcessor).OnEnd()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/simple_span_processor.go:58 +0x23e
  go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:419 +0xc3d
  main.main.func1.1()
      /Users/brian.gavin/code/otel-jaeger-race/main.go:26 +0x52
  runtime.deferreturn()
      /usr/local/go/src/runtime/panic.go:476 +0x32

Previous write at 0x00c00007f2c0 by goroutine 10:
  bytes.(*Buffer).tryGrowByReslice()
      /usr/local/go/src/bytes/buffer.go:108 +0xb3
  bytes.(*Buffer).Write()
      /usr/local/go/src/bytes/buffer.go:168 +0x18
  go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift.(*TMemoryBuffer).Write()
      <autogenerated>:1 +0x6e
  go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift.(*TCompactProtocol).writeVarint32()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/internal/third_party/thrift/lib/go/thrift/compact_protocol.go:691 +0xea
  go.opentelemetry.io/otel/exporters/jaeger/internal/third_party/thrift/lib/go/thrift.(*TCompactProtocol).WriteString()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/internal/third_party/thrift/lib/go/thrift/compact_protocol.go:333 +0x44
  go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger.(*Process).writeField1()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/internal/gen-go/jaeger/jaeger.go:1894 +0xef
  go.opentelemetry.io/otel/exporters/jaeger/internal/gen-go/jaeger.(*Process).Write()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/internal/gen-go/jaeger/jaeger.go:1874 +0xc8
  go.opentelemetry.io/otel/exporters/jaeger.(*agentClientUDP).calcSizeOfSerializedThrift()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/agent.go:206 +0x115
  go.opentelemetry.io/otel/exporters/jaeger.(*agentClientUDP).EmitBatch()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/agent.go:125 +0x93
  go.opentelemetry.io/otel/exporters/jaeger.(*agentUploader).upload()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/uploader.go:283 +0x59
  go.opentelemetry.io/otel/exporters/jaeger.(*Exporter).ExportSpans()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/exporters/[email protected]/jaeger.go:105 +0x491
  go.opentelemetry.io/otel/sdk/trace.(*simpleSpanProcessor).OnEnd()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/simple_span_processor.go:58 +0x23e
  go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End()
      /Users/brian.gavin/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:419 +0xc3d
  main.main.func2.1()
      /Users/brian.gavin/code/otel-jaeger-race/main.go:31 +0x52
  runtime.deferreturn()
      /usr/local/go/src/runtime/panic.go:476 +0x32

Goroutine 9 (running) created at:
  main.main()
      /Users/brian.gavin/code/otel-jaeger-race/main.go:24 +0xce

Goroutine 10 (running) created at:
  main.main()
      /Users/brian.gavin/code/otel-jaeger-race/main.go:29 +0x136
==================

This code can be found at https://github.com/brian-gavin/otel-jaeger-race.

Thanks!

Environment

• OS: macOS 13.1
• Architecture: x86_64
• Go Version: 1.20
• opentelemetry-go version: v1.14.0

Steps To Reproduce

1. git clone [email protected]:brian-gavin/otel-jaeger-race.git
2. cd otel-jaeger-race
3. go run -race .
4. See error

Expected behavior

For no race to be detected.

[Kaushal28]

Hey @MrAlias, I would like to contribute here, could you please provide me with any entry point from where I can start?

[pellared]

@Kaushal28 answering before @MrAlias gets up 😉

1. Here is the source code https://github.com/open-telemetry/opentelemetry-go/tree/main/exporters/jaeger
2. I suggest starting with writing a unit test that would reproduce the race condition
3. See the contributing documentation

Do you need any more information?

[Lim3nius]

Hi, please, could someone either fix title or some other way make it visible, that bug was in go.opentelemetry.io/otel/sdk/trace.NewSimpleSpanProcessor and in WithSyncer(), and not in package exporters/jaeger? Thank you.