tracerOptions.RandomNumber accepts uint64 generator, but ProbabilisticSampler compares as 63bit integer.

[funny-falcon]

Requirement - what kind of business use case are you trying to solve?

Correct probabilistic sampler tuning.

Problem - what in Jaeger blocks you from solving the requirement?

tracerOptions.RandomNumber accepts func() uint64 generator. User assumes it should generate full 64bit integer, and documentation of RandomNumber doesn't mention anything contradict to this assumption.

jaeger-client-go/tracer_options.go

Line 72 in 47258d8

func (tracerOptions) RandomNumber(randomNumber func() uint64) TracerOption {

but Probabilistic sampler acts as if it gots 63bit integer from random generator:

jaeger-client-go/sampler.go

Line 106 in 47258d8

const maxRandomNumber = ^(uint64(1) << 63) // i.e. 0x7fffffffffffffff

jaeger-client-go/sampler.go

Line 129 in 47258d8

s.samplingBoundary = uint64(float64(maxRandomNumber) * s.samplingRate)

and default random generator actually returns 63bit integers:

jaeger-client-go/tracer.go

Line 135 in 47258d8

number := uint64(generator.Int63())

Proposal - what do you suggest to solve the problem or improve the existing situation?

• either change default generator to return full 64bit integer and fix Probabilistic sampler to react on full 64bit integer
• or document tracerOption.RandomNumber that generator function should return 63bit integer

[yurishkuro]

I think it's ok if the custom generator is 64 or 63 bits, but if the sampler is expecting 63bits, then it should unset the top bit of the ID before making a comparison here:

jaeger-client-go/sampler.go

Line 144 in 47258d8

return s.samplingBoundary >= id.Low, s.tags

[funny-falcon]

Master said, and follower was enlighted :-)

Your suggestion is really "beautiful in its simplicity"