From a571c52b0ae529feb9df2570802aa213995e52dc Mon Sep 17 00:00:00 2001 From: Sean Liao Date: Thu, 15 May 2025 08:21:49 +0100 Subject: [PATCH] all: replace math/rand with math/rand/v2 (#6732) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update to new stdlib apis. The new Float64 is uniform, which resolves a long comment. https://cs.opensource.google/go/go/+/refs/tags/go1.24.2:src/math/rand/v2/rand.go;l=209 > // There are exactly 1<<53 float64s in [0,1). Use Intn(1<<53) / (1<<53). return float64(r.Uint64()<<11>>11) / (1 << 53) ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/trace cpu: 12th Gen Intel(R) Core(TM) i7-1260P │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ TraceStart/with_a_simple_span-16 387.1n ± 5% 306.8n ± 15% -20.73% (p=0.000 n=10) TraceStart/with_several_links-16 542.2n ± 5% 501.0n ± 2% -7.61% (p=0.000 n=10) TraceStart/with_attributes-16 521.4n ± 14% 571.6n ± 6% +9.64% (p=0.009 n=10) geomean 478.3n 444.6n -7.05% │ old.txt │ new.txt │ │ B/op │ B/op vs base │ TraceStart/with_a_simple_span-16 528.0 ± 0% 528.0 ± 0% ~ (p=1.000 n=10) ¹ TraceStart/with_several_links-16 704.0 ± 0% 704.0 ± 0% ~ (p=1.000 n=10) ¹ TraceStart/with_attributes-16 784.0 ± 0% 784.0 ± 0% ~ (p=1.000 n=10) ¹ geomean 663.0 663.0 +0.00% ¹ all samples are equal │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ TraceStart/with_a_simple_span-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=10) ¹ TraceStart/with_several_links-16 3.000 ± 0% 3.000 ± 0% ~ (p=1.000 n=10) ¹ TraceStart/with_attributes-16 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹ geomean 2.884 2.884 +0.00% ¹ all samples are equal ``` --------- Co-authored-by: Robert Pająk Co-authored-by: Damien Mathieu <42@dmathieu.com> --- baggage/baggage_test.go | 12 +++---- .../internal/ocmetric/metric_test.go | 4 +-- bridge/opentracing/internal/mock.go | 8 +++-- metric/example_test.go | 6 ++-- sdk/metric/exemplar/fixed_size_reservoir.go | 36 +++++-------------- .../exemplar/fixed_size_reservoir_test.go | 8 +++-- sdk/resource/benchmark_test.go | 8 ++--- sdk/trace/id_generator.go | 26 ++++---------- sdk/trace/provider_test.go | 2 +- sdk/trace/sampling_test.go | 2 +- 10 files changed, 41 insertions(+), 71 deletions(-) diff --git a/baggage/baggage_test.go b/baggage/baggage_test.go index fed443e6e..749bd328f 100644 --- a/baggage/baggage_test.go +++ b/baggage/baggage_test.go @@ -5,7 +5,7 @@ package baggage import ( "fmt" - "math/rand" + "math/rand/v2" "slices" "strings" "testing" @@ -17,12 +17,8 @@ import ( "go.opentelemetry.io/otel/internal/baggage" ) -var rng *rand.Rand - -func init() { - // Seed with a static value to ensure deterministic results. - rng = rand.New(rand.NewSource(1)) -} +// Seed with a static value to ensure deterministic results. +var rng = rand.New(rand.NewChaCha8([32]byte{})) func TestValidateKeyChar(t *testing.T) { // ASCII only @@ -255,7 +251,7 @@ func key(n int) string { b := make([]rune, n) for i := range b { - b[i] = r[rng.Intn(len(r))] + b[i] = r[rng.IntN(len(r))] } return string(b) } diff --git a/bridge/opencensus/internal/ocmetric/metric_test.go b/bridge/opencensus/internal/ocmetric/metric_test.go index f60ace316..5f1e3e75e 100644 --- a/bridge/opencensus/internal/ocmetric/metric_test.go +++ b/bridge/opencensus/internal/ocmetric/metric_test.go @@ -7,7 +7,7 @@ import ( "errors" "fmt" "math" - "math/rand" + "math/rand/v2" "reflect" "strconv" "testing" @@ -1189,7 +1189,7 @@ func BenchmarkConvertExemplar(b *testing.B) { for i := range data { a := make(ocmetricdata.Attachments, attchmentsN) for j := 0; j < attchmentsN; j++ { - a[strconv.Itoa(j)] = rand.Int63() + a[strconv.Itoa(j)] = rand.Int64() } data[i] = &ocmetricdata.Exemplar{ Value: rand.NormFloat64(), diff --git a/bridge/opentracing/internal/mock.go b/bridge/opentracing/internal/mock.go index bf99d1db7..6e09992e4 100644 --- a/bridge/opentracing/internal/mock.go +++ b/bridge/opentracing/internal/mock.go @@ -5,7 +5,7 @@ package internal // import "go.opentelemetry.io/otel/bridge/opentracing/internal import ( "context" - "math/rand" + "math/rand/v2" "reflect" "sync" "time" @@ -44,7 +44,7 @@ type MockTracer struct { TraceFlags trace.TraceFlags randLock sync.Mutex - rand *rand.Rand + rand *rand.ChaCha8 } var ( @@ -53,13 +53,15 @@ var ( ) func NewMockTracer() *MockTracer { + u := rand.Uint32() + seed := [32]byte{byte(u), byte(u >> 8), byte(u >> 16), byte(u >> 24)} return &MockTracer{ FinishedSpans: nil, SpareTraceIDs: nil, SpareSpanIDs: nil, SpareContextKeyValues: nil, - rand: rand.New(rand.NewSource(time.Now().Unix())), + rand: rand.NewChaCha8(seed), } } diff --git a/metric/example_test.go b/metric/example_test.go index a60c8d479..eaa72f9a8 100644 --- a/metric/example_test.go +++ b/metric/example_test.go @@ -7,7 +7,7 @@ import ( "context" "database/sql" "fmt" - "math/rand" + "math/rand/v2" "net/http" "runtime" "time" @@ -159,7 +159,7 @@ func ExampleMeter_gauge() { getCPUFanSpeed := func() int64 { // Generates a random fan speed for demonstration purpose. // In real world applications, replace this to get the actual fan speed. - return int64(1500 + rand.Intn(1000)) + return int64(1500 + rand.IntN(1000)) } fanSpeedSubscription := make(chan int64, 1) @@ -170,7 +170,7 @@ func ExampleMeter_gauge() { // Synchronous gauges are used when the measurement cycle is // synchronous to an external change. // Simulate that external cycle here. - time.Sleep(time.Duration(rand.Intn(3)) * time.Second) + time.Sleep(time.Duration(rand.IntN(3)) * time.Second) fanSpeed := getCPUFanSpeed() fanSpeedSubscription <- fanSpeed } diff --git a/sdk/metric/exemplar/fixed_size_reservoir.go b/sdk/metric/exemplar/fixed_size_reservoir.go index d4aab0aad..1fb1e0095 100644 --- a/sdk/metric/exemplar/fixed_size_reservoir.go +++ b/sdk/metric/exemplar/fixed_size_reservoir.go @@ -6,7 +6,7 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/exemplar" import ( "context" "math" - "math/rand" + "math/rand/v2" "time" "go.opentelemetry.io/otel/attribute" @@ -44,18 +44,11 @@ type FixedSizeReservoir struct { // w is the largest random number in a distribution that is used to compute // the next next. w float64 - - // rng is used to make sampling decisions. - // - // Do not use crypto/rand. There is no reason for the decrease in performance - // given this is not a security sensitive decision. - rng *rand.Rand } func newFixedSizeReservoir(s *storage) *FixedSizeReservoir { r := &FixedSizeReservoir{ storage: s, - rng: rand.New(rand.NewSource(time.Now().UnixNano())), } r.reset() return r @@ -64,26 +57,15 @@ func newFixedSizeReservoir(s *storage) *FixedSizeReservoir { // randomFloat64 returns, as a float64, a uniform pseudo-random number in the // open interval (0.0,1.0). func (r *FixedSizeReservoir) randomFloat64() float64 { - // TODO: This does not return a uniform number. rng.Float64 returns a - // uniformly random int in [0,2^53) that is divided by 2^53. Meaning it - // returns multiples of 2^-53, and not all floating point numbers between 0 - // and 1 (i.e. for values less than 2^-4 the 4 last bits of the significand - // are always going to be 0). + // TODO: Use an algorithm that avoids rejection sampling. For example: // - // An alternative algorithm should be considered that will actually return - // a uniform number in the interval (0,1). For example, since the default - // rand source provides a uniform distribution for Int63, this can be - // converted following the prototypical code of Mersenne Twister 64 (Takuji - // Nishimura and Makoto Matsumoto: - // http://www.math.sci.hiroshima-u.ac.jp/m-mat/MT/VERSIONS/C-LANG/mt19937-64.c) - // - // (float64(rng.Int63()>>11) + 0.5) * (1.0 / 4503599627370496.0) - // - // There are likely many other methods to explore here as well. - - f := r.rng.Float64() + // const precision = 1 << 53 // 2^53 + // // Generate an integer in [1, 2^53 - 1] + // v := rand.Uint64() % (precision - 1) + 1 + // return float64(v) / float64(precision) + f := rand.Float64() for f == 0 { - f = r.rng.Float64() + f = rand.Float64() } return f } @@ -146,7 +128,7 @@ func (r *FixedSizeReservoir) Offer(ctx context.Context, t time.Time, n Value, a } else { if r.count == r.next { // Overwrite a random existing measurement with the one offered. - idx := int(r.rng.Int63n(int64(cap(r.store)))) + idx := int(rand.Int64N(int64(cap(r.store)))) r.store[idx] = newMeasurement(ctx, t, n, a) r.advance() } diff --git a/sdk/metric/exemplar/fixed_size_reservoir_test.go b/sdk/metric/exemplar/fixed_size_reservoir_test.go index 0a901fd9c..3655b5862 100644 --- a/sdk/metric/exemplar/fixed_size_reservoir_test.go +++ b/sdk/metric/exemplar/fixed_size_reservoir_test.go @@ -6,10 +6,9 @@ package exemplar import ( "context" "math" - "math/rand" + "math/rand/v2" "slices" "testing" - "time" "github.com/stretchr/testify/assert" ) @@ -28,7 +27,10 @@ func TestNewFixedSizeReservoirSamplingCorrectness(t *testing.T) { intensity := 0.1 sampleSize := 1000 - rng := rand.New(rand.NewSource(time.Now().UnixNano())) + u := rand.Uint32() + seed := [32]byte{byte(u), byte(u >> 8), byte(u >> 16), byte(u >> 24)} + t.Logf("rng seed: %x", seed) + rng := rand.New(rand.NewChaCha8(seed)) data := make([]float64, sampleSize*1000) for i := range data { diff --git a/sdk/resource/benchmark_test.go b/sdk/resource/benchmark_test.go index 6f1e6afe1..94f89b244 100644 --- a/sdk/resource/benchmark_test.go +++ b/sdk/resource/benchmark_test.go @@ -5,7 +5,7 @@ package resource_test import ( "fmt" - "math/rand" + "math/rand/v2" "testing" "go.opentelemetry.io/otel/attribute" @@ -21,18 +21,18 @@ func makeAttrs(n int) (_, _ *resource.Resource) { for i := 0; i < n; i++ { var k string for { - k = fmt.Sprint("k", rand.Intn(1000000000)) + k = fmt.Sprint("k", rand.IntN(1000000000)) if !used[k] { used[k] = true break } } - l1[i] = attribute.String(k, fmt.Sprint("v", rand.Intn(1000000000))) + l1[i] = attribute.String(k, fmt.Sprint("v", rand.IntN(1000000000))) if rand.Float64() < conflict { l2[i] = l1[i] } else { - l2[i] = attribute.String(k, fmt.Sprint("v", rand.Intn(1000000000))) + l2[i] = attribute.String(k, fmt.Sprint("v", rand.IntN(1000000000))) } } return resource.NewSchemaless(l1...), resource.NewSchemaless(l2...) diff --git a/sdk/trace/id_generator.go b/sdk/trace/id_generator.go index 925bcf993..c8d3fb7e3 100644 --- a/sdk/trace/id_generator.go +++ b/sdk/trace/id_generator.go @@ -5,10 +5,8 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace" import ( "context" - crand "crypto/rand" "encoding/binary" - "math/rand" - "sync" + "math/rand/v2" "go.opentelemetry.io/otel/trace" ) @@ -29,20 +27,15 @@ type IDGenerator interface { // must never be done outside of a new major release. } -type randomIDGenerator struct { - sync.Mutex - randSource *rand.Rand -} +type randomIDGenerator struct{} var _ IDGenerator = &randomIDGenerator{} // NewSpanID returns a non-zero span ID from a randomly-chosen sequence. func (gen *randomIDGenerator) NewSpanID(ctx context.Context, traceID trace.TraceID) trace.SpanID { - gen.Lock() - defer gen.Unlock() sid := trace.SpanID{} for { - _, _ = gen.randSource.Read(sid[:]) + binary.NativeEndian.PutUint64(sid[:], rand.Uint64()) if sid.IsValid() { break } @@ -53,18 +46,17 @@ func (gen *randomIDGenerator) NewSpanID(ctx context.Context, traceID trace.Trace // NewIDs returns a non-zero trace ID and a non-zero span ID from a // randomly-chosen sequence. func (gen *randomIDGenerator) NewIDs(ctx context.Context) (trace.TraceID, trace.SpanID) { - gen.Lock() - defer gen.Unlock() tid := trace.TraceID{} sid := trace.SpanID{} for { - _, _ = gen.randSource.Read(tid[:]) + binary.NativeEndian.PutUint64(tid[:8], rand.Uint64()) + binary.NativeEndian.PutUint64(tid[8:], rand.Uint64()) if tid.IsValid() { break } } for { - _, _ = gen.randSource.Read(sid[:]) + binary.NativeEndian.PutUint64(sid[:], rand.Uint64()) if sid.IsValid() { break } @@ -73,9 +65,5 @@ func (gen *randomIDGenerator) NewIDs(ctx context.Context) (trace.TraceID, trace. } func defaultIDGenerator() IDGenerator { - gen := &randomIDGenerator{} - var rngSeed int64 - _ = binary.Read(crand.Reader, binary.LittleEndian, &rngSeed) - gen.randSource = rand.New(rand.NewSource(rngSeed)) - return gen + return &randomIDGenerator{} } diff --git a/sdk/trace/provider_test.go b/sdk/trace/provider_test.go index 4ccc69d7d..bb4459284 100644 --- a/sdk/trace/provider_test.go +++ b/sdk/trace/provider_test.go @@ -7,7 +7,7 @@ import ( "context" "errors" "fmt" - "math/rand" + "math/rand/v2" "testing" "github.com/stretchr/testify/assert" diff --git a/sdk/trace/sampling_test.go b/sdk/trace/sampling_test.go index 7480d5ef8..c871ecf4f 100644 --- a/sdk/trace/sampling_test.go +++ b/sdk/trace/sampling_test.go @@ -6,7 +6,7 @@ package trace import ( "context" "fmt" - "math/rand" + "math/rand/v2" "testing" "github.com/stretchr/testify/assert"