Update Sampler API

env.go

@@ -2,7 +2,6 @@ package elasticapm
 
 import (
 	"fmt"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -123,8 +122,7 @@ func initialSampler() (Sampler, error) {
 			envTransactionSampleRate, value,
 		)
 	}
-	source := rand.NewSource(time.Now().Unix())
-	return NewRatioSampler(ratio, source), nil
+	return NewRatioSampler(ratio), nil
 }
 
 func initialSanitizedFieldNamesRegexp() (*regexp.Regexp, error) {

sampler.go

@@ -1,55 +1,49 @@
 package elasticapm
 
 import (
-	"math/rand"
-	"sync"
+	"encoding/binary"
+	"math"
+	"math/big"
 
 	"github.com/pkg/errors"
 )
 
 // Sampler provides a means of sampling transactions.
 type Sampler interface {
-	// Sample indicates whether or not the transaction
+	// Sample indicates whether or not a transaction
 	// should be sampled. This method will be invoked
-	// by calls to Tracer.StartTransaction, so it must
-	// be goroutine-safe, and should avoid synchronization
-	// as far as possible.
-	Sample(*Transaction) bool
+	// by calls to Tracer.StartTransaction for the root
+	// of a trace, so it must be goroutine-safe, and
+	// should avoid synchronization as far as possible.
+	Sample(TraceContext) bool
 }
 
-// RatioSampler is a Sampler that samples probabilistically
-// based on the given ratio within the range [0,1.0].
+// NewRatioSampler returns a new Sampler with the given ratio
 //
 // A ratio of 1.0 samples 100% of transactions, a ratio of 0.5
-// samples ~50%, and so on.
-type RatioSampler struct {
-	mu  sync.Mutex
-	rng *rand.Rand
-	r   float64
-}
-
-// NewRatioSampler returns a new RatioSampler with the given ratio
-// and math/rand.Source. The source will be called from multiple
-// goroutines, but the sampler will internally synchronise access
-// to it; the source should not be used by any other goroutines.
+// samples ~50%, and so on. If the ratio provided does not lie
+// within the range [0,1.0], NewRatioSampler will panic.
 //
-// If the ratio provided does not lie within the range [0,1.0],
-// NewRatioSampler will panic.
-func NewRatioSampler(r float64, source rand.Source) *RatioSampler {
+// The returned Sampler bases its decision on the value of the
+// transaction ID, so there is no synchronization involved.
+func NewRatioSampler(r float64) Sampler {
 	if r < 0 || r > 1.0 {
 		panic(errors.Errorf("ratio %v out of range [0,1.0]", r))
 	}
-	return &RatioSampler{
-		rng: rand.New(source),
-		r:   r,
-	}
+	var x big.Float
+	x.SetUint64(math.MaxUint64)
+	x.Mul(&x, big.NewFloat(r))
+	ceil, _ := x.Uint64()
+	return ratioSampler{ceil}
+}
+
+type ratioSampler struct {
+	ceil uint64
 }
 
 // Sample samples the transaction according to the configured
 // ratio and pseudo-random source.
-func (s *RatioSampler) Sample(*Transaction) bool {
-	s.mu.Lock()
-	v := s.rng.Float64()
-	s.mu.Unlock()
-	return s.r > v
+func (s ratioSampler) Sample(c TraceContext) bool {
+	v := binary.BigEndian.Uint64(c.Span[:])
+	return v > 0 && v-1 < s.ceil
 }

sampler_test.go

@@ -1,6 +1,7 @@
 package elasticapm_test
 
 import (
+	"encoding/binary"
 	"math/rand"
 	"sync"
 	"testing"
@@ -12,8 +13,7 @@ import (
 
 func TestRatioSampler(t *testing.T) {
 	ratio := 0.75
-	source := rand.NewSource(0) // fixed seed for test
-	s := elasticapm.NewRatioSampler(ratio, source)
+	s := elasticapm.NewRatioSampler(ratio)
 
 	const (
 		numGoroutines = 100
@@ -26,8 +26,13 @@ func TestRatioSampler(t *testing.T) {
 		wg.Add(1)
 		go func(i int) {
 			defer wg.Done()
+
+			// fixed seed to avoid intermittent failures
+			rng := rand.New(rand.NewSource(int64(i)))
 			for j := 0; j < numIterations; j++ {
-				if s.Sample(nil) {
+				var traceContext elasticapm.TraceContext
+				binary.LittleEndian.PutUint64(traceContext.Span[:], rng.Uint64())
+				if s.Sample(traceContext) {
 					sampled[i]++
 				}
 			}
@@ -42,3 +47,25 @@ func TestRatioSampler(t *testing.T) {
 	}
 	assert.InDelta(t, ratio, float64(total)/(numGoroutines*numIterations), 0.1)
 }
+
+func TestRatioSamplerAlways(t *testing.T) {
+	s := elasticapm.NewRatioSampler(1.0)
+	assert.False(t, s.Sample(elasticapm.TraceContext{})) // invalid span ID
+	assert.True(t, s.Sample(elasticapm.TraceContext{
+		Span: elasticapm.SpanID{0, 0, 0, 0, 0, 0, 0, 1},
+	}))
+	assert.True(t, s.Sample(elasticapm.TraceContext{
+		Span: elasticapm.SpanID{255, 255, 255, 255, 255, 255, 255, 255},
+	}))
+}
+
+func TestRatioSamplerNever(t *testing.T) {
+	s := elasticapm.NewRatioSampler(0)
+	assert.False(t, s.Sample(elasticapm.TraceContext{})) // invalid span ID
+	assert.False(t, s.Sample(elasticapm.TraceContext{
+		Span: elasticapm.SpanID{0, 0, 0, 0, 0, 0, 0, 1},
+	}))
+	assert.False(t, s.Sample(elasticapm.TraceContext{
+		Span: elasticapm.SpanID{255, 255, 255, 255, 255, 255, 255, 255},
+	}))
+}

span_test.go

@@ -1,7 +1,6 @@
 package elasticapm_test
 
 import (
-	"math/rand"
 	"testing"
 	"time"
 
@@ -15,7 +14,7 @@ func TestStartSpanTransactionNotSampled(t *testing.T) {
 	tracer, _ := elasticapm.NewTracer("tracer_testing", "")
 	defer tracer.Close()
 	// sample nothing
-	tracer.SetSampler(elasticapm.NewRatioSampler(0, rand.New(rand.NewSource(0))))
+	tracer.SetSampler(elasticapm.NewRatioSampler(0))
 
 	tx := tracer.StartTransaction("name", "type")
 	assert.False(t, tx.Sampled())

transaction.go

@@ -65,7 +65,7 @@ func (t *Tracer) StartTransactionOptions(name, transactionType string, opts Tran
 		t.samplerMu.RLock()
 		sampler := t.sampler
 		t.samplerMu.RUnlock()
-		if sampler == nil || sampler.Sample(tx) {
+		if sampler == nil || sampler.Sample(tx.traceContext) {
 			o := tx.traceContext.Options.WithRequested(true).WithMaybeRecorded(true)
 			tx.traceContext.Options = o
 		}

transaction_test.go

@@ -30,7 +30,7 @@ func TestStartTransactionTraceContextOptions(t *testing.T) {
 func startTransactionTraceContextOptions(t *testing.T, requested, maybeRecorded bool) elasticapm.TraceContext {
 	tracer, _ := transporttest.NewRecorderTracer()
 	defer tracer.Close()
-	tracer.SetSampler(samplerFunc(func(*elasticapm.Transaction) bool {
+	tracer.SetSampler(samplerFunc(func(elasticapm.TraceContext) bool {
 		panic("nope")
 	}))
 
@@ -65,7 +65,7 @@ func startTransactionInvalidTraceContext(t *testing.T, traceContext elasticapm.T
 	defer tracer.Close()
 
 	var samplerCalled bool
-	tracer.SetSampler(samplerFunc(func(*elasticapm.Transaction) bool {
+	tracer.SetSampler(samplerFunc(func(elasticapm.TraceContext) bool {
 		samplerCalled = true
 		return true
 	}))
@@ -76,8 +76,8 @@ func startTransactionInvalidTraceContext(t *testing.T, traceContext elasticapm.T
 	assert.True(t, samplerCalled)
 }
 
-type samplerFunc func(*elasticapm.Transaction) bool
+type samplerFunc func(elasticapm.TraceContext) bool
 
-func (f samplerFunc) Sample(tx *elasticapm.Transaction) bool {
-	return f(tx)
+func (f samplerFunc) Sample(t elasticapm.TraceContext) bool {
+	return f(t)
 }