Missing lock around rng access in rand.go #5814
Comments
pkwarren
added a commit
to pkwarren/opentelemetry-go
that referenced
this issue
Sep 12, 2024
Update rand.Rand usage to add locking around a global rand.Rand instance, which is not safe for concurrent access by multiple goroutines. Fixes open-telemetry#5814.
pkwarren
added a commit
to pkwarren/opentelemetry-go
that referenced
this issue
Sep 12, 2024
Update rand.Rand usage to add locking around a global rand.Rand instance, which is not safe for concurrent access by multiple goroutines. Fixes open-telemetry#5814.
|
For those also looking into this, this looks to be cause by an |
|
Reproduction: package metric // import "go.opentelemetry.io/otel/sdk/metric"
import (
"context"
"sync"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
)
func TestRandomExemplarConcurrentSafe(t *testing.T) {
t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "always_on")
r := NewManualReader()
p := NewMeterProvider(WithReader(r))
m := p.Meter("exemplar-concurrency")
// Use two instruments to get concurrent access to any shared globals.
i0, err := m.Int64Counter("counter.0")
require.NoError(t, err)
i1, err := m.Int64Counter("counter.1")
require.NoError(t, err)
ctx := context.Background()
const goRoutines = 10
var wg sync.WaitGroup
wg.Add(goRoutines)
done := make(chan struct{})
go func() {
wg.Wait()
close(done)
}()
go func() {
select {
case <-done:
return
default:
}
var out metricdata.ResourceMetrics
require.NotPanics(t, func() {
require.NoError(t, r.Collect(ctx, &out))
})
}()
for n := 0; n < goRoutines; n++ {
go func() {
defer wg.Done()
require.NotPanics(t, func() {
i0.Add(ctx, 1)
i1.Add(ctx, 2)
})
}()
}
<-done
var out metricdata.ResourceMetrics
require.NoError(t, r.Collect(ctx, &out))
p.Shutdown(ctx)
}The key here is the duplicate instruments (i.e. This does not fail reliably with only one test run. Need to build multiple collections into this test to cause it to do so. |
MrAlias
added a commit
to MrAlias/opentelemetry-go
that referenced
this issue
Sep 13, 2024
This verifies this as a fix to open-telemetry#5814.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
A rand.Rand instance is used without locking in rand.go, leading to potential panics.
Environment
Steps To Reproduce
Int64Histogram.Recordfrom multiple goroutines.Expected behavior
No panics from code with concurrent calls to a histogram.
The text was updated successfully, but these errors were encountered: