From 3b9de114ae00f63f929137e0b842ec72e9261796 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 5 May 2020 15:22:42 +0200 Subject: [PATCH 1/5] rand algorithm --- tracer/util.go | 61 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/tracer/util.go b/tracer/util.go index 42a4f83a..321a4d0a 100644 --- a/tracer/util.go +++ b/tracer/util.go @@ -2,7 +2,8 @@ package tracer import ( cryptorand "crypto/rand" - "encoding/binary" + "math" + "math/big" "math/rand" "sync" "time" @@ -10,33 +11,49 @@ import ( "go.undefinedlabs.com/scopeagent/instrumentation" ) -var ( - seededIDGen = rand.New(rand.NewSource(generateSeed())) - // The golang rand generators are *not* intrinsically thread-safe. - seededIDLock sync.Mutex -) +// random holds a thread-safe source of random numbers. +var random *rand.Rand -func generateSeed() int64 { - var b [8]byte - _, err := cryptorand.Read(b[:]) - if err != nil { - instrumentation.Logger().Printf("cryptorand error: %v. \n falling back to time.Now()", err) - // Cannot seed math/rand package with cryptographically secure random number generator - // Fallback to time.Now() - return time.Now().UnixNano() +func init() { + var seed int64 + n, err := cryptorand.Int(cryptorand.Reader, big.NewInt(math.MaxInt64)) + if err == nil { + seed = n.Int64() + } else { + instrumentation.Logger().Printf("cryptorand error generating seed: %v. \n falling back to time.Now()", err) + seed = time.Now().UnixNano() } - - return int64(binary.LittleEndian.Uint64(b[:])) + random = rand.New(&safeSource{ + source: rand.NewSource(seed), + }) } func randomID() uint64 { - seededIDLock.Lock() - defer seededIDLock.Unlock() - return uint64(seededIDGen.Int63()) + return random.Uint64() } func randomID2() (uint64, uint64) { - seededIDLock.Lock() - defer seededIDLock.Unlock() - return uint64(seededIDGen.Int63()), uint64(seededIDGen.Int63()) + return random.Uint64(), random.Uint64() +} + +// safeSource holds a thread-safe implementation of rand.Source64. +type safeSource struct { + source rand.Source + sync.Mutex +} + +func (rs *safeSource) Int63() int64 { + rs.Lock() + n := rs.source.Int63() + rs.Unlock() + + return n +} + +func (rs *safeSource) Uint64() uint64 { return uint64(rs.Int63()) } + +func (rs *safeSource) Seed(seed int64) { + rs.Lock() + rs.source.Seed(seed) + rs.Unlock() } From d08b169b49c0e2cdd6bb9ab87bb7a530b8d41449 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 5 May 2020 16:08:14 +0200 Subject: [PATCH 2/5] refactoring --- tracer/bench_test.go | 2 +- tracer/tracer.go | 5 +++-- tracer/util.go | 18 +++++++----------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/tracer/bench_test.go b/tracer/bench_test.go index c9c7d57b..808a974f 100644 --- a/tracer/bench_test.go +++ b/tracer/bench_test.go @@ -14,7 +14,7 @@ var tags []string func init() { tags = make([]string, 1000) for j := 0; j < len(tags); j++ { - tags[j] = fmt.Sprintf("%d", randomID()) + tags[j] = fmt.Sprintf("%d", random.Uint64()) } } diff --git a/tracer/tracer.go b/tracer/tracer.go index a6c05c7e..3059d2d6 100644 --- a/tracer/tracer.go +++ b/tracer/tracer.go @@ -182,7 +182,7 @@ ReferencesLoop: refCtx := ref.ReferencedContext.(SpanContext) sp.raw.Context.TraceID = refCtx.TraceID - sp.raw.Context.SpanID = randomID() + sp.raw.Context.SpanID = random.Uint64() sp.raw.Context.Sampled = refCtx.Sampled sp.raw.ParentSpanID = refCtx.SpanID @@ -198,7 +198,8 @@ ReferencesLoop: if sp.raw.Context.TraceID == 0 { // No parent Span found; allocate new trace and span ids and determine // the Sampled status. - sp.raw.Context.TraceID, sp.raw.Context.SpanID = randomID2() + sp.raw.Context.TraceID = random.Uint64() + sp.raw.Context.SpanID = random.Uint64() sp.raw.Context.Sampled = t.options.ShouldSample(sp.raw.Context.TraceID) } diff --git a/tracer/util.go b/tracer/util.go index 321a4d0a..9bf7bca2 100644 --- a/tracer/util.go +++ b/tracer/util.go @@ -15,6 +15,12 @@ import ( var random *rand.Rand func init() { + random = rand.New(&safeSource{ + source: rand.NewSource(getSeed()), + }) +} + +func getSeed() int64 { var seed int64 n, err := cryptorand.Int(cryptorand.Reader, big.NewInt(math.MaxInt64)) if err == nil { @@ -23,17 +29,7 @@ func init() { instrumentation.Logger().Printf("cryptorand error generating seed: %v. \n falling back to time.Now()", err) seed = time.Now().UnixNano() } - random = rand.New(&safeSource{ - source: rand.NewSource(seed), - }) -} - -func randomID() uint64 { - return random.Uint64() -} - -func randomID2() (uint64, uint64) { - return random.Uint64(), random.Uint64() + return seed } // safeSource holds a thread-safe implementation of rand.Source64. From 462baae7f82fbb496763cddd007659d3880e94f1 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 5 May 2020 16:43:52 +0200 Subject: [PATCH 3/5] adding some jitter in the clock based seed --- tracer/util.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tracer/util.go b/tracer/util.go index 9bf7bca2..52e83ba3 100644 --- a/tracer/util.go +++ b/tracer/util.go @@ -20,6 +20,7 @@ func init() { }) } +//go:noinline func getSeed() int64 { var seed int64 n, err := cryptorand.Int(cryptorand.Reader, big.NewInt(math.MaxInt64)) @@ -27,8 +28,18 @@ func getSeed() int64 { seed = n.Int64() } else { instrumentation.Logger().Printf("cryptorand error generating seed: %v. \n falling back to time.Now()", err) - seed = time.Now().UnixNano() + + // Adding some jitter to the clock seed using golang channels and goroutines + jitterStart := time.Now() + cb := make(chan time.Time, 0) + go func() { cb <- <-time.After(time.Nanosecond) }() + now := <-cb + jitter := time.Since(jitterStart) + + // Seed based on the clock + some jitter + seed = now.Add(jitter).UnixNano() } + instrumentation.Logger().Printf("seed: %d", seed) return seed } From 2d11565630dd47833ffe78f6264e3a846cc30243 Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 5 May 2020 16:52:53 +0200 Subject: [PATCH 4/5] initialize random on demand --- tracer/bench_test.go | 2 +- tracer/tracer.go | 6 +++--- tracer/util.go | 19 +++++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/tracer/bench_test.go b/tracer/bench_test.go index 808a974f..68b5ffff 100644 --- a/tracer/bench_test.go +++ b/tracer/bench_test.go @@ -14,7 +14,7 @@ var tags []string func init() { tags = make([]string, 1000) for j := 0; j < len(tags); j++ { - tags[j] = fmt.Sprintf("%d", random.Uint64()) + tags[j] = fmt.Sprintf("%d", getRandomId()) } } diff --git a/tracer/tracer.go b/tracer/tracer.go index 3059d2d6..dcc38cbf 100644 --- a/tracer/tracer.go +++ b/tracer/tracer.go @@ -182,7 +182,7 @@ ReferencesLoop: refCtx := ref.ReferencedContext.(SpanContext) sp.raw.Context.TraceID = refCtx.TraceID - sp.raw.Context.SpanID = random.Uint64() + sp.raw.Context.SpanID = getRandomId() sp.raw.Context.Sampled = refCtx.Sampled sp.raw.ParentSpanID = refCtx.SpanID @@ -198,8 +198,8 @@ ReferencesLoop: if sp.raw.Context.TraceID == 0 { // No parent Span found; allocate new trace and span ids and determine // the Sampled status. - sp.raw.Context.TraceID = random.Uint64() - sp.raw.Context.SpanID = random.Uint64() + sp.raw.Context.TraceID = getRandomId() + sp.raw.Context.SpanID = getRandomId() sp.raw.Context.Sampled = t.options.ShouldSample(sp.raw.Context.TraceID) } diff --git a/tracer/util.go b/tracer/util.go index 52e83ba3..2fb4a447 100644 --- a/tracer/util.go +++ b/tracer/util.go @@ -11,13 +11,20 @@ import ( "go.undefinedlabs.com/scopeagent/instrumentation" ) -// random holds a thread-safe source of random numbers. -var random *rand.Rand +var ( + random *rand.Rand + mu sync.Mutex +) -func init() { - random = rand.New(&safeSource{ - source: rand.NewSource(getSeed()), - }) +func getRandomId() uint64 { + mu.Lock() + if random == nil { + random = rand.New(&safeSource{ + source: rand.NewSource(getSeed()), + }) + } + mu.Unlock() + return random.Uint64() } //go:noinline From 631170428388025ee1e609b7f9ccc98296c16f7f Mon Sep 17 00:00:00 2001 From: Daniel Redondo Date: Tue, 5 May 2020 16:54:20 +0200 Subject: [PATCH 5/5] refactor --- tracer/util.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tracer/util.go b/tracer/util.go index 2fb4a447..04f5c33e 100644 --- a/tracer/util.go +++ b/tracer/util.go @@ -17,14 +17,18 @@ var ( ) func getRandomId() uint64 { + ensureRandom() + return random.Uint64() +} + +func ensureRandom() { mu.Lock() + defer mu.Unlock() if random == nil { random = rand.New(&safeSource{ source: rand.NewSource(getSeed()), }) } - mu.Unlock() - return random.Uint64() } //go:noinline