BoundarySampler has an effective minimum rate of 0.5 on amd64 architectures

[jamesmoessis]

I've written a test to demonstrate the behaviour:

func TestFindTraceIDs(t *testing.T) {
	rand.Uint64()
	c := 0
	samplingRate := 0.01
	salt := rand.Int63()
	sampler, _ := zipkin.NewBoundarySampler(samplingRate, salt)
	n := 10000
	for i := 0; i < n; i++ {
		id := rand.Uint64()
		val := sampler(id)
		if val {
			c++
			//t.Log(fmt.Sprintf("id sampled: %08x", id))
		}
	}
	t.Log(fmt.Sprintf("total IDs sampled: %d out of %d", c, n))
}

Example run on arm64:

tracing_test.go:106: total IDs sampled: 49 out of 10000

Example run on amd64 (using golang:1.22 docker container):

tracing_test.go:106: total IDs sampled: 4987 out of 10000

This huge difference in sampled trace IDs is due to the conversion from float64 to int64 that occurs in the sampler, which is platform dependent with high values such as trace IDs.

[codefromthecrypt]

This project is somewhat dormant, but just for fun I'd love to review an alternative that avoids this. Up for a PR?

[jamesmoessis]

@codefromthecrypt thanks for quick reply. I've implemented a sampler locally that avoids this. I'd be happy to raise a PR

[jamesmoessis]

This is what I have

func newProbabilisticSampler(rate float64) (zipkin.Sampler, error) {
	if rate > 1.0 || rate < 0.0 {
		return nil, errors.New("invalid rate, must be between 0 and 1")
	}
	upperBound := uint64(rate * (1 << 63))
	return func(id uint64) bool {
		return (id >> 1) < upperBound
	}, nil
}

[jamesmoessis]

PR up^