Multiple NewTracerProvider calls with the same batcher cause data race

[SOF3]

Description

It is clearly documented that ExponentialBackoff is not thread-safe:

https://github.com/cenkalti/backoff/blob/a214dad550bb72db341d655094a496034bbf7e0a/exponential.go#L52

However it is used in retry.go:

opentelemetry-go/exporters/otlp/internal/retry/retry.go

Line 105 in ecf0838

bOff := b.NextBackOff()

The value b is scoped at RequestFunc instead of the returned closure. Reuse of the returned closure in multiple goroutines results in data race. The RequestFunc return value is used in

opentelemetry-go/exporters/otlp/otlptrace/otlptracegrpc/client.go

Line 200 in ecf0838

return c.requestFunc(ctx, func(iCtx context.Context) error {

It is not documented anywhere that otelsdktrace.WithBatcher shall not be called more than once on the same exporter.

Environment

• OS: Linux 5.4.56 Debian
• Architecture: amd64
• Go Version: 1.19
• opentelemetry-go version: go.opentelemetry.io/otel/sdk@v1.11.1
• cenkalti/backoff/v4@v4.1.3

Steps To Reproduce

Compile with go run -race, create multiple new tracer providers with WithBatcher(exporter) with the same exporter.

  github.com/cenkalti/backoff/v4.(*ExponentialBackOff).NextBackOff()
      /home/chankyin/go/pkg/mod/github.com/cenkalti/backoff/v4@v4.1.3/exponential.go:119 +0x74
  go.opentelemetry.io/otel/exporters/otlp/internal/retry.Config.RequestFunc.func2()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/exporters/otlp/internal/retry@v1.11.1/retry.go:105 +0xb6
  go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc.(*client).UploadTraces()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc@v1.11.1/client.go:200 +0x266
  go.opentelemetry.io/otel/exporters/otlp/otlptrace.(*Exporter).ExportSpans()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/exporters/otlp/otlptrace@v1.11.1/exporter.go:48 +0xc6
  go.opentelemetry.io/otel/sdk/trace.(*batchSpanProcessor).exportSpans()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:269 +0x427
  go.opentelemetry.io/otel/sdk/trace.(*batchSpanProcessor).processQueue()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:297 +0x5a4
  go.opentelemetry.io/otel/sdk/trace.NewBatchSpanProcessor.func1()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:128 +0x72

Previous write at 0x00c0001c6d00 by goroutine 537:
  github.com/cenkalti/backoff/v4.(*ExponentialBackOff).incrementCurrentInterval()
      /home/chankyin/go/pkg/mod/github.com/cenkalti/backoff/v4@v4.1.3/exponential.go:143 +0x1bd
  github.com/cenkalti/backoff/v4.(*ExponentialBackOff).NextBackOff()
      /home/chankyin/go/pkg/mod/github.com/cenkalti/backoff/v4@v4.1.3/exponential.go:120 +0xce
  go.opentelemetry.io/otel/exporters/otlp/internal/retry.Config.RequestFunc.func2()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/exporters/otlp/internal/retry@v1.11.1/retry.go:105 +0xb6
  go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc.(*client).UploadTraces()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc@v1.11.1/client.go:200 +0x266
  go.opentelemetry.io/otel/exporters/otlp/otlptrace.(*Exporter).ExportSpans()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/exporters/otlp/otlptrace@v1.11.1/exporter.go:48 +0xc6
  go.opentelemetry.io/otel/sdk/trace.(*batchSpanProcessor).exportSpans()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:269 +0x427
  go.opentelemetry.io/otel/sdk/trace.(*batchSpanProcessor).processQueue()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:297 +0x5a4
  go.opentelemetry.io/otel/sdk/trace.NewBatchSpanProcessor.func1()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:128 +0x72

Goroutine 538 (running) created at:
  go.opentelemetry.io/otel/sdk/trace.NewBatchSpanProcessor()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:126 +0x52e
  go.opentelemetry.io/otel/sdk/trace.WithBatcher()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/provider.go:287 +0x58
--- omitted ---

Goroutine 537 (running) created at:
  go.opentelemetry.io/otel/sdk/trace.NewBatchSpanProcessor()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/batch_span_processor.go:126 +0x52e
  go.opentelemetry.io/otel/sdk/trace.WithBatcher()
      /home/chankyin/go/pkg/mod/go.opentelemetry.io/otel/sdk@v1.11.1/trace/provider.go:287 +0x58
--- omitted ---

Equivalent go code:

exporter, _ := otlptrace.New(ctx, otlptracegrpc.NewClient())
tp1 := otelsdktrace.NewTracerProvider(otelsdktrace.WithBatcher(exporter))
tp2 := otelsdktrace.NewTracerProvider(otelsdktrace.WithBatcher(exporter))

[dmathieu]

Thank you for reporting this. I have opened a fix in #3756.