Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data Race: exemplars share rand source without synchronization #5455

Closed
rodaine opened this issue May 31, 2024 · 0 comments · Fixed by #5456
Closed

Data Race: exemplars share rand source without synchronization #5455

rodaine opened this issue May 31, 2024 · 0 comments · Fixed by #5456
Assignees
Labels
bug Something isn't working
Milestone

Comments

@rodaine
Copy link

rodaine commented May 31, 2024

Description

exemplar.random() is used concurrently resulting in a data race (below). Access to a rand.Rand is not safe for concurrent use per the documentation:

Random numbers are generated by a Source, usually wrapped in a Rand. Both types should be used by a single goroutine at a time: sharing among multiple goroutines requires some kind of synchronization.

The resulting race:

WARNING: DATA RACE
Read at 0x00c0001ae000 by goroutine 28650:
  math/rand.(*rngSource).Uint64()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rng.go:239 +0x34
  math/rand.(*rngSource).Int63()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rng.go:234 +0x30
  math/rand.(*Rand).Int63()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rand.go:96 +0x4c
  math/rand.(*Rand).Float64()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rand.go:207 +0x38
  go.opentelemetry.io/otel/sdk/metric/internal/exemplar.random()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/exemplar/rand.go:41 +0x40
  go.opentelemetry.io/otel/sdk/metric/internal/exemplar.(*randRes).reset()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/exemplar/rand.go:142 +0x94
  go.opentelemetry.io/otel/sdk/metric/internal/exemplar.FixedSize()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/exemplar/rand.go:54 +0xf0
  go.opentelemetry.io/otel/sdk/metric.reservoirFunc.func1.2()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/exemplar.go:65 +0x34
  go.opentelemetry.io/otel/sdk/metric.reservoirFunc.func2()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/exemplar.go:82 +0x34
  go.opentelemetry.io/otel/sdk/metric/internal/aggregate.(*valueMap[go.shape.int64]).measure()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/aggregate/sum.go:47 +0x314
  go.opentelemetry.io/otel/sdk/metric/internal/aggregate.Builder[go.shape.int64].Sum.func3()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/aggregate/aggregate.go:119 +0xc8
  go.opentelemetry.io/otel/sdk/metric/internal/aggregate.Builder[go.shape.int64].filter.func2()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/aggregate/aggregate.go:72 +0xa8
  go.opentelemetry.io/otel/sdk/metric.(*int64Inst).aggregate()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/instrument.go:204 +0xf4
  go.opentelemetry.io/otel/sdk/metric.(*int64Inst).Add()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/instrument.go:194 +0xc0

Previous write at 0x00c0001ae000 by goroutine 28621:
  math/rand.(*rngSource).Uint64()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rng.go:239 +0x54
  math/rand.(*rngSource).Int63()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rng.go:234 +0x30
  math/rand.(*Rand).Int63()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rand.go:96 +0x4c
  math/rand.(*Rand).Float64()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/math/rand/rand.go:207 +0x38
  go.opentelemetry.io/otel/sdk/metric/internal/exemplar.random()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/exemplar/rand.go:41 +0x40
  go.opentelemetry.io/otel/sdk/metric/internal/exemplar.(*randRes).reset()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/exemplar/rand.go:142 +0x94
  go.opentelemetry.io/otel/sdk/metric/internal/exemplar.FixedSize()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/exemplar/rand.go:54 +0xf0
  go.opentelemetry.io/otel/sdk/metric.reservoirFunc.func1.2()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/exemplar.go:65 +0x34
  go.opentelemetry.io/otel/sdk/metric.reservoirFunc.func2()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/exemplar.go:82 +0x34
  go.opentelemetry.io/otel/sdk/metric/internal/aggregate.(*valueMap[go.shape.int64]).measure()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/aggregate/sum.go:47 +0x314
  go.opentelemetry.io/otel/sdk/metric/internal/aggregate.Builder[go.shape.int64].Sum.func3()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/aggregate/aggregate.go:119 +0xc8
  go.opentelemetry.io/otel/sdk/metric/internal/aggregate.Builder[go.shape.int64].filter.func2()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/internal/aggregate/aggregate.go:72 +0xa8
  go.opentelemetry.io/otel/sdk/metric.(*int64Inst).aggregate()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/instrument.go:204 +0xf4
  go.opentelemetry.io/otel/sdk/metric.(*int64Inst).Add()
      /Users/rodaine/pkg/mod/go.opentelemetry.io/otel/sdk/metric@v1.27.0/instrument.go:194 +0xc0

Environment

  • OS: macOS Sonoma 14.5 (darwin)
  • Architecture: Apple M1 (arm64)
  • Go Version: 1.22.3
  • opentelemetry-go version: 1.27.0

Steps To Reproduce

  1. Enable trace exemplars with OTEL_GO_X_EXEMPLAR
  2. Have some concurrent code that uses both traces and int64 counters concurrently
  3. Build (or test) with the -race flag enabled.

Expected behavior

No data race; likely via adding a mutex to exemplar.random similar to the math/rand package-level functions.

@rodaine rodaine added the bug Something isn't working label May 31, 2024
@MrAlias MrAlias self-assigned this May 31, 2024
@MrAlias MrAlias added this to the v1.28.0 milestone May 31, 2024
MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue May 31, 2024
pellared pushed a commit that referenced this issue Jun 3, 2024
Fix #5455

The `math/rand.Rand` type is not safe for concurrent access. Concurrent
measurements, and therefore concurrent exemplar computation, are
allowed. Ensure this concurrent design does not lead to data races with
`rng`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants